-
Notifications
You must be signed in to change notification settings - Fork 67
Clustered Databases in Valkey 9 blog post #353
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
Conversation
Signed-off-by: Kyle J. Davis <[email protected]>
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.
I don't know how I feel about the intro. I see it has a thesis that in the past people said don't use databases, but you can use them now. It might be more straight forward to make the case that they weren't well supported and we're working to make them easier to support?
The rest of it makes enough sense. I think telling a story about how we're working to continue improve it might also work a bit better. There is an open PR for database ACLs.
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.
I think this is a very important blog. The way I see it is that this is the fist public statement of divergent philosophy we are taken from the old fork.
For this to be motivating I would like to suggest a slightly different structure that might also work:
- I would start (as you did) with some mention of the old philosophy and how we see it today in Valkey.
- Then, I would provide good usecases were applications can use multi-databases. things like data separation, sandboxing, rollback etc...
- The next part (IMO) should be focused on how we extended the ability to scale with databases
- I would now add a section about future plans, and maybe also include our strategy (which does not target full multi-tenancy support, but aims for better observability and control over "namespaces")
Signed-off-by: Kyle J. Davis <[email protected]>
|
||
Aside from the aforementioned lack of resource isolation, numbered databases in clustered Valkey have a few places that you need to look out for today. First, right now numbered databases don’t have dedicated metrics, so it can be hard to gain insights into per-database resource usage. Second, the ACL system doesn’t currently address numbered databases, so you can’t meaningfully restrict access to databases (at time of writing, there is an [open pull request that addresses this](https://github.com/valkey-io/valkey/pull/2309)). Unlike in the previous project, databases are an active area of work for the Valkey project and the database abstraction holds potential as a way to implement new features. | ||
|
||
Finally, while numbered databases are well supported in client libraries there are rough edges: |
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.
Some client do not allow to update the DB post client creation. That is, the DB is immutable. We recommend in such case to use a DB per client.
It is complex in async clients or clients that supports pipelines to support updates of the DB using select command post the client creation as it may cause data corruption in edge cases. I can elaborate more
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.
Line 88 covers most of this. I'm well aware of the issues of clients not fully managing SELECT
the correct way.
Do you have a list of the clients don't support SELECT
(e.g. only at connection)? I'm glad to add this to 88.
Co-authored-by: Ran Shidlansik <[email protected]> Signed-off-by: Kyle J. Davis <[email protected]>
Signed-off-by: Kyle J. Davis <[email protected]>
Signed-off-by: Kyle J. Davis <[email protected]>
Signed-off-by: Kyle J. Davis <[email protected]>
Signed-off-by: Kyle J. Davis <[email protected]>
Signed-off-by: Kyle J. Davis <[email protected]>
Signed-off-by: Kyle J. Davis <[email protected]>
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.
One material comment, I could be convinced to leave it though, but other than that looks good to me.
The blog still feels very much like you wrote it but comes across a lot more positive now, I love most of the changes.
Co-authored-by: Madelyn Olson <[email protected]> Signed-off-by: Kyle J. Davis <[email protected]>
Co-authored-by: Madelyn Olson <[email protected]> Signed-off-by: Kyle J. Davis <[email protected]>
Signed-off-by: Kyle J. Davis <[email protected]>
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.
Overall looks very good!
I added some suggestions to consider
Co-authored-by: Ran Shidlansik <[email protected]> Signed-off-by: Kyle J. Davis <[email protected]>
Description
Issues Resolved
#334
Check List
--signoff
By submitting this pull request, I confirm that my contribution is made under the terms of the BSD-3-Clause License.