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

MemoryBlobStore limits the delegate store to a single thread #1275

Open
brettniven opened this issue May 30, 2024 · 1 comment
Open

MemoryBlobStore limits the delegate store to a single thread #1275

brettniven opened this issue May 30, 2024 · 1 comment

Comments

@brettniven
Copy link

The MemoryBlobStore limits the delegate store to a single thread, which can degrade performance. It effectively forces all 'put', 'get' and other operations to be bound to a single queue.

I've written a sample test and a one-liner change to highlight the problem. This is here: brettniven@b7f08e2. By increasing the executor threads, it obviously has much better performance.

I can assist on a fix - I'm just not sure on the best approach as I can't figure out why it would have the '1 thread' limit in the first place. Some links to explain what is happening:

... but I can't work out a reason for it needing to be limited to 1 thread. Each delegate store should have it's own locking code, where applicable.

For reference, our setup is:

  • GWC embedded in GeoServer, 1.24.x, using Kartoza docker image
  • We're hosting in Azure, using an Azure File Share (which unfortunately for us, is 'slow' for a few potential reasons, which exacerbates this issue - we're in the process of trialling the Azure Blob Store plugin)
  • We're using a MemoryBlobStore as a 2-layer cache, fronting a FileBlobStore. i.e. innerCachingEnabled=true.
  • We investigated performance and found that we get substantially better performance when we turn the MemoryBlobStore off, which is a very simple workaround for this issue. However... we believe we've also found a thread deadlock issue in the FileBlobStore code (which I'll raise in another issue when time permits)
@aaime
Copy link
Member

aaime commented May 31, 2024

@brettniven I don't know either (some research would be needed) but:

  • I also have the impression the code is over-using locks and concurrency limitations
  • The class is 10 years old
  • The original author was well-meaning, but also inexperienced. Maybe he wanted to avoid potential double-computation of the same tile? But that's the job of other parts of GWC.

Generally speaking, AFAIK a BlobStore implementation should be thread-safe, and a wrapper should not need to care for synchronizing/serializing access to it. @smithkm can you provide any more information/insight?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants