Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[GEOS-9219] Fix gwc tile layer load performance regression at startup #3451

Merged

Conversation

groldan
Copy link
Member

@groldan groldan commented Apr 28, 2019

DefaultTileLayerCatalog listens to Resouce events under
the data directory's gwc-layers folder resource in order to
notify its local instance listeners of tile layer configuration
changes even when on a shared directory by several service
instances.

To do so, it was using both a "folder" listener on gwc-layers,
and one listener per tile layer info resource (i.e. gwc-layers/*.xml).

This resulted in a severe startup hit when there are several
tile layers.

This patch uses a single listener on the gwc-cache folder resource
to handle all the scenarios, and hence removes the need to set up
and cache one resource listener per tile layer.

@aaime
Copy link
Member

aaime commented Apr 28, 2019

Yep, this one really got me mad but the original author cannot be relied to fix on his mess and the reviewer did not catch the problem..... Thank you for fixing it!

@groldan groldan force-pushed the perf/gwc_startup_tilelayer_load/master branch from 32b3034 to 1af1e51 Compare April 28, 2019 06:44
@groldan
Copy link
Member Author

groldan commented Apr 28, 2019

oh, cool then, glad to be back in the game :P

DefaultTileLayerCatalog listens to Resouce events under
the data directory's gwc-layers folder resource in order to
notify its local instance listeners of tile layer configuration
changes even when on a shared directory by several service
instances.

To do so, it was using both a "folder" listener on gwc-layers,
and one listener per tile layer info resource (i.e. gwc-layers/*.xml).

This resulted in a severe startup hit when there are several
tile layers.

This patch uses a single listener on the gwc-cache folder resource
to handle all the scenarios, and hence removes the need to set up
and cache one resource listener per tile layer.
@groldan groldan force-pushed the perf/gwc_startup_tilelayer_load/master branch from 1af1e51 to dd54983 Compare April 28, 2019 06:46
Copy link
Contributor

@NielsCharlier NielsCharlier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aha, I have been mostly working with jdbc configurations which is why I was not aware of causing this performance issue in a file-based setup. I have read through the code changes and all seems really clear. The functionality is preserved, as also proven by the passing unit test, but the issue is fixed. Thanks @groldan

@groldan
Copy link
Member Author

groldan commented Apr 29, 2019

ok, merging then, thanks for the review @NielsCharlier

@groldan
Copy link
Member Author

groldan commented Apr 29, 2019

Indeed, I didn't need to add any extra test case cause it was well covered already so I based the refactoring on those

@groldan groldan merged commit 5c11618 into geoserver:master Apr 29, 2019
@aaime
Copy link
Member

aaime commented Apr 30, 2019

@groldan are you going to backport this one? I believe 2.15.x is affected, not sure about 2.14.x

@groldan
Copy link
Member Author

groldan commented May 16, 2019

@aaime sorry I couldn't come back to this before. Created a ticket for the backport: https://osgeo-org.atlassian.net/browse/GEOS-9219

@groldan groldan changed the title Fix gwc tile layer load performance regression at startup [GEOS-9219] Fix gwc tile layer load performance regression at startup May 31, 2019
@groldan groldan deleted the perf/gwc_startup_tilelayer_load/master branch May 31, 2019 12:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants