Skip to content
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

Make sure sparse mode is used on secrets where it is explicit #271

Merged
merged 1 commit into from
May 15, 2017

Conversation

kamaradclimber
Copy link
Contributor

Sparse mode is marked in xxx_keys item with:

mode: "sparse"

but when decrypting secrets, each node is trying to read the sparse format first
(xxx_key_[node]) and then fallback to normal xxx_keys.
This adds a performance penalty both on reading secrets and during refresh.

With this patch, sparse format is checked only when secret is marked as sparse.

This makes refresh a fast no-op (it was already a no-op with
#269 now it is faster) since the only
cost is now searching nodes matching search_query.

Sparse mode is marked in xxx_keys item with:

> mode: "sparse"

but when decrypting secrets, each node is trying to read the sparse format first
(xxx_key_[node]) and then fallback to normal xxx_keys.
This adds a performance penalty both on reading secrets and during refresh.

With this patch, sparse format is checked only when secret is marked as sparse.

This makes refresh a fast no-op (it was already a no-op with
chef#269 now it is faster) since the only
cost is now searching nodes matching search_query.

Change-Id: I38f511b9f590240775085a386b387c476d3a1f5c
Signed-off-by: Grégoire Seux <g.seux@criteo.com>
@kamaradclimber
Copy link
Contributor Author

For reference, performance penalty during refresh is a few milliseconds per node (~40ms on my test).
On a medium secret encrypted for 500 nodes, refresh operation takes:

with this patch, refresh operation is ~9s.

@thommay thommay merged commit de4a1f9 into chef:master May 15, 2017
@thommay thommay added the Type: Bug Does not work as expected. label Jun 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Does not work as expected.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants