-
Notifications
You must be signed in to change notification settings - Fork 239
perf(cmd): scale db-cache-size to available memory #3927
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | |||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -147,6 +147,8 @@ const ( | ||||||||||||||||||||||||||||
| defaultRemoteDB = "" | |||||||||||||||||||||||||||||
| defaultRPCMaxBlockScan = math.MaxUint | |||||||||||||||||||||||||||||
| defaultCacheSizeMb = 1024 | |||||||||||||||||||||||||||||
| maxAutoCacheSizeMb = 8192 | |||||||||||||||||||||||||||||
| cacheMemoryFraction = 4 | |||||||||||||||||||||||||||||
| defaultDBMaxHandlesFloor = 1024 | |||||||||||||||||||||||||||||
| defaultDBMaxHandlesCeiling = 1_048_576 | |||||||||||||||||||||||||||||
| defaultGwAPIKey = "" | |||||||||||||||||||||||||||||
|
|
@@ -234,8 +236,10 @@ const ( | ||||||||||||||||||||||||||||
| maxVMQueueUsage = "Maximum number for requests to queue after reaching max-vms before starting to reject incoming requests" | |||||||||||||||||||||||||||||
| remoteDBUsage = "gRPC URL of a remote Juno node" | |||||||||||||||||||||||||||||
| rpcMaxBlockScanUsage = "Maximum number of blocks scanned in single starknet_getEvents call" | |||||||||||||||||||||||||||||
| dbCacheSizeUsage = "Determines the amount of memory (in megabytes) allocated for caching data in the database." | |||||||||||||||||||||||||||||
| dbMaxHandlesUsage = "A soft limit on the number of open files that can be used by the DB. " + | |||||||||||||||||||||||||||||
| dbCacheSizeUsage = "Determines the amount of memory (in megabytes) allocated for " + | |||||||||||||||||||||||||||||
| "caching data in the database. When not set, defaults to a quarter of total " + | |||||||||||||||||||||||||||||
| "memory (host RAM or cgroup limit), between 1024 and 8192" | |||||||||||||||||||||||||||||
| dbMaxHandlesUsage = "A soft limit on the number of open files that can be used by the DB. " + | |||||||||||||||||||||||||||||
| "When not set, defaults to half of the process fd limit (min 1024, max 1048576)" | |||||||||||||||||||||||||||||
| //nolint: gosec // usage text, not a credential | |||||||||||||||||||||||||||||
| gwAPIKeyUsage = "API key for gateway endpoints to avoid throttling" | |||||||||||||||||||||||||||||
|
|
@@ -281,7 +285,8 @@ const ( | ||||||||||||||||||||||||||||
| "may consume; a compilation exceeding it is aborted. Enforced on Linux only. " + | |||||||||||||||||||||||||||||
| "0 disables the limit." | |||||||||||||||||||||||||||||
| maxCompilationReserveMemory = "Memory (in MB) excluded from the compilations memory budget when " + | |||||||||||||||||||||||||||||
| "calculating the default for `max-concurrent-compilations`" | |||||||||||||||||||||||||||||
| "calculating the default for `max-concurrent-compilations`. The DB cache size is " + | |||||||||||||||||||||||||||||
| "excluded on top of this reserve" | |||||||||||||||||||||||||||||
| pruneModeUsage = "Enables block-data and state-history pruning. Pruning is " + | |||||||||||||||||||||||||||||
| "disabled by default; passing this flag (with or without a value) turns " + | |||||||||||||||||||||||||||||
| "it on. The value is the size of the retention window in blocks, counted " + | |||||||||||||||||||||||||||||
|
|
@@ -592,7 +597,7 @@ func NewCmd(config *node.Config, run func(*cobra.Command, []string) error) *cobr | ||||||||||||||||||||||||||||
|
|
|||||||||||||||||||||||||||||
| // --- Database --- | |||||||||||||||||||||||||||||
| junoCmd.Flags().String(dbPathF, defaultDBPath, dbPathUsage) | |||||||||||||||||||||||||||||
| junoCmd.Flags().Uint(dbCacheSizeF, defaultCacheSizeMb, dbCacheSizeUsage) | |||||||||||||||||||||||||||||
| junoCmd.Flags().Uint(dbCacheSizeF, defaultDBCacheSize(), dbCacheSizeUsage) | |||||||||||||||||||||||||||||
| junoCmd.Flags().Int(dbMaxHandlesF, defaultDBMaxHandles(), dbMaxHandlesUsage) | |||||||||||||||||||||||||||||
| junoCmd.Flags().String( | |||||||||||||||||||||||||||||
| dbCompactionConcurrencyF, defaultDBCompactionConcurrency, dbCompactionConcurrencyUsage, | |||||||||||||||||||||||||||||
|
|
@@ -716,6 +721,16 @@ func NewCmd(config *node.Config, run func(*cobra.Command, []string) error) *cobr | ||||||||||||||||||||||||||||
| return junoCmd | |||||||||||||||||||||||||||||
| } | |||||||||||||||||||||||||||||
|
|
|||||||||||||||||||||||||||||
| // defaultDBCacheSize gives the DB block cache a quarter of the memory this | |||||||||||||||||||||||||||||
| // process can use, clamped to [defaultCacheSizeMb, maxAutoCacheSizeMb]. | |||||||||||||||||||||||||||||
| func defaultDBCacheSize() uint { | |||||||||||||||||||||||||||||
| return dbCacheSizeForMemoryMB(utils.AvailableMemoryMB()) | |||||||||||||||||||||||||||||
| } | |||||||||||||||||||||||||||||
|
|
|||||||||||||||||||||||||||||
| func dbCacheSizeForMemoryMB(memMB uint64) uint { | |||||||||||||||||||||||||||||
| return min(max(uint(memMB/cacheMemoryFraction), defaultCacheSizeMb), maxAutoCacheSizeMb) | |||||||||||||||||||||||||||||
| } | |||||||||||||||||||||||||||||
|
Comment on lines
+730
to
+732
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. important — the lower clamp isn't memory-aware, which is the one case where being memory-aware matters most. For any
A 1–2 GB cgroup (small k8s pods, CI containers) gets a block cache equal to half or all of its limit, and since Consider bounding the floor too: func dbCacheSizeForMemoryMB(memMB uint64) uint {
quarter := uint(memMB / cacheMemoryFraction)
return min(max(quarter, min(defaultCacheSizeMb, uint(memMB))), maxAutoCacheSizeMb)
}or, if the 1024 floor is deliberate, say so in the comment and log a warning when the floor exceeds a quarter of detected memory so operators of small containers see it. Separately:
Comment on lines
+730
to
+732
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. important (standing item, re-anchored after this rebase) — and a correction to the fix I suggested earlier: the snippet I proposed in my previous review does not actually fix the case it describes. Please ignore it. The problem stands: for any Why my earlier suggestion was wrong —
// dbCacheSizeForMemoryMB gives the cache a quarter of memMB, capped at
// maxAutoCacheSizeMb. The minAutoCacheSizeMb floor only applies when memory is
// large enough for it to stay within the quarter budget, so a small container
// does not get a cache the size of its whole limit.
func dbCacheSizeForMemoryMB(memMB uint64) uint {
quarter := uint(memMB / cacheMemoryFraction)
if quarter < minAutoCacheSizeMb {
return max(quarter, minCacheSizeMb) // e.g. 64; pebble's own floor is 8 MB
}
return min(quarter, maxAutoCacheSizeMb)
}…or keep the flat floor deliberately (defensible — a 1–2 GB host can't sync Juno anyway), and in that case say so in the doc comment on line 724 and log a warning when the floor exceeds a quarter of detected memory, so operators of memory-capped containers can see why the process is being killed. Either way, Separately (unchanged): |
|||||||||||||||||||||||||||||
|
|
|||||||||||||||||||||||||||||
| // defaultDBMaxHandles gives the DB half of the process fd limit, clamped to | |||||||||||||||||||||||||||||
| // [defaultDBMaxHandlesFloor, defaultDBMaxHandlesCeiling]. | |||||||||||||||||||||||||||||
| func defaultDBMaxHandles() int { | |||||||||||||||||||||||||||||
|
|
|||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -73,7 +73,7 @@ func TestConfigPrecedence(t *testing.T) { | |
| defaultRPCMaxConcurrentRequests := uint(256000) | ||
| defaultRPCMaxRequestQueue := uint(256000) | ||
| defaultRPCMaxBlockScan := uint(math.MaxUint) | ||
| defaultMaxCacheSize := uint(1024) | ||
| defaultMaxCacheSize := min(max(uint(utils.AvailableMemoryMB()/4), 1024), 8192) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit — this re-implements |
||
| defaultMaxHandles := max(int(min(fdLimit/2, 1_048_576)), 1024) | ||
| defaultDBMemtableSize := uint(256) | ||
| defaultDBMemtableCount := uint(2) | ||
|
|
@@ -224,7 +224,7 @@ func TestConfigPrecedence(t *testing.T) { | |
| "custom network all flags": { | ||
| inputArgs: []string{ | ||
| "--log-level", "debug", "--http-port", "4576", "--http-host", "0.0.0.0", | ||
| "--db-path", "/home/.juno", "--pprof", "--db-cache-size", "1024", | ||
| "--db-path", "/home/.juno", "--pprof", | ||
| "--cn-name", "custom", "--cn-feeder-url", "http://awesome.feeder", "--cn-gateway-url", "http://awesome.gateway", | ||
| "--cn-l1-chain-id", "0x1", "--cn-l2-chain-id", "SN_AWESOME", | ||
| "--cn-unverifiable-range", "0,10", | ||
|
|
@@ -416,7 +416,8 @@ http-port: 4576 | |
| "all flags without config file": { | ||
| inputArgs: []string{ | ||
| "--log-level", "debug", "--http-port", "4576", "--http-host", "0.0.0.0", | ||
| "--db-path", "/home/.juno", "--network", "sepolia-integration", "--pprof", "--db-cache-size", "1024", | ||
| "--db-path", "/home/.juno", "--network", "sepolia-integration", "--pprof", | ||
| "--db-cache-size", "2222", | ||
| }, | ||
| expectedConfig: &node.Config{ | ||
| LogLevel: "debug", | ||
|
|
@@ -443,7 +444,7 @@ http-port: 4576 | |
| RPCMaxConcurrentRequests: defaultRPCMaxConcurrentRequests, | ||
| RPCMaxRequestQueue: defaultRPCMaxRequestQueue, | ||
| RPCMaxBlockScan: defaultRPCMaxBlockScan, | ||
| DBCacheSize: defaultMaxCacheSize, | ||
| DBCacheSize: 2222, | ||
| DBMaxHandles: defaultMaxHandles, | ||
| DBMemtableSize: defaultDBMemtableSize, | ||
| DBMemtableCount: defaultDBMemtableCount, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,86 @@ | ||
| package node | ||
|
|
||
| import ( | ||
| "testing" | ||
|
|
||
| "github.com/NethermindEth/juno/utils/log" | ||
| "github.com/stretchr/testify/assert" | ||
| ) | ||
|
|
||
| func TestCalculateCompilerConcurrencyBudget(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
| cfg Config | ||
| cores uint64 | ||
| memMB uint64 | ||
| wantConc uint64 | ||
| wantQueue uint64 | ||
| }{ | ||
| { | ||
| name: "db cache is reserved on top of node reserve", | ||
| cfg: Config{MaxCompilationMemory: 4096, NodeMemoryReserve: 4096, DBCacheSize: 8192}, | ||
| cores: 64, | ||
| memMB: 65536, | ||
| wantConc: 13, // (65536 - 4096 - 8192) / 4096 | ||
| wantQueue: 26, | ||
| }, | ||
| { | ||
| name: "remote db ignores the local cache size", | ||
| cfg: Config{ | ||
| MaxCompilationMemory: 4096, NodeMemoryReserve: 4096, DBCacheSize: 8192, | ||
| RemoteDB: "localhost:9090", | ||
| }, | ||
| cores: 64, | ||
| memMB: 65536, | ||
| wantConc: 15, // (65536 - 4096) / 4096 | ||
| wantQueue: 30, | ||
| }, | ||
| { | ||
| name: "reserve plus cache covering all memory floors to 1", | ||
| cfg: Config{MaxCompilationMemory: 4096, NodeMemoryReserve: 4096, DBCacheSize: 8192}, | ||
| cores: 64, | ||
| memMB: 12288, | ||
| wantConc: 1, | ||
| wantQueue: 2, | ||
| }, | ||
| { | ||
| name: "no compilation memory limit uses core count", | ||
| cfg: Config{MaxCompilationMemory: 0, NodeMemoryReserve: 4096, DBCacheSize: 8192}, | ||
| cores: 64, | ||
| memMB: 65536, | ||
| wantConc: 64, | ||
| wantQueue: 128, | ||
| }, | ||
| { | ||
| name: "explicit setting bypasses derivation", | ||
| cfg: Config{ | ||
| MaxConcurrentCompilations: 5, MaxConcurrentCompilationsExplicit: true, | ||
| MaxCompilationMemory: 4096, NodeMemoryReserve: 4096, DBCacheSize: 8192, | ||
| }, | ||
| cores: 64, | ||
| memMB: 65536, | ||
| wantConc: 5, | ||
| wantQueue: 10, | ||
| }, | ||
| { | ||
| name: "explicit queue bypasses derivation", | ||
| cfg: Config{ | ||
| MaxCompilationMemory: 4096, NodeMemoryReserve: 4096, DBCacheSize: 8192, | ||
| MaxCompilationQueue: 7, MaxCompilationQueueExplicit: true, | ||
| }, | ||
| cores: 64, | ||
| memMB: 65536, | ||
| wantConc: 13, | ||
| wantQueue: 7, | ||
| }, | ||
| } | ||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| conc, queue := calculateCompilerConcurrencyBudget( | ||
| &tt.cfg, tt.cores, tt.memMB, log.NewNopZapLogger(), | ||
| ) | ||
| assert.Equal(t, tt.wantConc, conc) | ||
| assert.Equal(t, tt.wantQueue, queue) | ||
| }) | ||
| } | ||
| } |
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.
important (re-anchored after the rebase — the original comment was marked outdated) — the lower clamp isn't memory-aware, which is the one range where being memory-aware matters most.
For any
memMB < 4096the result is pinned atdefaultCacheSizeMb(1024):pebble.NewCacheis a ceiling the process grows into, so a 1–2 GB cgroup (small k8s pods, CI containers) ends up with a block cache at half or all of its limit. This is not a regression — the fixed 1024 default had the same property — but this function is now the thing that claims to scale with memory, so it's the right place to close the gap:If the flat 1024 floor is deliberate, saying so in the doc comment above (and logging a warning when the floor exceeds a quarter of detected memory) would at least make it visible to operators of small containers.
Separately:
defaultCacheSizeMbis no longer the default, only the floor —minAutoCacheSizeMbwould pair withmaxAutoCacheSizeMband keep the doc comment on line 721 self-explanatory.