cli: add --experimental-secondary-cache flag#111143
cli: add --experimental-secondary-cache flag#111143craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
|
It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR? 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
RaduBerinde
left a comment
There was a problem hiding this comment.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @itsbilal, @jbowens, and @joshimhoff)
pkg/cli/flags.go line 492 at r1 (raw file):
cliflagcfg.VarFlag(f, &serverCfg.StorageEngine, cliflags.StorageEngine) cliflagcfg.StringFlag(f, &serverCfg.SharedStorage, cliflags.SharedStorage) cliflagcfg.IntFlag(f, &serverCfg.SecondaryCache, cliflags.SecondaryCache)
Flag is documented as accepting GiB, I don't think that's true with IntFlag?
902ed23 to
bf0e6f0
Compare
itsbilal
left a comment
There was a problem hiding this comment.
TFTR!
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @jbowens, @joshimhoff, and @RaduBerinde)
pkg/cli/flags.go line 492 at r1 (raw file):
Previously, RaduBerinde wrote…
Flag is documented as accepting
GiB, I don't think that's true with IntFlag?
Good catch. Now it supports GiB.
RaduBerinde
left a comment
There was a problem hiding this comment.
Reviewable status:
complete! 1 of 0 LGTMs obtained (waiting on @itsbilal, @jbowens, and @joshimhoff)
pkg/cli/cliflags/flags.go line 1045 at r2 (raw file):
--experimental-secondary-cache=20GB -> 20000000000 bytes --experimental-secondary-cache=20GiB -> 21474836480 bytes --experimental-secondary-cache=0.02TiB -> 21474836480 bytes
[nit] This value is not correct, it's more like 21990232555 bytes. I'd just remove the example.
This change adds a new `--experimental-secondary-cache` flag for use with `--experimental-shared-storage` to enable the use of a secondary cache to speed up reads of objects in shared storage. This option sets the max cache size for each store using disaggregated storage; for per-store granularity, pebble options can also be used. Epic: none Release note: None
bf0e6f0 to
6d0643c
Compare
itsbilal
left a comment
There was a problem hiding this comment.
TFTR!
Reviewable status:
complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @jbowens, @joshimhoff, and @RaduBerinde)
pkg/cli/cliflags/flags.go line 1045 at r2 (raw file):
Previously, RaduBerinde wrote…
[nit] This value is not correct, it's more like 21990232555 bytes. I'd just remove the example.
Done.
|
bors r=RaduBerinde |
|
Build failed (retrying...): |
|
Build failed (retrying...): |
|
Build failed (retrying...): |
|
Build succeeded: |
This change adds a new
--experimental-secondary-cacheflag for use with--experimental-shared-storageto enable the use of a secondary cache to speed up reads of objects in shared storage. This option sets the max cache size for each store using disaggregated storage; for per-store granularity, pebble options can also be used.Epic: none
Release note: None