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

Multi-Database Support #1005

Open
wants to merge 109 commits into
base: main
Choose a base branch
from
Open

Multi-Database Support #1005

wants to merge 109 commits into from

Conversation

TalZaccai
Copy link
Contributor

@TalZaccai TalZaccai commented Feb 7, 2025

This PR enables multiple logical databases in a single Garnet server instance.

This PR includes:

  • Adding MaxDatabases configuration to garnet.conf + compatibility with redis.conf
  • Multi-DB support in StoreWrapper, including checkpointing & AOF
  • Current-DB context switching support in RespServerSession
  • Update GarnetInfoMetrics to reflect multiple databases data
  • Support for the following commands:
    • SELECT - switch current DB context
    • SWAPDB - swap two DB instances
    • FLUSHDB - flush all keys from current DB in context
    • FLUSHALL - flush all keys from all active databases
  • Add support for specifying a DB ID in the following commands:
    • SAVE - synchronous checkpointing database by ID
    • BGSAVE - asynchronous checkpointing database by ID
    • LASTSAVE - number of seconds since last checkpoint of database by ID
    • COMMITAOF - synchronous database AOF commit by ID
  • Add DB ID to client info
  • Testing for supported commands
  • Benchmarking for supported commands
  • Add website docs for new commands

@prvyk
Copy link
Contributor

prvyk commented Mar 9, 2025

Some tiny nitpicking if it's not too early:

A) Some more bounds checking can be done on the lower bound.
e.g. 'LASTSAVE -1' or starting with --max-databases=0 (both trigger an assertion).

B) SWAPDB doesn't update CLIENT LIST db field if the current connection's database is swapped. Also doesn't update the current database when called from lua inside the script context.

C) Redis also adds a line for each db under 'Keyspace' in INFO command.

@TalZaccai
Copy link
Contributor Author

Some tiny nitpicking if it's not too early:

A) Some more bounds checking can be done on the lower bound. e.g. 'LASTSAVE -1' or starting with --max-databases=0 (both trigger an assertion).

B) SWAPDB doesn't update CLIENT LIST db field if the current connection's database is swapped. Also doesn't update the current database when called from lua inside the script context.

C) Redis also adds a line for each db under 'Keyspace' in INFO command.

Thanks @prvyk! Super helpful!
A - fixed.
B - not sure what you mean here, say the active DB is 1 and we do a SWAPDB 0 1, the active db is still 1 (and the CLIENT LIST sould show so).
C - we currently don't support keyspace, even when we had just one logical database, I'm assuming it's because it's a fairly slow operation (since it requires scanning through all keys in each store), so I left it as-is.

@TalZaccai TalZaccai marked this pull request as ready for review March 11, 2025 04:35
@TalZaccai TalZaccai requested a review from darrenge as a code owner March 11, 2025 04:35
@TalZaccai TalZaccai requested review from badrishc and vazois March 11, 2025 04:35
@prvyk
Copy link
Contributor

prvyk commented Mar 11, 2025

Thanks @prvyk! Super helpful!

A - fixed.

In some cases the check is "db < -1" (and not db < 0). What is -1 supposed to mean when it's 'legal'?

B - not sure what you mean here

I think I got confused with CLIENT LIST. The lua thing is this:

SET a 0
SELECT 2
SET a 2
SELECT 0
EVAL "return { redis.call('SWAPDB', 0, 2); redis.call('GET', 'a') }" 0

Redis ->

  1. OK
  2. "2"

Garnet ->

  1. "OK"
  2. "0"

(Afterwards, 'GET a' returns "2" with both)

Redis seems to switch during the script context.

@TalZaccai
Copy link
Contributor Author

Thanks @prvyk! Super helpful!

A - fixed.

In some cases the check is "db < -1" (and not db < 0). What is -1 supposed to mean when it's 'legal'?

-1 is the same as not specifying an ID (i.e. "save all etc."), I suppose I can just disable it from the input as to not cause confusion.

B - not sure what you mean here

I think I got confused with CLIENT LIST. The lua thing is this:

SET a 0
SELECT 2
SET a 2
SELECT 0
EVAL "return { redis.call('SWAPDB', 0, 2); redis.call('GET', 'a') }" 0

Redis ->

  1. OK
  2. "2"

Garnet ->

  1. "OK"
  2. "0"

(Afterwards, 'GET a' returns "2" with both)

Redis seems to switch during the script context.

Good to know, I haven't tested SWAPDB from a LUA context, I'll add that to the tests as well!

@TalZaccai
Copy link
Contributor Author

@prvyk fixed the Lua swap issue, let me know if you see anything else that needs attention :) Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants