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

agent/cache: Store leases in-order in persistent cache so that restore respects dependencies #12843

Merged
merged 11 commits into from Oct 27, 2021

Conversation

tomhjp
Copy link
Contributor

@tomhjp tomhjp commented Oct 15, 2021

An alternative to #12765. Here we depend on BoltDB storing, and iterating on keys in its bucket, in byte slice order, and use an auto-incrementing index to store each lease in the same order it was created. When we then restore from the persistent storage, it should automatically restore in dependency order.

@tomhjp tomhjp requested review from tvoran, calvn and a team October 15, 2021 15:06
@vercel vercel bot temporarily deployed to Preview – vault October 18, 2021 11:03 Inactive
@vercel vercel bot temporarily deployed to Preview – vault-storybook October 18, 2021 11:03 Inactive
@vercel vercel bot temporarily deployed to Preview – vault-storybook October 18, 2021 15:40 Inactive
@vercel vercel bot temporarily deployed to Preview – vault October 18, 2021 15:40 Inactive
@vercel vercel bot temporarily deployed to Preview – vault-storybook October 19, 2021 14:48 Inactive
@vercel vercel bot temporarily deployed to Preview – vault October 19, 2021 14:48 Inactive
@benashz benashz self-requested a review October 21, 2021 16:00
@vercel vercel bot temporarily deployed to Preview – vault-storybook October 25, 2021 15:11 Inactive
@vercel vercel bot temporarily deployed to Preview – vault October 25, 2021 15:11 Inactive
@vercel vercel bot temporarily deployed to Preview – vault October 25, 2021 16:04 Inactive
@vercel vercel bot temporarily deployed to Preview – vault-storybook October 25, 2021 16:04 Inactive
@vercel vercel bot temporarily deployed to Preview – vault-storybook October 25, 2021 16:27 Inactive
@vercel vercel bot temporarily deployed to Preview – vault October 25, 2021 16:27 Inactive
@calvn calvn self-requested a review October 25, 2021 16:57
Copy link
Member

@calvn calvn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One last question, but otherwise looks great!

command/agent/cache/lease_cache_test.go Show resolved Hide resolved
@vercel vercel bot temporarily deployed to Preview – vault October 26, 2021 11:44 Inactive
@vercel vercel bot temporarily deployed to Preview – vault-storybook October 26, 2021 11:44 Inactive
})

if err := tx.DeleteBucket([]byte(v1BucketType)); err != nil {
return fmt.Errorf("failed to clean up %s bucket during v1 to v2 schema migration: %w", v1BucketType, err)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean we need to do the migration once more? Could we end up in a state where a migration will never succeed? If so, perhaps it would make sense to scrap the db and start fresh.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the migration fails, something super weird has happened because migrations should be very rare (we only explicitly support persistent stores in k8s today) and failures should also be very rare because the schema creation/migration code is very lenient. So I'd prefer that an operator gets a chance to clean up/debug before we wipe everything automatically.

command/agent/cache/cacheboltdb/bolt.go Show resolved Hide resolved
command/agent/cache/cacheboltdb/bolt.go Outdated Show resolved Hide resolved
command/agent/cache/cacheboltdb/bolt_test.go Show resolved Hide resolved
…s' of github.com:hashicorp/vault into agent-persistent-cache-restore-dependencies-ordered-keys
@vercel vercel bot temporarily deployed to Preview – vault October 27, 2021 10:08 Inactive
@vercel vercel bot temporarily deployed to Preview – vault-storybook October 27, 2021 10:08 Inactive
@tomhjp tomhjp merged commit 14cffc3 into main Oct 27, 2021
@tomhjp tomhjp deleted the agent-persistent-cache-restore-dependencies-ordered-keys branch October 27, 2021 10:36
@tvoran tvoran added this to the 1.9 milestone Oct 28, 2021
qk4l pushed a commit to qk4l/vault that referenced this pull request Feb 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants