[fdo-internals] RE: PATCH: Raster stability fixes, ticket #462

[Jason Birch]

Is there any way at that point in the code to identify the provider being used for the Raster operation, and only apply the guard if it's the GDAL provider?

I believe that there was a fair amount of effort put into guarding all of the FDO APIs in the GDAL provider already, but perhaps some got missed.  I'll be interested to hear back from Haris on his investigations into the connection manager to see if this is a combination of factors, not just the GDAL provider.

BTW, yes, having the guard in that location is somewhat slow with multiple layers/users. But it also means that my MapGuide server doesn't crash after a minute of operation once I get a few concurrent sessions going.  These patches mean that it takes at least ten minutes (I'm seeing the problem of exhausting the client connections too )...

In an ideal world, I'd be happier if there was a bare bones, multithreaded, non-configurable, seat of your pants GDAL provider that supported VRTs.  However, just being able to publicly launch the City's new web maps after a year and a half, countless hours of my time, and over $80k in external development costs would be nice :)  I've definitely had cause to regret jumping on this bandwagon early.

Jason

-----Original Message-----
From: Trevor Wekel 
Sent: Friday, March 27, 2009 11:45 AM
To: 'MapGuide Internals Mail List'; 'FDO Internals Mail List'
Subject: RE: PATCH: Raster stability fixes, ticket #462

Just to add a little more detail, the guard in question will also impact WMS consumption in MapGuide.  Querying and rendering of WMS layers from external data sources will become completely single threaded.

Moving the guard forces most of the stylization/rendering work to be single threaded.  I suspect these operations are fairly lengthy.  From what I recall, the Fdo ReadNext() call is also done within StylizeGridLayer.  If ReadNext() is "slow" for WMS and GDAL then there will be an even greater performance impact to multi-user performance when the guard is moved.

The current guard only protects setup and execution of the MgFeatureService::SelectFeatures call.  From an Fdo perspective, I believe SelectFeatures is fairly fast so guarding it does not significantly affect multi-user performance.

In my opinion, moving the guard in MapGuide will impose a heavy price performance wise.  The best option is to add guards to the GDAL Raster provider.  However, Jason is bang on with his analysis of the likelihood of any collaboration happening short term.  One possible solution would be to put a guard around all of the public Fdo APIs in the GDAL Provider.  That would not require much collaboration and would be "drop in" testable.

Thanks,
Trevor


-----Original Message-----
From: mapguide-internals-bounces at lists.osgeo.org [mailto:mapguide-internals-bounces at lists.osgeo.org] On Behalf Of Jason Birch
Sent: Friday, March 27, 2009 11:07 AM
To: 'MapGuide Internals Mail List'; 'FDO Internals Mail List'
Subject: [mapguide-internals] RE: PATCH: Raster stability fixes, ticket #462

I think that this patch should be implemented for two reasons.

First, I believe that this is no more of a sledgehammer approach than the existing guard.

Second, getting coordination between a skilled MapGuide developers that can work though the threading issues and the GDAL Provider developer has been attempted in the past and failed because nobody stepped up to the plate from the MapGuide community.

Checking this in as a temporary stability measure allows us to have a reasonable (if slow) raster story for the 2.1 release.

Of course, if there are developers from both the FDO and MapGuide communities that are able to work together to properly resolve this issue before the FDO 3.4 release (it's at RC3) and the MapGuide 2.1 release, then that would be ideal.

Jason

-----Original Message-----
From: mapguide-internals-bounces at lists.osgeo.org [mailto:mapguide-internals-bounces at lists.osgeo.org] On Behalf Of Bruce Dechant
Sent: March-27-09 8:55 AM
To: MapGuide Internals Mail List
Subject: [mapguide-internals] RE: PATCH: Raster stability fixes, ticket #462

I totally agree - the locking needs to be in the provider.

Thanks,
Bruce


-----Original Message-----
From: mapguide-internals-bounces at lists.osgeo.org [mailto:mapguide-internals-bounces at lists.osgeo.org] On Behalf Of Traian Stanev
Sent: Friday, March 27, 2009 9:40 AM
To: MapGuide Internals Mail List
Subject: [mapguide-internals] RE: PATCH: Raster stability fixes, ticket #462


It does look like a sledgehammer approach -- what if some day there is a raster provider that works ok and doesn't need the locks? If anything, the lock should be in the FDO provider.

Traian


-----Original Message-----
From: mapguide-internals-bounces at lists.osgeo.org [mailto:mapguide-internals-bounces at lists.osgeo.org] On Behalf Of Walt Welton-Lair
Sent: Friday, March 27, 2009 11:11 AM
To: MapGuide Internals Mail List
Subject: [mapguide-internals] RE: PATCH: Raster stability fixes, ticket #462

The memory patch (mapguide_raster_unalloc.5.patch) looks good, and I'll submit that.

The stability patch (mapguide_raster_stability.patch) looks reasonable, but I'd like another 1 or 2 people to also look at it.  It's significant since it limits MapGuide to processing one raster stylization request at a time, but I guess that's better than having a dead lock.  Anyone know where the thread-unsafe code actually is?  Is it in the provider itself?  Can the mutex be moved closer to the actual thread-unsafe code?

Walt

-----Original Message-----
From: mapguide-internals-bounces at lists.osgeo.org [mailto:mapguide-internals-bounces at lists.osgeo.org] On Behalf Of Jason Birch
Sent: Friday, March 27, 2009 3:10 AM
To: mapguide-internals at lists.osgeo.org
Subject: [mapguide-internals] PATCH: Raster stability fixes, ticket #462

Hi,

Haris has been working on the raster stability problem against MapGuide 2.0.2.  I was having the same problems, and was able to duplicate on 2.1.  I have worked with Haris to create patches against trunk that address a couple of the problems and increase stability immensely.

Could someone please review the patches attached to:

http://trac.osgeo.org/mapguide/ticket/462

and commit them to trunk if appropriate?

My feeling is that the connection management stuff may need some additional work, since limiting the connection pool to 1 for single-threaded providers doesn't appear to have the desired effect.  However, getting both of these patches into 2.1 would at least improve stability for Raster users.

Jason