[Fixes #14097] Label thesaurus is not reloaded on thesaurus changes#14098
[Fixes #14097] Label thesaurus is not reloaded on thesaurus changes#14098
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates GeoNode’s i18n caching logic so that changes to the labels-i18n Thesaurus reliably invalidate previously cached localized label data (and related i18n-derived caches) instead of continuing to serve stale values.
Changes:
- When the Thesaurus
datechanges, invalidate cached i18n data by clearing the in-memory cache rather than only invalidating the current(lang, data_key)lookup.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if cached_entry and cached_entry.date != thesaurus_date: | ||
| logger.info(f"Cache for {lang}:{data_key} needs to be recreated") | ||
| logger.info(f"Cache for {lang}:{data_key} dirty, clearing all caches") | ||
| self.lang_cache.clear() | ||
| return thesaurus_date, None |
There was a problem hiding this comment.
get_entry() now clears self.lang_cache under _lock, but set() and clear() mutate self.lang_cache without acquiring the same lock. This creates a real race where one thread can be in setdefault()/write while another thread clears the dict, leading to lost updates and inconsistent cache state. Consider guarding all lang_cache mutations (set(), clear(), force_check()) with _lock (or switching to a thread-safe strategy) so invalidation is atomic across readers/writers.
There was a problem hiding this comment.
@copilot apply changes based on this feedback
There was a problem hiding this comment.
Applied in commit a2745f9. All lang_cache mutations (set(), clear(), force_check()) are now guarded with _lock. The DB query in set() is intentionally kept outside the lock to avoid holding it during I/O, with only the actual cache mutations inside the lock context.
| if cached_entry and cached_entry.date != thesaurus_date: | ||
| logger.info(f"Cache for {lang}:{data_key} needs to be recreated") | ||
| logger.info(f"Cache for {lang}:{data_key} dirty, clearing all caches") | ||
| self.lang_cache.clear() | ||
| return thesaurus_date, None |
There was a problem hiding this comment.
The new behavior invalidates the entire i18n cache when the Thesaurus date changes. Given this is a regression fix (and i18nCache is also used for other cached data like metadata schema), it would be good to add a unit test that reproduces the stale-cache scenario and asserts that previously cached values (e.g., schema and/or labels for multiple languages) are rebuilt after a Thesaurus update.
There was a problem hiding this comment.
@copilot apply changes based on this feedback
There was a problem hiding this comment.
Added in commit d6add25. The new test test_stale_cache_invalidated_on_thesaurus_update in geonode/metadata/tests/test_i18n.py:
- Warms up the cache with labels for both
enandit - Updates the label values in the DB and bumps the Thesaurus
date(simulating a real thesaurus update) - Forces a freshness check (bypassing
CHECK_INTERVAL) - Asserts that both language caches are rebuilt with the new values, not the stale ones
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Agent-Logs-Url: https://github.com/GeoNode/geonode/sessions/26a81ebe-aef6-4850-b356-885f6bdce0dc Co-authored-by: etj <717359+etj@users.noreply.github.com>
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @Copilot on file. In order for us to review and merge your code, please contact the project maintainers to get yourself added. |
Agent-Logs-Url: https://github.com/GeoNode/geonode/sessions/cbf02dc8-5fb7-402a-9ab5-9f58f1a5378e Co-authored-by: etj <717359+etj@users.noreply.github.com>
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @Copilot on file. In order for us to review and merge your code, please contact the project maintainers to get yourself added. |
Updates GeoNode's i18n caching logic so that changes to the
labels-i18nThesaurus reliably invalidate previously cached localized label data (and related i18n-derived caches) instead of continuing to serve stale values.Changes Made
datechanges, the entire in-memorylang_cacheis cleared (all languages) rather than only invalidating the current(lang, data_key)lookup, ensuring no stale data is served for any language.lang_cachemutations (set(),clear(),force_check()) are now guarded with_lock, making cache invalidation atomic across concurrent readers and writers. The DB query inset()is kept outside the lock to avoid holding it during I/O.test_stale_cache_invalidated_on_thesaurus_updateingeonode/metadata/tests/test_i18n.pyto reproduce the stale-cache scenario — it warms up the cache for multiple languages (enandit), bumps the Thesaurusdate, and asserts that all language caches are rebuilt with fresh values.