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

Add module event for repl-diskless-load swapdb #8153

Merged
merged 2 commits into from Dec 13, 2020

Conversation

oranagra
Copy link
Member

@oranagra oranagra commented Dec 7, 2020

When a replica uses the diskless-load swapdb approach, it backs up the old database,
then attempts to load a new one, and in case of failure, it restores the backup.

this means that modules with global out of keyspace data, must have an option to
subscribe to events and backup/restore/discard their global data too.

@oranagra
Copy link
Member Author

oranagra commented Dec 7, 2020

@MeirShpilraien please let me know what you think.
do you see any reason to add tests for this? seems kinda pointless to me.

@redis/core-team please approve the interfaces or comment.

P.s. personally i don't like the "swapdb" option of repl-diskless-load that much.
it has possibility to trigger the kernel's OOM killer instead of actually restore a backup.
Personally i would have wanted to add an option to "always" use repl-diskless-load (taking the risk that a master failure at this stage will lead to complete data loss), this option doesn't exist today.
But anyway, this "experimental" feature exists (the entire repl-diskless-load, not just the swapdb option), so we probably need to let modules be compatible with it.
if some day we'll trim it, we can deprecate the interface, and never call the events.

src/redismodule.h Outdated Show resolved Hide resolved
@MeirShpilraien
Copy link
Collaborator

@oranagra small comment, other than this LGTM.

@oranagra oranagra added state:major-decision Requires core team consensus state:needs-test-code the PR is missing test code labels Dec 8, 2020
@oranagra oranagra added this to In progress in 6.2 Dec 8, 2020
src/redismodule.h Outdated Show resolved Hide resolved
@oranagra oranagra requested a review from yossigo December 8, 2020 10:38
@oranagra oranagra added release-notes indication that this issue needs to be mentioned in the release notes state:to-be-merged The PR should be merged soon, even if not yet ready, this is used so that it won't be forgotten and removed state:needs-test-code the PR is missing test code labels Dec 8, 2020
@yossigo yossigo added the approval-needed Waiting for core team approval to be merged label Dec 10, 2020
@oranagra oranagra merged commit ab60dcf into redis:unstable Dec 13, 2020
@itamarhaber itamarhaber moved this from In progress to Done in 6.2 Dec 13, 2020
@oranagra oranagra mentioned this pull request Jan 13, 2021
JackieXie168 pushed a commit to JackieXie168/redis that referenced this pull request Mar 2, 2021
When a replica uses the diskless-load swapdb approach, it backs up the old database,
then attempts to load a new one, and in case of failure, it restores the backup.

this means that modules with global out of keyspace data, must have an option to
subscribe to events and backup/restore/discard their global data too.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approval-needed Waiting for core team approval to be merged release-notes indication that this issue needs to be mentioned in the release notes state:major-decision Requires core team consensus state:to-be-merged The PR should be merged soon, even if not yet ready, this is used so that it won't be forgotten
Projects
No open projects
6.2
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants