Skip to content

Task/dogpile orm event invalidation - #3033

Draft
whabanks wants to merge 4 commits into
mainfrom
task/dogpile-orm-event-invalidation-2
Draft

Task/dogpile orm event invalidation#3033
whabanks wants to merge 4 commits into
mainfrom
task/dogpile-orm-event-invalidation-2

Conversation

@whabanks

Copy link
Copy Markdown
Contributor

Summary | Résumé

This PR experimentally implements an ORM-event driven invalidation strategy for Service entities. Rather than calling an explicit invalidate_service_cache_keys() function on every path that mutates service and service related data; we instead listen to after_flush, after_commit, and after_rollback sqlalchemy session events, weaving cache invalidation calls into that process. (similar in nature to aspect oriented programming)

This nets us:

  • Centralized invalidation logic
  • Reduced code complexity, and improve maintainability particularly on larger models where multiple mutating code paths are involved
  • Tighter control over when we invalidate i.e. if a transaction is rolled back there's no need to invalidate existing cache
  • Since we're handling this logic much closer to the actual time the DB is updated, we reduce the chances of stale cache reads after a successful write.

How it works

TODO

Related Issues | Cartes liées

Test instructions | Instructions pour tester la modification

  1. Access a service, note the cache key is created
  2. Navigate to service settings and turn off email sending (or change any setting) -> note the cache key was updated
  3. Add or remove a team member from a service -> note the cache key reflects this change

Release Instructions | Instructions pour le déploiement

None.

Reviewer checklist | Liste de vérification du réviseur

  • This PR does not break existing functionality.
  • This PR does not violate GCNotify's privacy policies.
  • This PR does not raise new security concerns. Refer to our GC Notify Risk Register document on our Google drive.
  • This PR does not significantly alter performance.
  • Additional required documentation resulting of these changes is covered (such as the README, setup instructions, a related ADR or the technical documentation).

⚠ If boxes cannot be checked off before merging the PR, they should be moved to the "Release Instructions" section with appropriate steps required to verify before release. For example, changes to celery code may require tests on staging to verify that performance has not been affected.

Implement a basic ORM event driven invalidation strategy for service
keys. Listeners are attached to the sqlalchemy session `after_flush`,
`after_commit` and `after_rollback` events which will invalidate service
cache keys under the following conditions
- `service` entity is updated/deleted
- `service_permissions` are updated
- `service_user` list is added to, updated, or removed from

(cherry picked from commit 2744298)
(cherry picked from commit dc74a95)
- Rename service_cache_events -> cache_events
- Add cache_invalidation_registry to centralize and define entity ->
  cache key relationships
- Entity id collection is now generic and only collects ids of entities
  that exist in the cache_invalidation_registry
- _invalidate_cache_after_commit now references the registry to
  determine which cache key groups should be invalidated
- Register cache_orm_events on application startup
Comment thread app/cache/cache_events.py
for namespace, entity_id in invalidations:
try:
invalidate_group_keys(namespace, entity_id)
except Exception:
Comment thread app/cache/cache_events.py Dismissed
@whabanks

Copy link
Copy Markdown
Contributor Author

Original convo from here
From @jimleroyer

I am concerned that this approach (and the other PR too but maybe slightly less) might be difficult to scale, i.e. every time a developer wants certain cache to be added, they would have to make sure proper specific events fire, and that these are for service IDs, is that right? And when we want to add a cache specific to users, and templates, and keys, and etc.. we'd have also to add specific glue code for these? The cache isn't generic enough to handle these automatically?

Response

every time a developer wants certain cache to be added, they would have to make sure proper specific events fire, and that these are for service IDs, is that right?

With the current implementation, yes this would have to be done to accommodate more than just the service entity.

And when we want to add a cache specific to users, and templates, and keys, and etc.. we'd have also to add specific glue code for these? The cache isn't generic enough to handle these automatically?

The baseline dogpile library isn't smart enough to graph and evaluate the potential entity relationships and invalidate corresponding keys on it's own. One possibility to facilitate a generic implementation would be to leverage a registry that centrally stores (and therefore documents) entity <-> cache key dependency metadata.

Then developers only need to update the registry when new relationships are added in the DB, or additional DAO methods are marked via annotation to leverage dogpile.

Something like this:

image

The two new things here would be the registry and dependency resolver.

  • The session events are already in place
  • The change collector would be adjusted to generically map collected ids to an entity type
  • The cache invalidator would be changed to map entities to cache keys base on the registry and execute the invalidation

I've taken the liberty to implement the beginnings of such an approach and it is now included in this PR.

@whabanks whabanks changed the title Task/dogpile orm event invalidation 2 Task/dogpile orm event invalidation Aug 31, 2026
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

Successfully merging this pull request may close these issues.

2 participants