Hi everybody,
recently one of our customers was having some issues with threads blocked inside the default LRUAuthenticationCacheImpl class (default implementation of the AuthenticationCache mechanism in Geoserver).
Searching on the internet we found other users experimenting similar behaviours, so probably the default cache implementation is not very stable under heavy load or particular conditions.
To solve the issue we experimented an alternative implementation using the guava Cache class.
We found that guava Cache already implements all the LRUAuthenticationCacheImpl functionality (expiration on idle and live timeouts, eviction, and so on), so it was a very easy task.
In the future we could also think to leverage advanced guava cache functionality (such as the automatic “loading” of missing entries).
This new implementation is working better for our customer, so far.
The only thing that we did not implement is support for entry by entry expiration times (I found that this is used in GeoServerCompositeFilter using request attributes, but I don’t understand exactly the purpose, can anyone explain me why is that used?).
My proposal is to add the new implementation to Geoserver main, since guava is already a dependency and this would lead to a simpler implementation than the existing one, and one that is based on a well known library.
We could:
add the new implementation but leave the old one as the default (the new one can be enabled adding a bean definition in applicationContext.xml)
add the new implementation and set it as the default, to see if it behaves better than previous one for a larger audience (the old one could be reenabled using the same applicationContext.xml approach)
What do you think? Is anyone interested in this?
Thanks
Mauro
–
==
GeoServer Professional Services from the experts! Visit http://goo.gl/NWWaa2 for more information.
If the guava approach works well then I would think it would make sense to make it the new default, at least on master and perhaps let the old one remain the default on stable.
But perhaps we need to hear from Christian first about the important of this feature as I am not familiar with it.
Hi everybody,
recently one of our customers was having some issues with threads blocked inside the default LRUAuthenticationCacheImpl class (default implementation of the AuthenticationCache mechanism in Geoserver).
Searching on the internet we found other users experimenting similar behaviours, so probably the default cache implementation is not very stable under heavy load or particular conditions.
To solve the issue we experimented an alternative implementation using the guava Cache class.
We found that guava Cache already implements all the LRUAuthenticationCacheImpl functionality (expiration on idle and live timeouts, eviction, and so on), so it was a very easy task.
In the future we could also think to leverage advanced guava cache functionality (such as the automatic “loading” of missing entries).
This new implementation is working better for our customer, so far.
The only thing that we did not implement is support for entry by entry expiration times (I found that this is used in GeoServerCompositeFilter using request attributes, but I don’t understand exactly the purpose, can anyone explain me why is that used?).
My proposal is to add the new implementation to Geoserver main, since guava is already a dependency and this would lead to a simpler implementation than the existing one, and one that is based on a well known library.
We could:
add the new implementation but leave the old one as the default (the new one can be enabled adding a bean definition in applicationContext.xml)
add the new implementation and set it as the default, to see if it behaves better than previous one for a larger audience (the old one could be reenabled using the same applicationContext.xml approach)
What do you think? Is anyone interested in this?
Thanks
Mauro
–
==
GeoServer Professional Services from the experts! Visit http://goo.gl/NWWaa2 for more information.
Open source business process management suite built on Java and Eclipse
Turn processes into business applications with Bonita BPM Community Edition
Quickly connect people, data, and systems into organized workflows
Winner of BOSSIE, CODIE, OW2 and Gartner awards http://p.sf.net/sfu/Bonitasoft
We are getting very close to feature freeze Mauro, if you would like to try the new one we should do so now as the default for the upcoming beta. If it survives I would be comfortable leaving it as the default for the 2.6 release.
Hi everybody,
recently one of our customers was having some issues with threads blocked inside the default LRUAuthenticationCacheImpl class (default implementation of the AuthenticationCache mechanism in Geoserver).
Searching on the internet we found other users experimenting similar behaviours, so probably the default cache implementation is not very stable under heavy load or particular conditions.
To solve the issue we experimented an alternative implementation using the guava Cache class.
We found that guava Cache already implements all the LRUAuthenticationCacheImpl functionality (expiration on idle and live timeouts, eviction, and so on), so it was a very easy task.
In the future we could also think to leverage advanced guava cache functionality (such as the automatic “loading” of missing entries).
This new implementation is working better for our customer, so far.
The only thing that we did not implement is support for entry by entry expiration times (I found that this is used in GeoServerCompositeFilter using request attributes, but I don’t understand exactly the purpose, can anyone explain me why is that used?).
My proposal is to add the new implementation to Geoserver main, since guava is already a dependency and this would lead to a simpler implementation than the existing one, and one that is based on a well known library.
We could:
add the new implementation but leave the old one as the default (the new one can be enabled adding a bean definition in applicationContext.xml)
add the new implementation and set it as the default, to see if it behaves better than previous one for a larger audience (the old one could be reenabled using the same applicationContext.xml approach)
What do you think? Is anyone interested in this?
Thanks
Mauro
–
==
GeoServer Professional Services from the experts! Visit http://goo.gl/NWWaa2 for more information.
Open source business process management suite built on Java and Eclipse
Turn processes into business applications with Bonita BPM Community Edition
Quickly connect people, data, and systems into organized workflows
Winner of BOSSIE, CODIE, OW2 and Gartner awards http://p.sf.net/sfu/Bonitasoft
I put the new implementation as a default (directly changing GeoServerSecurityManager, because adding a bean in applicationContext.xml was breaking some tests depending on a mock implementation.
We are getting very close to feature freeze Mauro, if you would like to try the new one we should do so now as the default for the upcoming beta. If it survives I would be comfortable leaving it as the default for the 2.6 release.
Jody
–
==
GeoServer Professional Services from the experts! Visit http://goo.gl/NWWaa2 for more information.
Hi everybody,
recently one of our customers was having some issues with threads blocked inside the default LRUAuthenticationCacheImpl class (default implementation of the AuthenticationCache mechanism in Geoserver).
Searching on the internet we found other users experimenting similar behaviours, so probably the default cache implementation is not very stable under heavy load or particular conditions.
To solve the issue we experimented an alternative implementation using the guava Cache class.
We found that guava Cache already implements all the LRUAuthenticationCacheImpl functionality (expiration on idle and live timeouts, eviction, and so on), so it was a very easy task.
In the future we could also think to leverage advanced guava cache functionality (such as the automatic “loading” of missing entries).
This new implementation is working better for our customer, so far.
The only thing that we did not implement is support for entry by entry expiration times (I found that this is used in GeoServerCompositeFilter using request attributes, but I don’t understand exactly the purpose, can anyone explain me why is that used?).
My proposal is to add the new implementation to Geoserver main, since guava is already a dependency and this would lead to a simpler implementation than the existing one, and one that is based on a well known library.
We could:
add the new implementation but leave the old one as the default (the new one can be enabled adding a bean definition in applicationContext.xml)
add the new implementation and set it as the default, to see if it behaves better than previous one for a larger audience (the old one could be reenabled using the same applicationContext.xml approach)
What do you think? Is anyone interested in this?
Thanks
Mauro
–
==
GeoServer Professional Services from the experts! Visit http://goo.gl/NWWaa2 for more information.
Open source business process management suite built on Java and Eclipse
Turn processes into business applications with Bonita BPM Community Edition
Quickly connect people, data, and systems into organized workflows
Winner of BOSSIE, CODIE, OW2 and Gartner awards http://p.sf.net/sfu/Bonitasoft
Sorry for the late reply, I am in holidays at the moment.
You asked about the expiration times for individual entries used in GeoServerCompositeFilter. Take a look at GeoServerDigestAuthenticationFilter. Digest authentication uses a secret token derived from the password and avoids sending the password over the network.
This token has its own expiration time and the principal has to authenticate again after it expires, independent of the cache. The token expiration time has a higher precedence as the expiration time used by the cache.
Example:
The cache has expiration times of 50 secs, but the digest authentication filter uses expiration times of 30 secs. In this constellation, a cache entry generated by the digest authentication filter is only valid for 30 secs. Using 50 secs for this entry is not correct. Using the cache entry after 40 seconds (as example) simple violates the digest authentication configuration.
When I introduced this cache I also wanted to use guava but I did not find a proper solution.
To summarize:
If an authentication mechanism has its own expiration times, the cache has return null if this period expires.
The get method in your pull request simply ignores this fact, any idea to add this functionality ?
Btw, I would be happy to switch to guava and getting rid of the race conditions.
I put the new implementation as a default (directly changing GeoServerSecurityManager, because adding a bean in applicationContext.xml was breaking some tests depending on a mock implementation.
Mauro
Open source business process management suite built on Java and Eclipse
Turn processes into business applications with Bonita BPM Community Edition
Quickly connect people, data, and systems into organized workflows
Winner of BOSSIE, CODIE, OW2 and Gartner awards http://p.sf.net/sfu/Bonitasoft
We are getting very close to feature freeze Mauro, if you would like to try the new one we should do so now as the default for the upcoming beta. If it survives I would be comfortable leaving it as the default for the 2.6 release.
Jody
–
==
GeoServer Professional Services from the experts! Visit http://goo.gl/NWWaa2 for more information.
Hi everybody,
recently one of our customers was having some issues with threads blocked inside the default LRUAuthenticationCacheImpl class (default implementation of the AuthenticationCache mechanism in Geoserver).
Searching on the internet we found other users experimenting similar behaviours, so probably the default cache implementation is not very stable under heavy load or particular conditions.
To solve the issue we experimented an alternative implementation using the guava Cache class.
We found that guava Cache already implements all the LRUAuthenticationCacheImpl functionality (expiration on idle and live timeouts, eviction, and so on), so it was a very easy task.
In the future we could also think to leverage advanced guava cache functionality (such as the automatic “loading” of missing entries).
This new implementation is working better for our customer, so far.
The only thing that we did not implement is support for entry by entry expiration times (I found that this is used in GeoServerCompositeFilter using request attributes, but I don’t understand exactly the purpose, can anyone explain me why is that used?).
My proposal is to add the new implementation to Geoserver main, since guava is already a dependency and this would lead to a simpler implementation than the existing one, and one that is based on a well known library.
We could:
add the new implementation but leave the old one as the default (the new one can be enabled adding a bean definition in applicationContext.xml)
add the new implementation and set it as the default, to see if it behaves better than previous one for a larger audience (the old one could be reenabled using the same applicationContext.xml approach)
What do you think? Is anyone interested in this?
Thanks
Mauro
–
==
GeoServer Professional Services from the experts! Visit http://goo.gl/NWWaa2 for more information.
Open source business process management suite built on Java and Eclipse
Turn processes into business applications with Bonita BPM Community Edition
Quickly connect people, data, and systems into organized workflows
Winner of BOSSIE, CODIE, OW2 and Gartner awards http://p.sf.net/sfu/Bonitasoft
Sorry for the late reply, I am in holidays at the moment.
You asked about the expiration times for individual entries used in GeoServerCompositeFilter. Take a look at GeoServerDigestAuthenticationFilter. Digest authentication uses a secret token derived from the password and avoids sending the password over the network.
This token has its own expiration time and the principal has to authenticate again after it expires, independent of the cache. The token expiration time has a higher precedence as the expiration time used by the cache.
Example:
The cache has expiration times of 50 secs, but the digest authentication filter uses expiration times of 30 secs. In this constellation, a cache entry generated by the digest authentication filter is only valid for 30 secs. Using 50 secs for this entry is not correct. Using the cache entry after 40 seconds (as example) simple violates the digest authentication configuration.
I understood the problem. In my opinion we are mixing concerns in this case.
If the AuthenticationCache is a cache, entries can also expire for other reasons than the entry expiration time (for example because the cache is full, so some entries have to be evicted before their expiration time).
I think the digest token expiration should be handled at a higher level, and the cache simplified to only act as a cache (in this case we are using it also as a storage of expiring items).
Can you think of any way to move digest token expiration handling directly into the GeoServerDigestAuthenticationFilter (for example we could do a double check of the entry from the cache to verify if it is still valid, outside of the cache code)? What do you think? Am I missing something?
Mauro
–
==
GeoServer Professional Services from the experts! Visit http://goo.gl/NWWaa2 for more information.
I am fine with your idea, but we should implement a clear solution.
remove GeoServerCompositeFilter.CACHE_KEY_IDLE_SECS and refactor the code
remove GeoServerCompositeFilter.CACHE_KEY_LIVE_SECS and refactor the code
remove method AuthenticationCache.put(String, String, Authentication, Integer, Integer) and refactor the code
add the missing logic to GeoServerDigestAuthentication filter
The idea is to remove the possibility to have individual time out values for a cache entry. This will make it easier to add cache implementations in the future.
Sorry for the late reply, I am in holidays at the moment.
You asked about the expiration times for individual entries used in GeoServerCompositeFilter. Take a look at GeoServerDigestAuthenticationFilter. Digest authentication uses a secret token derived from the password and avoids sending the password over the network.
This token has its own expiration time and the principal has to authenticate again after it expires, independent of the cache. The token expiration time has a higher precedence as the expiration time used by the cache.
Example:
The cache has expiration times of 50 secs, but the digest authentication filter uses expiration times of 30 secs. In this constellation, a cache entry generated by the digest authentication filter is only valid for 30 secs. Using 50 secs for this entry is not correct. Using the cache entry after 40 seconds (as example) simple violates the digest authentication configuration.
I understood the problem. In my opinion we are mixing concerns in this case.
If the AuthenticationCache is a cache, entries can also expire for other reasons than the entry expiration time (for example because the cache is full, so some entries have to be evicted before their expiration time).
I think the digest token expiration should be handled at a higher level, and the cache simplified to only act as a cache (in this case we are using it also as a storage of expiring items).
Can you think of any way to move digest token expiration handling directly into the GeoServerDigestAuthenticationFilter (for example we could do a double check of the entry from the cache to verify if it is still valid, outside of the cache code)? What do you think? Am I missing something?
Mauro
–
==
GeoServer Professional Services from the experts! Visit http://goo.gl/NWWaa2 for more information.
If I understood correctly, the issue is that when the entry timeout is shorter than the cache timeout, the entry is considered valid, because the cache is trusted to return “not expired” values only, while in the opposite case (netry timeout longer than cache timeout) there is no problem, because if the cache returns null for an entry, the digest is checked from the request headers again. Is this correct?
If this is the only problematic use case, we can probably simply add back the check for the entry expiration in the get method:
if (entry.hasExpired(currentTime)) {
return null;
}
···
What do you think? Is this enough for now?
Thanks
Mauro
–
==
GeoServer Professional Services from the experts! Visit http://goo.gl/NWWaa2 for more information.
If I understood correctly, the issue is that when the entry timeout is shorter than the cache timeout, the entry is considered valid, because the cache is trusted to return “not expired” values only, while in the opposite case (netry timeout longer than cache timeout) there is no problem, because if the cache returns null for an entry, the digest is checked from the request headers again. Is this correct?
If this is the only problematic use case, we can probably simply add back the check for the entry expiration in the get method:
if (entry.hasExpired(currentTime)) {
return null;
}
–
DI Christian Mueller MSc (GIS), MSc (IT-Security)
OSS Open Source Solutions GmbH
What do you think? Is this enough for now?
Thanks
Mauro
–
==
GeoServer Professional Services from the experts! Visit http://goo.gl/NWWaa2 for more information.
If I understood correctly, the issue is that when the entry timeout is shorter than the cache timeout, the entry is considered valid, because the cache is trusted to return “not expired” values only, while in the opposite case (netry timeout longer than cache timeout) there is no problem, because if the cache returns null for an entry, the digest is checked from the request headers again. Is this correct?
If this is the only problematic use case, we can probably simply add back the check for the entry expiration in the get method:
if (entry.hasExpired(currentTime)) {
return null;
}
–
DI Christian Mueller MSc (GIS), MSc (IT-Security)
OSS Open Source Solutions GmbH
What do you think? Is this enough for now?
Thanks
Mauro
–
==
GeoServer Professional Services from the experts! Visit http://goo.gl/NWWaa2 for more information.
