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

initialize client woff with server.master_repl_offset #6393

Open
wants to merge 1 commit into
base: unstable
Choose a base branch
from

Conversation

soloestoy
Copy link
Collaborator

@soloestoy soloestoy commented Sep 17, 2019

Initialize client woff with server.master_repl_offset, in case 0 write offset.

BTW, now woff is only used in wait command, and it seems that server.master_repl_offset is enough here, will it be useful in the future?

@antirez
Copy link
Contributor

antirez commented Sep 30, 2019

@soloestoy thank you very much, please could you provide a bit more background about this change, especially when you say "0 write offset"? If the connection was just created, I assume we are always happy to return ASAP from WAIT.

About the other question about woff, yes it is only used by WAIT, but we need to remember what was the value of the master replication offset at the moment we executed the last write command in that client (even if currently we store there the offset of any command, actually, even if it was a read-only one).

So for instance, we execute a command when the offset was 100

We wait some time, then call WAIT in that connection.

Now the acknowledged offset is 110, so we can immediately continue.

Another client however executed the command when the offset was 130, so WAIT should block instead.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

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.

None yet

3 participants