fix: remember keys_changed_at when reallocating after node replacement. #173
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jrconlin
approved these changes
Feb 10, 2020
|
Thanks @jrconlin! Weirdly, I don't seem to be able to merge this due to commit-signing, but AFAICT my commit is in fact signed. Not sure what's going on there... |
|
Yeah, I was wondering about that too, and yes, it appears verified to me as well. Looks like it's a problem for other PRs, too, so I have no idea aside from "weird github issue". Sadly, I don't have admin on this package so I can't force it. |
|
Submitted a help request with github to see what might be wrong here. |
If a user's current storage node is decomissioned, they may end up without an active `users` record in the db, and there is code to handle this case by lazily recreating an active record on demand. Previously, this code would only copy `generation` and `client_state` fields, essentially forgotten any previously-seen value for the new `keys_changed_at` field. This commit fixes things so that value is also remembered. I do not expect this bug to have caused any issues in practice, since we already have a bunch of code to handle users for which we have not yet seen a value of `keys_changed_at`. But we should remember it when we can because it helps the server enforce that clients are behaving correctly.
rfk
force-pushed
the
remember-keyschangedat-after-node-replacement
branch
from
a8f29aa
to
ee6c065
Compare
|
I did a |
|
Hooray, that seemed to work \o/ |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If a user's current storage node is decomissioned, they may end up
without an active
usersrecord in the db, and there is code tohandle this case by lazily recreating an active record on demand.
Previously, this code would only copy
generationandclient_statefields, essentially forgotten any previously-seen value for the new
keys_changed_atfield. This commit fixes things so that value isalso remembered.
I do not expect this bug to have caused any issues in practice, since
we already have a bunch of code to handle users for which we have
not yet seen a value of
keys_changed_at. But we should remember itwhen we can because it helps the server enforce that clients are
behaving correctly.