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

feat(dbless) use LMDB as DB-less backend #8224

Merged
merged 5 commits into from
Mar 31, 2022
Merged

feat(dbless) use LMDB as DB-less backend #8224

merged 5 commits into from
Mar 31, 2022

Conversation

dndx
Copy link
Member

@dndx dndx commented Dec 22, 2021

This PR adds LMDB (Lightning Memory-Mapped Database) support for
DB-less. At the same time, the shdict based DB-less storage backend has
been retired and removed from the codebase.

LMDB has better concurrency characters and is generally much more stable
than shdict for storing config data. Because it can be natively accessed
from different processes, we also removed the hack that sends the full
DB-less config to the stream subsystem, which should improve the
stability of DB-less reload at runtime as well.

New config options lmdb_environment_path and lmdb_map_size has been
added.

TODO: LMDB is fully capable of persisting the config between restarts, currently this is not enabled so it behaves like a in-memory storage (content is always wiped at startup). This will be enabled once we have more understanding of the impact.

kong/clustering/data_plane.lua Outdated Show resolved Hide resolved
kong/db/declarative/init.lua Show resolved Hide resolved
kong/db/declarative/marshaller.lua Show resolved Hide resolved
kong/global.lua Outdated Show resolved Hide resolved
kong/init.lua Outdated Show resolved Hide resolved
kong/init.lua Outdated Show resolved Hide resolved
kong/init.lua Outdated Show resolved Hide resolved
spec/02-integration/04-admin_api/15-off_spec.lua Outdated Show resolved Hide resolved
kong/api/routes/config.lua Outdated Show resolved Hide resolved
kong/init.lua Outdated Show resolved Hide resolved
kong/init.lua Show resolved Hide resolved
@dndx dndx force-pushed the feat/lmdb branch 5 times, most recently from c038130 to 2516980 Compare December 24, 2021 05:03
@dndx dndx force-pushed the feat/lmdb branch 2 times, most recently from a1e58fa to 2516980 Compare December 28, 2021 05:59
kikito
kikito previously requested changes Jan 6, 2022
.requirements Outdated Show resolved Hide resolved
kong/api/routes/config.lua Show resolved Hide resolved
kong/clustering/data_plane.lua Outdated Show resolved Hide resolved
kong/db/declarative/init.lua Show resolved Hide resolved
kong/db/declarative/init.lua Show resolved Hide resolved
kong/db/strategies/off/init.lua Show resolved Hide resolved
kong/db/declarative/init.lua Show resolved Hide resolved
kong/db/strategies/off/init.lua Show resolved Hide resolved
kong/init.lua Show resolved Hide resolved
.requirements Outdated Show resolved Hide resolved
spec/02-integration/04-admin_api/15-off_spec.lua Outdated Show resolved Hide resolved
kong/templates/kong_defaults.lua Show resolved Hide resolved
-- If it takes more than 60s it is very likely to be an internal error.
-- However it will be reported as: "failed to broadcast reconfigure event: recursive".
-- Let's paste the error message here in case someday we try to search it.
-- Should we handle this case specially?
Copy link
Contributor

Choose a reason for hiding this comment

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

As somebody who has seen this specific error message on a production system, big big +1 to anything that we can do to make the error more meaningful and explanatory.

Copy link
Member Author

Choose a reason for hiding this comment

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

I am not a 100% sure about the triggering condition of this, and I hesitate to make a change without understanding why the "recursive" error occurs. Will try to syncup with @suika-kong more offline on this error handling after it is merged (the current PR is getting hard to manage).

kong/init.lua Outdated Show resolved Hide resolved
@dndx dndx force-pushed the feat/lmdb branch 3 times, most recently from fe85dae to 5dccba6 Compare March 30, 2022 13:39
@dndx dndx requested a review from kikito March 30, 2022 13:39
Copy link
Contributor

@aboudreault aboudreault left a comment

Choose a reason for hiding this comment

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

gojira PR also merged: Kong/gojira#43

dndx and others added 5 commits March 30, 2022 22:33
This PR adds LMDB (Lightning Memory-Mapped Database) support for
DB-less. At the same time, the shdict based DB-less storage backend has
been retired and removed from the codebase.

LMDB has better concurrency characters and is generally much more stable
than shdict for storing config data. Because it can be natively accessed
from different processes, we also removed the hack that sends the full
DB-less config to the stream subsystem, which should improve the
stability of DB-less reload at runtime as well.

New config options `lmdb_environment_path` and `lmdb_map_size` has been
added.

Co-authored-by: Suika <xumin.zhou@konghq.com>
@dndx dndx dismissed kikito’s stale review March 31, 2022 11:55

Already addressed

@dndx dndx merged commit 5b7676c into master Mar 31, 2022
@dndx dndx deleted the feat/lmdb branch March 31, 2022 11:55
bungle added a commit that referenced this pull request Jun 14, 2022
### Summary

The reconfigure event needs to pass hashes and not only the default
workspace. This broke when PR #8519 was merged and then `LMDB` PR
#8224 got merged after it.
bungle added a commit that referenced this pull request Jun 14, 2022
### Summary

The reconfigure event needs to pass hashes and not only the default
workspace. This broke when PR #8519 was merged and then `LMDB` PR
#8224 got merged after it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants