-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
reset watch's lastIndex on error #2621
Conversation
When a -dev agent is restarted it'll have a clean state, including a reset index. A watch() will reconnect after a restart, but it won't notice that the index counter has reset and it will keep waiting until we reached the old index again, which is wrong. Resetting the index will prevent that and makes watch work for -dev agents.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @alicebob thanks for opening a PR - I think we need a tiny logic update to catch all the cases.
@@ -57,6 +57,7 @@ OUTER: | |||
if err != nil { | |||
// Perform an exponential backoff | |||
failures++ | |||
p.lastIndex = 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this'll catch all cases since you won't always get an error. It's probably better to add something down on line 88 like this:
if p.lastIndex < oldIndex {
p.lastIndex = 0
}
This'll reset the blocking query whenever the index slips backwards.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your reply, @slackpad.
The original fix I can easily reproduce in a test setup (watch() something and restart the Consul server in -dev mode), but a index which goes back (but doesn't drop the connection) I can't test. So I've added the lines you suggested, but I can't test it.
I also changed my original fix to reset the index to 0, not to 1.
(if this is fine I'll squash all commits)
Thanks! |
Watch()ing a agent in
-dev
mode misses changes when the agent restarts.When a -dev agent is restarted it'll have a clean state, including a
reset index. A watch() will reconnect after a restart of the agent, but it won't
notice that the index counter has reset and it will keep waiting until
the server reached the old index again, which is wrong. Resetting the index
prevents that and makes watch() work for -dev agents over restarts.