On-demand database allocation instead of preallocation by xbasel · Pull Request #1609 · valkey-io/valkey

xbasel · 2025-01-24T10:37:01Z

Allocate database structures lazily to prevent excessive memory usage
when a large number of databases is configured but not actually used.

fixes #1597

ranshid

some immediate comments.

codecov · 2025-01-27T00:24:52Z

Codecov Report

Attention: Patch coverage is 99.21875% with 1 line in your changes missing coverage. Please review.

Project coverage is 71.53%. Comparing base (3789b29) to head (57573e5).
Report is 10 commits behind head on unstable.

Files with missing lines	Patch %	Lines
src/replication.c	83.33%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##           unstable    #1609      +/-   ##
============================================
+ Coverage     71.43%   71.53%   +0.09%     
============================================
  Files           122      122              
  Lines         66210    66484     +274     
============================================
+ Hits          47300    47561     +261     
- Misses        18910    18923      +13

Files with missing lines	Coverage Δ
src/aof.c	`80.40% <ø> (+0.08%)`	⬆️
src/cluster.c	`90.38% <100.00%> (ø)`
src/cluster_legacy.c	`86.79% <100.00%> (+0.07%)`	⬆️
src/db.c	`90.07% <100.00%> (-0.01%)`	⬇️
src/debug.c	`53.58% <100.00%> (+0.12%)`	⬆️
src/defrag.c	`81.59% <100.00%> (-1.04%)`	⬇️
src/evict.c	`98.50% <100.00%> (+0.02%)`	⬆️
src/expire.c	`96.62% <100.00%> (+0.02%)`	⬆️
src/object.c	`81.41% <100.00%> (ø)`
src/rdb.c	`76.89% <100.00%> (+0.56%)`	⬆️
... and 3 more

... and 19 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

reducing overhead and improving scalability for large database counts. Signed-off-by: xbasel <103044017+xbasel@users.noreply.github.com>

xbasel · 2025-01-27T16:58:11Z

Running valkey with 10 million databases:

valkey-server --databases 10000000

empty dbs:

Before:

# Memory
used_memory:6086209848
used_memory_human:5.67G
used_memory_rss:6063915008
used_memory_rss_human:5.65G

After:

# Memory
used_memory:84790224
used_memory_human:80.86M
used_memory_rss:7602176
used_memory_rss_human:7.25M

db 8 million:

# Keyspace
db0:keys=1,expires=0,avg_ttl=0
db8000000:keys=1,expires=0,avg_ttl=0
127.0.0.1:6379[8000000]>

zuiderkwast

Great, this looks pretty strait-forward.

The benchmark with 10M databases is great, but not a very common I think. :) The main motivation for this optimization IMO is in combination with #1319. For cluster mode with 16 empty databases, does it go from 10MB to 1MB?

xbasel · 2025-02-04T13:37:08Z

Great, this looks pretty strait-forward.

The benchmark with 10M databases is great, but not a very common I think. :) The main motivation for this optimization IMO is in combination with #1319. For cluster mode with 16 empty databases, does it go from 10MB to 1MB?

This has been created artificially (this change hasn't been merged into multi-database code, i changed the code to create 16k slots without cluster mode)

before (preallocated) with (16 databases, 16k slots):

# Memory
used_memory:15887960
used_memory_human:15.15M
used_memory_rss:15204352
used_memory_rss_human:14.50M
used_memory_peak:15887960
used_memory_peak_human:15.15M

allocated-on-demand (16 databases, 16k slots), database 0 is always pre-allocated..

# Memory
used_memory:7327440
used_memory_human:6.99M
used_memory_rss:14548992
used_memory_rss_human:13.88M

Signed-off-by: xbasel <103044017+xbasel@users.noreply.github.com>

xbasel · 2025-02-17T10:28:11Z

@zuiderkwast / @ranshid reminder :)

zuiderkwast · 2025-02-17T10:42:56Z

It's not forgotten, but the focus now is on stabilizing the 8.1 release and fix the flaky tests before 8.1 GA. This one will have to wait until after 8.1. It will be good to release it together with the related cluster-multidb feature.

soloestoy · 2025-02-24T02:58:52Z

I see many changes from server.db->keys to server.db[0]->keys, such as in countKeysInSlot. This will conflict with multi-databases. I think this PR should be aligned after the multi-databases merge.

Signed-off-by: xbasel <103044017+xbasel@users.noreply.github.com>

Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech> Signed-off-by: xbasel <103044017+xbasel@users.noreply.github.com>

Signed-off-by: xbasel <103044017+xbasel@users.noreply.github.com>

Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech> Signed-off-by: xbasel <103044017+xbasel@users.noreply.github.com>

zuiderkwast

Yeah this looks good to merge.

Signed-off-by: xbasel <103044017+xbasel@users.noreply.github.com>

Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech> Signed-off-by: xbasel <103044017+xbasel@users.noreply.github.com>

Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>

Allocate database structures lazily to prevent excessive memory usage when a large number of databases is configured but not actually used. fixes valkey-io#1597 --------- Signed-off-by: xbasel <103044017+xbasel@users.noreply.github.com> Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech> Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech> Signed-off-by: chzhoo <czawyx@163.com>

Allocate database structures lazily to prevent excessive memory usage when a large number of databases is configured but not actually used. fixes valkey-io#1597 --------- Signed-off-by: xbasel <103044017+xbasel@users.noreply.github.com> Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech> Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech> Signed-off-by: shanwan1 <shanwan1@intel.com>

In valkey-io#1609, we now doing on-demand database allocation instead of preallocation. And in beginDefragCycle, we should not skip empty databases if they exist. The defrag still defrags the internal allocations of the hashtables structs if they exist. Call chain: defragStageDbKeys -> defragStageKvstoreHelper -> kvstoreHashtableDefragTables -> hashtableDefragTables. Signed-off-by: Binbin <binloveplay1314@qq.com>

In #1609, we now doing on-demand database allocation instead of preallocation. And in beginDefragCycle, we should not skip empty databases if they exist. The defrag still defrags the internal allocations of the hashtables structs if they exist. Call chain: defragStageDbKeys -> defragStageKvstoreHelper -> kvstoreHashtableDefragTables -> hashtableDefragTables. Signed-off-by: Binbin <binloveplay1314@qq.com>

ranshid reviewed Jan 24, 2025

View reviewed changes

Comment thread src/aof.c Outdated

Comment thread src/cluster_legacy.c Outdated

Comment thread src/db.c Outdated

xbasel force-pushed the lazyinit branch from 12ef00e to 33da0ea Compare January 27, 2025 00:09

xbasel force-pushed the lazyinit branch 2 times, most recently from 1bfa8b4 to 9f07a4f Compare January 27, 2025 08:49

xbasel requested a review from ranshid January 27, 2025 09:14

xbasel force-pushed the lazyinit branch from 26256e0 to 25028e4 Compare January 27, 2025 14:48

Switch from preallocating all databases to lazy allocation,

028c7df

reducing overhead and improving scalability for large database counts. Signed-off-by: xbasel <103044017+xbasel@users.noreply.github.com>

xbasel force-pushed the lazyinit branch from 25028e4 to 028c7df Compare January 27, 2025 15:15

xbasel mentioned this pull request Jan 27, 2025

[BUG] The engine isn't usable when running with a large number of databases. #1597

Closed

zuiderkwast reviewed Jan 30, 2025

View reviewed changes

Comment thread src/server.c Outdated

Comment thread src/server.c Outdated

Comment thread src/server.c Outdated

xbasel requested a review from zuiderkwast February 4, 2025 14:31

xbasel force-pushed the lazyinit branch from de09b94 to f553b46 Compare February 4, 2025 15:04

changes following code review.

c70492e

Signed-off-by: xbasel <103044017+xbasel@users.noreply.github.com>

xbasel force-pushed the lazyinit branch from f553b46 to c70492e Compare February 4, 2025 15:22

This was referenced Feb 5, 2025

[NEW] Multiple DB supports in cluster mode #1319

Closed

[NEW] Multi-database support in cluster mode - Implementation Plan #1681

Closed

ranshid added performance cluster release-notes This issue should get a line item in the release notes labels Feb 17, 2025

xbasel mentioned this pull request Feb 23, 2025

Add multi-database support to cluster mode #1671

Merged

Merge remote-tracking branch 'origin/unstable' into lazyinit

f3b7a55

xbasel added 2 commits May 9, 2025 23:37

fix

c61bbad

Signed-off-by: xbasel <103044017+xbasel@users.noreply.github.com>

format

7d5bcd3

Signed-off-by: xbasel <103044017+xbasel@users.noreply.github.com>

xbasel marked this pull request as ready for review May 9, 2025 21:17

xbasel added 2 commits May 11, 2025 13:20

Remove file

6a5be07

Signed-off-by: xbasel <103044017+xbasel@users.noreply.github.com>

Merge remote-tracking branch 'origin/unstable' into lazyinit

f7f3e3d

zuiderkwast reviewed May 11, 2025

View reviewed changes

Comment thread src/cluster_legacy.c Outdated

Comment thread src/db.c

Comment thread src/defrag.c

Comment thread src/evict.c Outdated

Comment thread src/evict.c Outdated

Comment thread src/db.c Outdated

xbasel and others added 3 commits May 11, 2025 17:32

Move dbsHaveNoKeys to server.c

5352ed0

Signed-off-by: xbasel <103044017+xbasel@users.noreply.github.com>

Update src/cluster_legacy.c

1f16b7d

Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech> Signed-off-by: xbasel <103044017+xbasel@users.noreply.github.com>

fix comment

12a4944

Signed-off-by: xbasel <103044017+xbasel@users.noreply.github.com>

xbasel force-pushed the lazyinit branch from af9c81e to 12a4944 Compare May 11, 2025 14:32

Update src/evict.c

7794bf4

Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech> Signed-off-by: xbasel <103044017+xbasel@users.noreply.github.com>

xbasel requested a review from zuiderkwast May 26, 2025 10:01

zuiderkwast approved these changes May 26, 2025

View reviewed changes

Comment thread src/db.c Outdated

Comment thread src/replication.c

xbasel and others added 3 commits June 5, 2025 14:17

Merge branch 'unstable' into lazyinit

29745de

fix after merge

7269a82

Signed-off-by: xbasel <103044017+xbasel@users.noreply.github.com>

Update src/db.c

a8599e6

Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech> Signed-off-by: xbasel <103044017+xbasel@users.noreply.github.com>

zuiderkwast mentioned this pull request Jun 10, 2025

Send the database ID of the last replicated item (i.e. server.replicas_eldb) to replica #2194

Closed

zuiderkwast reviewed Jun 10, 2025

View reviewed changes

Comment thread src/server.h Outdated

xbasel and others added 2 commits June 10, 2025 10:59

Update src/server.h

0e1c05c

Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech> Signed-off-by: xbasel <103044017+xbasel@users.noreply.github.com>

Remove excessive semicolon

57573e5

Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>

zuiderkwast removed the cluster label Jun 10, 2025

zuiderkwast merged commit 8e2f355 into valkey-io:unstable Jun 10, 2025
61 checks passed

github-project-automation Bot moved this from In Progress to Done in Valkey 9.0 Jun 10, 2025

enjoy-binbin mentioned this pull request Jul 26, 2025

Defrag should only skip non-existent dbs, not empty dbs #2379

Merged

enjoy-binbin mentioned this pull request Jul 28, 2025

Allow dynamic modification of databases num if the db is not been used #2385

Open

ranshid mentioned this pull request Sep 16, 2025

Clustered Databases in Valkey 9 blog post valkey-io/valkey-io.github.io#353

Merged

1 task

Uh oh!

Conversation

xbasel commented Jan 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ranshid left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented Jan 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

xbasel commented Jan 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zuiderkwast left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

xbasel commented Feb 4, 2025

Uh oh!

xbasel commented Feb 17, 2025

Uh oh!

zuiderkwast commented Feb 17, 2025

Uh oh!

soloestoy commented Feb 24, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zuiderkwast left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

xbasel commented Jan 24, 2025 •

edited

Loading

codecov Bot commented Jan 27, 2025 •

edited

Loading

xbasel commented Jan 27, 2025 •

edited

Loading