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
client/db/bolt: ignore notes with bad IDs #2144
Conversation
client/db/bolt/db.go
Outdated
// and thus note ID did not include the TopicID. Ignore it. | ||
db.log.Tracef("Ignoring stored note with bad key: %x != %x \"%s\"", | ||
[]byte(note.Id), trio.k, note.String()) | ||
// trio.b.Put(ackKey, byteTrue) |
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.
We could even move it to a new bucket with the correct key, but these are 1-1.5 year old notifications and I kinda don't wanna bother.
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'm good with auto-acking these.
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.
Acking. My two unacknowledgeable notes logged once, and are now gone after restart. I think this should be good for this fairly rare situation (upgraded with unacked notes, over a year ago).
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.
Also worth noting that these notes would fall off the latest 100 list eventually anyway, but that can take a while for light use.
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.
Auto-acking in last force push.
e48e3cd
to
39d57e1
Compare
The db.Notification serialization and thus ID changed in c994f03#diff-592072666205a423f50bc3061a74c2cda51bfe4689d4044315ff285d29645bc3R832
The ID was used as the bucket key.
Therefore any notes prior to the upgrade will be stored in a bucket with a different key than it's new ID. This isn't a big deal except that acknowledging such notes will fail for eternity.
In draft because I'm considering auto-acking such notes on load.