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

[Traian Stanev]

OK, if I now enable connection pooling for GDAL and *completely remove* the global raster lock from MappingUtil.cpp, I get CPU at 100% and the test still passes! And it's fassssst... YMMV.

Traian


> -----Original Message-----
> From: mapguide-internals-bounces at lists.osgeo.org [mailto:mapguide-
> internals-bounces at lists.osgeo.org] On Behalf Of Traian Stanev
> Sent: Sunday, March 29, 2009 1:22 AM
> To: MapGuide Internals Mail List
> Cc: FDO Internals Mail List
> Subject: RE: [mapguide-internals] RE: PATCH: Raster stability fixes,
> ticket #462
> 
> 
> For what it's worth, patching the provider to addref the parent
> connection from the feature reader fixes the problem. This is on top of
> Friday's trunk MapGuide and FDO code, without any other extra fixes. I
> do think it is a bug not to addref the connection from the reader, when
> it in fact depends on it to be alive, in order to get stuff from it at
> a later time.
> 
> I am pretty sure the addref fixes the problem, because the test I am
> using is fairly brutal and produces the crash/deadlock in less than 5
> seconds every time I run it with unpatched GDAL provider. With the
> change, I ran it multiple times to completion.
> 
> I am unhappy with the fix though, because it leaves CPU usage by
> mgserver at ~5%, even though I am doing something like 16 simultaneous
> requests... So then I decided to remove the global mutex from inside
> the provider -- that didn't make a difference in CPU usage, and the
> test still passed without problems. So that mutex can go. Then I messed
> with the connection pooling settings in serverconfig.ini -- those
> didn't make any difference. I tried them all for the GDAL provider and
> the test still passed with 5% CPU usage. I don't think the process is
> blocked for I/O because the TIFF I am using is not huge and I also
> generated an overlay pyramid (which I highly recommend to anyone by the
> way, it makes a huge performance difference for TIFFs).
> 
> 
> 
> 
> Traian
> 
> 
> > -----Original Message-----
> > From: mapguide-internals-bounces at lists.osgeo.org [mailto:mapguide-
> > internals-bounces at lists.osgeo.org] On Behalf Of Haris Kurtagic
> > Sent: Saturday, March 28, 2009 4:43 AM
> > To: MapGuide Internals Mail List
> > Subject: RE: [mapguide-internals] RE: PATCH: Raster stability fixes,
> > ticket #462
> >
> > Regarding MG raster problems ( or any other provider ) the main
> reason
> > is connection manager.
> > If you would set connection pool size 1 , for sdf provider MG will
> not
> > work properly.
> >
> > The patch Jason submitted, is workaround for issue. It will not cause
> > GDAL to work slower, because provider itself is basically
> > single-threaded now.
> > That was done trough work we were doing years or so ago. I was
> looking
> > at provider back then and realized that making gdal calls guarded
> will
> > make it more stable. Frank then submitted patch with guard on every
> > single gdal library call.
> >
> > I will try to explain what is happing now:
> > 1. Map with more than one gdal layer
> > 2. There are two calls in server code execute query and stylize 3.
> One
> > thread will open connection in execute query and went to stylize
> > procedure 4. Another thread will come into execute query and because
> > it could be another layer it will try to open new connection 5.
> > Connection manager will check pool size ( 1 in gdal case ) and
> because
> > provider didn't inc. reference count to connection (not  way to solve,
> > but anyhow provider needs to increment ) it will delete previous
> > connection and create new one 6. Access violation in first thread
> > while stylization because of deleted connection
> >
> >
> > In shorter version things to change:
> > 1. FDO GDAL provider is not increasing reference count on FDO
> > connection when used in other classes like georasterband , which is
> > later used in query (wrong) 2. connection manager is managing
> > connection pool by using fdo connection reference count ( wrong ) 3.
> > it looks like working for other providers, because connection pool
> > size is 200 and other providers are incrementing connection pointer
> in
> > select command for example, but if there would be provider with
> select
> > command with no reference count to connection, it will crash too 4.
> > Server code is making assumption that pool size for single threaded
> is
> > 1 (not correct), we need to make code change to allow for making
> calls
> > to provider single threaded
> >
> > Haris
> >
> > -----Original Message-----
> > From: mapguide-internals-bounces at lists.osgeo.org
> > [mailto:mapguide-internals-bounces at lists.osgeo.org] On Behalf Of
> Bruce
> > Dechant
> > Sent: Friday, March 27, 2009 4:55 PM
> > 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_______________________________________________
> > mapguide-internals mailing list
> > mapguide-internals at lists.osgeo.org
> > http://lists.osgeo.org/mailman/listinfo/mapguide-internals
> > _______________________________________________
> > mapguide-internals mailing list
> > mapguide-internals at lists.osgeo.org
> > http://lists.osgeo.org/mailman/listinfo/mapguide-internals
> > _______________________________________________
> > mapguide-internals mailing list
> > mapguide-internals at lists.osgeo.org
> > http://lists.osgeo.org/mailman/listinfo/mapguide-internals
> > _______________________________________________
> > mapguide-internals mailing list
> > mapguide-internals at lists.osgeo.org
> > http://lists.osgeo.org/mailman/listinfo/mapguide-internals
> > _______________________________________________
> > mapguide-internals mailing list
> > mapguide-internals at lists.osgeo.org
> > http://lists.osgeo.org/mailman/listinfo/mapguide-internals