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

Kenneth Skovhede, GEOGRAF A/S ks at geograf.dk
Thu Apr 2 04:38:53 EDT 2009


After testing the Dll's provided by Haris/Jason, I finally have a stable 
MapGuide solution, so a BIG thank you for that!

Next stop is faster rasters. Would it be possible to produce compiled 
dll's for testing the solution that Traian proposes,
and post them somewhere? I can host them if that is the blocker.

Regards, Kenneth Skovhede, GEOGRAF A/S



Traian Stanev skrev:
> I attached a new patch for the provider to the ticket 462, which speeds up GDAL FDO connection opening by 95%, in addition to the refcounting fix, and the global mutex removal. The global mutex is still needed in two places where I did not disable it. 
>
> This change allows you to completely disable MG connection pooling for the GDAL provider (like for SHP and SDF) and still have very good raster speed, since opening a connection is fast now. Also, the global raster mutex can be removed from the MG code.
>
> I will continue testing with this configuration, but so far it looks stable and fast. Obviously it is impossible to prove that a problem no longer exists, when threads are involved, but I will throw some more difficult tests at it -- so far I have only tested with a TIFF-based layer only. The TIFF is 450 megs or so.
>
> 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:29 AM
>> To: MapGuide Internals Mail List
>> Cc: FDO Internals Mail List
>> Subject: RE: [mapguide-internals] RE: PATCH: Raster stability fixes,
>> ticket #462
>>
>> I attached the patch for the GDAL provider to ticket 462.
>>
>> 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
>>>>         
>> _______________________________________________
>> 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
>   


More information about the mapguide-internals mailing list