Fix purgeRealms in LocalPolarisMetaStoreManagerFactory does not clear entityCacheMap

[ayushtkn]

Fixes: #4801

Checklist

• 🛡️ Don't disclose security issues! (contact security@apache.org)
• 🔗 Clearly explained why the changes are needed, or linked related issues: Fixes #
• 🧪 Added/updated tests with good coverage, or manually tested (and explained how)
• 💡 Added comments for complex logic
• 🧾 Updated CHANGELOG.md (if needed)
• 📚 Updated documentation in site/content/in-dev/unreleased (if needed)

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR ensures purging realms also clears the per-realm entity cache so subsequent realm bootstraps don’t reuse stale cached state.

Changes:

• Synchronize purgeRealms and remove the realm entry from entityCacheMap during purge.
• Add a JUnit test validating that purgeRealms causes a fresh EntityCache instance to be created after re-bootstrap.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
polaris-core/src/main/java/org/apache/polaris/core/persistence/LocalPolarisMetaStoreManagerFactory.java	Clears entityCacheMap on realm purge and synchronizes purgeRealms to keep map mutations thread-safe.
polaris-core/src/test/java/org/apache/polaris/core/persistence/LocalPolarisMetaStoreManagerFactoryTest.java	Adds regression test asserting cache instances aren’t reused after purge + re-bootstrap.

[nandorKollar]

LGTM

Since the key of each map in this class is the realm ID, I'm wondering whether we could use a single ConcurrentHashMap instead of three separate maps. This map could associate each realm ID with a record containing (PolarisMetaStoreManager, EntityCache, Supplier<TransactionalPersistence>). I think this would allow us to avoid the synchronized methods, wouldn't it?

[dimas-b]

@nandorKollar : just FYI: this class is used in Apache Polaris code only on "in memory" persistence call paths.

So this fix in OSS is probably not very critical... especially considering https://lists.apache.org/thread/nzoljc1ohnsq4f5o28dh4opqkqw3p09h

Do you need this fix for a downstream build?

[nandorKollar]

@nandorKollar : just FYI: this class is used in Apache Polaris code only on "in memory" persistence call paths.

So this fix in OSS is probably not very critical... especially considering https://lists.apache.org/thread/nzoljc1ohnsq4f5o28dh4opqkqw3p09h

Do you need this fix for a downstream build?

You were quicker than me. :) I wanted to mention the same after I submitted my comment, probably we should hold off on merging this, as as far as understood, the plan is to deprecate and remove this. No, afaik we don't need this fix downstream (@ayushtkn correct me if I'm wrong).

[dimas-b]

@dennishuo @collado-mike : WDYT about this fix?

[flyrain]

+1 Thanks @ayushtkn !