-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Improve the performance of snapshot installs by using rename #9247
Conversation
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.
Changes look like they follow the RFC 👍
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.
Reading through all of this was very educational. Thanks for the clarifications on my questions.
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.
Took another look at it, and got nothing else to add, 👍 !
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.
Performed another review, looks great @briankassouf. Left some minor questions & comments.
} | ||
|
||
atomic.StoreUint64(f.latestIndex, metadata.Index) | ||
atomic.StoreUint64(f.latestTerm, metadata.Term) | ||
f.latestConfig.Store(protoConfig) | ||
f.latestConfig.Store(raftConfigurationToProtoConfiguration(metadata.ConfigurationIndex, metadata.Configuration)) |
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.
nit: raftConfigToProtoConfig
slightly shorter nomenclature
Co-authored-by: Alexander Bezobchuk <[email protected]>
Co-authored-by: Alexander Bezobchuk <[email protected]>
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.
LGTM 🎉
* master: Entity and alias counts (#9262) Token gauge metrics implementation. (#9239) mfa: fix import path on test file (#9303) doc: update vault helm enterprise image examples (#9299) raft: add support for using backend for ha_storage (#9193) Document new and previously undocumented telemetry metrics: (#9283) Improve the performance of snapshot installs by using rename (#9247) docs: add additional info around transform for tweak and template type (#9203) Update CHANGELOG.md CL++: Add go version to server message output
* initial work on improving snapshot performance * Work on snapshots * rename a few functions * Cleanup the snapshot file * vendor the safeio library * Add a test * Add more tests * Some review comments * Fix comment * Update physical/raft/snapshot.go Co-authored-by: Alexander Bezobchuk <[email protected]> * Update physical/raft/snapshot.go Co-authored-by: Alexander Bezobchuk <[email protected]> * Review feedback Co-authored-by: Alexander Bezobchuk <[email protected]>
This PR moves the snapshot store away from using flat files to using newly written boltDB files for snapshot data. This allows us to do an atomic rename when that snapshot needs to be installed to the FSM instead of doing another copy of the data.