Skip to content

refactor(sdk): Improve StreamRegistry cache invalidation #2874

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

Merged
merged 5 commits into from
Nov 15, 2024

Conversation

teogeb
Copy link
Contributor

@teogeb teogeb commented Nov 12, 2024

Summary

Now there are two separate cache invalidations in StreamRegistry: one for metadata and another for permissions.

Changes

Previously the invalidateStreamCache() invalidated both caches. Now there are separate invalidatePermissionCaches() and invalidateMetadataCache() methods.

Also changed log level from debug to trace.

Future improvements

  • If there are multiple instances of Stream classes for same streamId, and setMetadata() is called for some of the instances, the metadata is not updated to other instances.
  • Maybe isStreamPublisher and isStreamSubscriber caches could be positive caches, i.e. cache the value only if result is true? We invalidate cache manually when we find out that an operation fails because of missing permission (see MessageFactory#69 and messagePipeline#60). Those are the only places outside of StreamRegistry manual where cache invalidation is done. Therefore we could encapsulate the caching fully into StreamRegistry and also simplify cache key handing by doing this refactoring.
  • The invalidatePermissionCaches should invalidate also the hasPublicSubscribePermission cache? (TODO comment in StreamRegistry)

@github-actions github-actions bot added the sdk label Nov 12, 2024
@teogeb teogeb force-pushed the sdk-invalidatePermissionCaches branch from fe68650 to 9c24cec Compare November 12, 2024 20:47
@teogeb teogeb requested a review from harbu November 12, 2024 21:07
@teogeb teogeb merged commit 27e7116 into main Nov 15, 2024
24 checks passed
@teogeb teogeb deleted the sdk-invalidatePermissionCaches branch November 15, 2024 11:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants