-
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
VAULT-31907: Entity loading speedup #29326
base: main
Are you sure you want to change the base?
VAULT-31907: Entity loading speedup #29326
Conversation
CI Results: |
b45d134
to
83f2147
Compare
Build Results: |
@@ -0,0 +1,3 @@ | |||
```release-note:improvement | |||
core/identity: Improve performance of loading entities when unsealing by batching updates, caching local alias storage reads, and doing more work in parallel. |
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.
Great clarity in the changelog entry
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.
Agreed!
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!
Description
This PR has 3 performance improvements in the
loadEntities
function:I ran benchmarks using:
I got the number 1024 by batching entities together across buckets (which this branch does not implement, but I used for benchmarking so that I wouldn't need to create millions of entities). These batches are counted by adding the number of entities and the number of aliases per entity (note: this is different than what I posted on slack earlier today. I was incorrectly only counting the number of entities per batch).
I also benchmarked the other 2 performance improvements;
cache_aliases_{true|false}
being optimization number 2, andunmarshal_parallel_{true|false}
being optimization number 3.The results of the benchmarks are:
TODO only if you're a HashiCorp employee
backport/
label that matches the desired release branch. Note that in the CE repo, the latest release branch will look likebackport/x.x.x
, but older release branches will bebackport/ent/x.x.x+ent
.of a public function, even if that change is in a CE file, double check that
applying the patch for this PR to the ENT repo and running tests doesn't
break any tests. Sometimes ENT only tests rely on public functions in CE
files.
in the PR description, commit message, or branch name.
description. Also, make sure the changelog is in this PR, not in your ENT PR.