double check for cached coverage reader on ResourcePool's synchronized block
----------------------------------------------------------------------------
Key: GEOS-3677
URL: http://jira.codehaus.org/browse/GEOS-3677
Project: GeoServer
Issue Type: Improvement
Components: Configuration
Affects Versions: 2.0.0
Reporter: Gabriel Roldán
Assignee: Gabriel Roldán
Fix For: 2.0.1, 2.1.x
From email:
{panel}
Thanks for the quick review Andrea,
I will commit with a related jira then just in case we want to roll it
back later.
Cheers,
Gabriel
Andrea Aime wrote:
> Gabriel Roldan ha scritto:
>> Justin, all
>>
>> during my last weeks of arcsde gce stress testing, I found a potential
>> concurrency penalty in ResourcePool.getGridCoverageReader. If hit with
>> various concurrent requests at it may instantiate lots of readers to
>> ultimately leave a single one cached.
>> The following patch applies the same trick than for
>> ResourcePool.getDataStore, please let me know if it seems appropriate
>> to you: <http://pastebin.com/m32ebb87b>
>>
>> It just double checks for the cached reader after acquiring the lock
>> on the cache.>
> The patch looks good to me.
> Maybe remove those capital letters comments
>
> (I noticed they are also in the original code, not sure why...)
>
> Cheers
> Andrea
{panel}
Patch is:
{code}
Index: src/main/java/org/geoserver/catalog/ResourcePool.java
--- src/main/java/org/geoserver/catalog/ResourcePool.java (revision 13649)
+++ src/main/java/org/geoserver/catalog/ResourcePool.java (working copy)
-682,16 +682,23 @@
synchronized ( hints != null ? hintCoverageReaderCache : coverageReaderCache ) {
- /////////////////////////////////////////////////////////
- //
- // Getting coverage reader using the format and the real path.
- //
- // /////////////////////////////////////////////////////////
- final File obj = GeoserverDataDirectory.findDataFile(info.getURL());
-
- // XXX CACHING READERS HERE
- reader = (info.getFormat()).getReader(obj,hints);
- (hints != null ? hintCoverageReaderCache : coverageReaderCache ).put(info, reader);
+ if (hints != null) {
+ reader = (GridCoverageReader) hintCoverageReaderCache.get(info);
+ } else {
+ reader = (GridCoverageReader) coverageReaderCache.get(info);
+ }
+ if (reader == null) {
+ /////////////////////////////////////////////////////////
+ //
+ // Getting coverage reader using the format and the real path.
+ //
+ // /////////////////////////////////////////////////////////
+ final File obj = GeoserverDataDirectory.findDataFile(info.getURL());
+
+ // XXX CACHING READERS HERE
+ reader = (info.getFormat()).getReader(obj,hints);
+ (hints != null ? hintCoverageReaderCache : coverageReaderCache ).put(info, reader);
+ }
}
return reader;
{code}
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://jira.codehaus.org/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira