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

Improve commit log handling for expired (expiresAt has passed) records #1660

Closed
gkc opened this issue Nov 14, 2023 · 3 comments · Fixed by #1713 or atsign-foundation/at_client_sdk#1187
Assignees
Labels
enhancement New feature or request

Comments

@gkc
Copy link
Contributor

gkc commented Nov 14, 2023

Is your feature request related to a problem? Please describe.

Summary

  • We are managing deletions of expired records on the client side via server creating delete entries in the commit log, and the sync process. This is unnecessary as clients also have the ability to detect expired records and delete them.
  • As a result, the commit log grows every time a record expires. So if you create a million records over a period of a week, all of which expire within an hour, you will have a commit log with a million entries in it, which will stay there forever

Detail

  • @alice sends notification to @bob with ttr:-1 (cache indefinitely on bob's atServer) and ttl:3600000 (1 hour)
    • or does a put with ttr:-1 and ttl:36000000
  • when bob's (and alice's, in the case of a put rather than a notify) atServers finds the expired records during a sweep, currently a record is added to the commit log
  • as a result, the commit log grows every time a record with a new key is shared / notified
  • this isn't necessary, since client-side storage can detect locally stored (e.g. by sync) expired records and delete them

Describe the solution you'd like

  • client-side, when a record is fetched, if it has expired then it should be deleted (and not added into local uncommitted entries)
  • client-side, there should be regular background sweeps for expired records - or, at a minimum, at startup time (i.e. creation of LocalSecondary object)
  • server-side, when a record is fetched, if it has expired then it should be deleted, and the existing commit entry should be deleted, and no new commit entry should be added

Describe alternatives you've considered

No response

Additional context

No response

@gkc gkc added enhancement New feature or request bug Something isn't working and removed enhancement New feature or request labels Nov 14, 2023
@ksanty ksanty removed the bug Something isn't working label Nov 14, 2023
@srieteja
Copy link
Contributor

Could not work on this previous sprint. Carrying it forward to PR76.

@srieteja
Copy link
Contributor

srieteja commented Jan 8, 2024

Changes have been implemented through multiple PRs which are linked to this ticket. We have decided that this feature will remain dormant on the server-side to preserve backwards compatibility. However, the client-side changes will be merged and published. The dormant feature will be enabled at some time in the future when most of the clients use a version of AtClient that contains the present changes in atsign-foundation/at_client_sdk#1187. The enabling of the dormant feature will be tracked through #1728. Carrying it to the next sprint to get the PRs merged systematically.

@srieteja
Copy link
Contributor

Testing each of the cases (e. g. Server feature enabled with old client, vice versa). Most of the cases are done, finishing on the last few. Moving it to PR79 for testing and review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
3 participants