OracleSpatial poor performance due to scollable cursor

[msmitherdc]

As part of the one pass query optimization, the main cursor retrieving data from Oracle Spatial was changed from OCI_DEFAULT to OCI_STMT_SCROLLABLE_READONLY. This adds significant overhead and slows down data retrieval.

It needs to be determined if the one-pass query optimizations actually need the scrollable cursor and if so, determine the best method of only invoking the scrollable cursor under those circumstances.

Location of the OCI directive is ​http://trac.osgeo.org/mapserver/browser/trunk/mapserver/maporaclespatial.c#L2210

[sdlime]

Any thoughts on this one? I'm not in a position to comment although I could make the change...

Steve

[msmitherdc]

If we know the map mode (query, nquery, wfs, wms, map) that is being requested, we could invoke the scollable cursor only in the query related situations (as it is needed in those cases) and use the default cursor for mode=map/wms requests.

[aboudreault]

I'm going to take a look tomorrow.

[sdlime]

I think Tamas had the same issue with the SQL Server driver. Wonder if/how he solved it?

Steve

[msmitherdc]

This is what Tamas did

This issue is possibly related to the single pass query implementation in the Oracle driver. I've run into a similar issue with mssql2008 when trying to implement the same. In my opinion there's no need to make the cursor scrollable in most cases (like in the normal drawing) since it affects the overall performance in a harmful way. With regards to mssql2008 I've ended up with a second fetch for the query results and use the forward only cursor to get the shapes sequentially then. This results in 2 fetch instead of one but avoid scrollable cursors at all.

[aboudreault]

In the function msOracleSpatialLayerWhichShapes, I can check what the map query mode is. So, if the query mode is MS_QUERY_IS_NULL, I use OCI_DEFAULT as cursor. However, I have a few questions:

• Will this cover all cases? I see that msQueryByFeature doesn't set any query mode. It probably require the scrollable cursor?..
• I think WFS requests use the internal msQueryByRect and other functions... So they should be fine?
• An issue with my solution is that if (ie. in mapscript) I execute a whichShapes, the default cursor will be used. but if I do a queryByRect(), then a whichShapes later... the cursor will still be scrollable because the query is still set. Is this an issue at this point?

Could anyone tell me what modes need a scrollable cursor (other than query, nquery) ? I will also have to add some other check. ie. all wms requests don't need a scrollable cursor.

[dmorissette]

When do we need the scrollable cursor anyway? I could be wrong, but do the CGI query modes, as well as WFS, etc. not simply do a single sequential scan of the results which is the whole idea of "single-pass query"? Would the scrollable cursor be useful only in the mapscript case then? If that's the case then perhaps MapScript could have an additional arg/hint to request the scrollable cursor?

[msmitherdc]

Wfs didn't work for me with the default cursor

[sdlime]

I'm assuming the scrolling cursor is needed with query processing. One-pass is a not really true. It's really two-pass instead the old method that did loads of selects by a unique id. Things like GML require a first pass to compile a bounding box and then you have the presentation of actual geometries. The cursor is reset then I guess.

All the query modes need it then I presume. I guess we need a reliable way to know if we're processing a query. The map->query object would be the place to manage it by setting and checking map->query.type. If it's set to MS_QUERY_IS_NULL then you shouldn't need the scrollable cursor.

msQueryByFeatures() actually should already have a query type set before it's called. It assumes another query has been done beforehand (e.g. find all counties with names starting with 'C') and uses those results to generate a new result set. In fact, we could check to make sure the query type is not MS_QUERY_IS_NULL at the top of msQueryByFeatures()...

Queries themselves are easy because msLayerWhichShapes() is called once you know a layer is to be queried so you just look at the map->query.type. It's query maps that present the problem since you might have a query set and then when drawing the querymap (with certain types) you could process layers not impacted by the query and slow things down with the scrollable cursor.

Might consider passing an extra context variable (e.g. MS_CONTEXT_RENDER, MS_CONTEXT_QUERY) to msLayerWhichShapes() and the underlying driver can make use of it if necessary. The function isn't called that many places and it's clear which context to use...

Steve

[tamas]

Currently the map->query.type is not handled in a consistent way. For example when using drawQuery in highlight mode the background image is drawn also in query mode. I'm also uncertain how these multiple drawing sessions (with switching the cursor type) would work in a connection pooling environment with the OCI driver.

Another question is that do we really make use of the scrollable cursor in a query operation? In most cases we just want to rewind the cursor back to the first record and continue to fetch the records sequentially then. In this case using a second query (to reset the forward only cursor) may probably be more efficient.

[sdlime]

map->query.type is probably not cleared consistent enough, should be reset in msLayerClose() if not already.

msLayerWhichShapes() is used to generate a list of candidate shapes. The query functions then refine that list (via msLayerNextShape()) and store row references. It's msLayerGetShape() then that uses the still-open result set (cursor). It's not a rewind. I don't know what makes sense in Oracle's case in this sequence of operations.

The more I think about it the more adding flag to msLayerWhichShapes() makes sense. We could try that in trunk and see how it works?

Steve

[tamas]

Replying to sdlime:

msLayerWhichShapes() is used to generate a list of candidate shapes. The query functions then refine that list (via msLayerNextShape()) and store row references. It's msLayerGetShape() then that uses the still-open result set (cursor). It's not a rewind. I don't know what makes sense in Oracle's case in this sequence of operations.

In most cases the shapes in the resultcache is accessed in the same order at it was retrieved during the query. In this regard there's no need to establish a random access to the resultset, a forward only cursor would also be sufficient (see the OGR and the MSSQL drivers for reference)

By utilizing this sequence the driver just require to store the recent query (statement) and reissue that to the data source in case if the specified resultindex is less then the last resultindex retrieved by the driver. (kinda like a rewind operation)

[sdlime]

So what needs to happen here then? These are changes I can't make...

Steve

[msmitherdc]

The forward only cursor is not sufficient in the WFS case, I get

msOracleSpatialLayerGetShape(): OracleSpatial error. Shape type is null... this probably means a record number was requested that could not have been in a result set (as returned by NextShape)

after changing to forward only cursor.

Steve, If you want to create a patch that adds the flag to msLayerWhichShapes, I can test it out.

[sdlime]

I've attached a patch against trunk. I've no way to test Oracle so you may have to tweak maporaclespatial.c just a touch. This basically just adds an "isQuery" flag to msLayerWhichShapes(). The Oracle driver is the only one to check it.

Steve

[sdlime]

Patch to add "isQuery" flag to msLayerWhichShapes().

[sdlime]

Mike: Any chance to test?

Steve

[msmitherdc]

Yes, i'll hopefully get to it this afternoon (which I see it is now, sigh). Maybe this evening

[msmitherdc]

All, I've tested this against Oracle using both the default cursor (mode=map) and the forward scrolling cursor (wfs). It worked fine in both cases.

[sdlime]

Any reason not to commit this fix in the 6.0 branch and trunk. It's a pretty benign change but does alter the C API. I do need to update MapScript, change will be transparent to users... Steve

[msmitherdc]

I'm happy with it on my end. I'd love to see some testing by others but it may just be easier to commit and deal with the issues that arrise.

[dmorissette]

No objection to backporting to 6.0.1 from my end, but that would have to be done ASAP.

[sdlime]

Committed in r11880 (trunk) and r11881 (branch-6-0). Marking as fixed. Let me know if anyone see's issues...

Steve

[rouault]

Steve, it seems that r11880 and r11881 broke mssql2008 plugin building. See ​http://vbkto.dyndns.org:1280/sdk/build-output/vc7-20110708-4-31-27-48-vc7-dev.txt

At first sight, it seems to be just a typo :

int msMSSQL2008LayerWhichShapes(layerObj *layer, rectObj rect, in isQuery)

instead of

int msMSSQL2008LayerWhichShapes(layerObj *layer, rectObj rect, int isQuery)

[sdlime]

Cripes, yes a typo. Will fix... Steve

[sdlime]

Fixed in r11883 and r11884. -Steve