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

Diskless-load emptyDb-related fixes #6822

Merged
merged 1 commit into from
Feb 6, 2020

Conversation

guybe7
Copy link
Collaborator

@guybe7 guybe7 commented Feb 3, 2020

  1. Call emptyDb even in case of diskless-load: We want modules
    to get the same FLUSHDB event as disk-based replication.
  2. Do not fire any module events when flushing the backups array.
  3. Delete redundant call to signalFlushedDb (Called from emptyDb).

@antirez
Copy link
Contributor

antirez commented Feb 6, 2020

Hi @guybe7, a few notes:

  1. Please could you document the new flag in the top comment?
  2. emptyDb() does not call signalFlushedDb() AFAIK.

1. Call emptyDb even in case of diskless-load: We want modules
   to get the same FLUSHDB event as disk-based replication.
2. Do not fire any module events when flushing the backups array.
3. Delete redundant call to signalFlushedDb (Called from emptyDb).
@guybe7 guybe7 force-pushed the diskless_load_module_hook_fix branch from 6e4416b to 92dc5e1 Compare February 6, 2020 11:18
@guybe7
Copy link
Collaborator Author

guybe7 commented Feb 6, 2020

  1. Done (pushed -f)
  2. emptyDb calls emptyDbGeneric which calls signalFlushedDb

@antirez
Copy link
Contributor

antirez commented Feb 6, 2020

@guybe7 About "2", I get that, I mean right now in my source tree I can't see emptyDb() already calling signalFlushedDb(). But now I see that I mis-understood your comment "Delete redundant call to signalFlushedDb (Called from emptyDb).", You mean that it's already called from emptyDb() so you removed it from the replication code. Perfect, merging, thanks for clarifying.

@antirez antirez merged commit 9c00bdd into redis:unstable Feb 6, 2020
JackieXie168 pushed a commit to JackieXie168/redis that referenced this pull request Feb 20, 2020
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