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

feat: Changes related to notify ephemeral #1544

Merged
merged 6 commits into from
Sep 12, 2023
Merged

feat: Changes related to notify ephemeral #1544

merged 6 commits into from
Sep 12, 2023

Conversation

sitaram-kalluri
Copy link
Member

@sitaram-kalluri sitaram-kalluri commented Sep 8, 2023

- What I did

  • Before, when TTR was not set, received notifications only contained the key and not the associated value. When TTR is enabled, it includes the value and also stores the key in the receiver's secondary cache. There was no way to notify both the key and the value without caching the key in the receiver's secondary cache.

  • The revised implementation aims to notify both the key and its value without generating a cached copy in the receiver's secondary storage.

- How I did it

  • In the resource_manager.dart-> "prepareNotifyCommandBody" method, the notify command is prepared to send notification to the receiver's secondary. The value is included to the commandBody only IF the TTR is NOT NULL.
  • Changed the above implementation to add the "value" to the "commandBody" if the "value" is not null. and if TTR is set, include only TTR to the commandBody and not the "value" (Since we are already including value to the notifyCommandBody, do not include value again to prevent duplication. From behaviour aspects it works as it was before). With this change, when TTR is not set, we can notify key and its associated value without creating a cached copy in the receiver's secondary. Also preserve the creation of cached keys when TTR is set.

- How to verify it

  • Updated the end-to-end tests. Added a version check in the e2e tests to skip the tests running in the canary and prod servers

Tested the changes manually and value is received without a cached copy getting created on the receiver's end.

@sitaram@notify:@murali:key-1.wavi@sitaram:value1
data:61ff3dd0-ad6b-43eb-b435-3ed8aee244d5

@murali@monitor
notification: {"id":"61ff3dd0-ad6b-43eb-b435-3ed8aee244d5","from":"@sitaram","to":"@murali","key":"@murali:key-1.wavi@sitaram","value":"value1","operation":"update","epochMillis":1694137061909,"messageType":"MessageType.key","isEncrypted":true,"metadata":{"encKeyName":null,"encAlgo":null,"ivNonce":null,"skeEncKeyName":null,"skeEncAlgo":null}}

- Description for the changelog

Copy link
Contributor

@gkc gkc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@gkc gkc merged commit d2a1756 into trunk Sep 12, 2023
@gkc gkc deleted the notify_ephemeral branch September 12, 2023 14:16
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