-
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
Use our fork of bbolt to improve freelist performance #24010
Conversation
I should note that this is also an upgrade from 1.3.7 to 1.3.8, it's not only a backport of the freelist fix. |
CI Results: |
…s the official bbolt lib.
Build Results: |
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!
// is, assuming it can be parsed as an int. Bolt itself sets this to 0 by default, | ||
// so if users are wanting to turn this off, they can also set it to 0. Setting it | ||
// to a negative value is the same as not setting it at all. | ||
if os.Getenv("VAULT_RAFT_INITIAL_MMAP_SIZE") == "" { |
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.
super nit: is there a reason that we are not following the same pattern as the above two options? Like setting the default in the instantiation of the struct, then consider overwriting it if the env var is set? Is the reason to have the comment on top of this line?
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.
Ah, I see there is similar implementation in boltOptions
.
We want to adopt etcd-io/bbolt#585 before it appears in an official bbolt release.
Note that we're not modifying raft-boltdb yet, so raft.db (the bbolt file containing the raft log) doesn't benefit.