storage: add cache target estimates and space management control loop skeleton #11133
Conversation
dotnwat
force-pushed
the
space-management2
branch
from
3bc6a08
to
fb0e895
Compare
|
Force-push:
|
| auto [size, chunked] = seg->min_cache_cost(); | ||
| if (chunked) { | ||
| return cache_usage{ | ||
| .target_min_bytes = min_chunks * size, | ||
| .target_bytes = wanted_chunks * size, | ||
| }; | ||
| } else { | ||
| return cache_usage{ | ||
| .target_min_bytes = min_segments * size, | ||
| .target_bytes = wanted_segments * size, | ||
| }; | ||
| } |
There was a problem hiding this comment.
nit: consider making min_cache_cost() return the usage, to avoid this chunked out bool? Especially if there's any appetite for making min_* and wanted_* configurable, it seems pretty natural to make the segment encapsulate this
src/v/resource_mgmt/storage.cc
Outdated
| co_await _gate.close(); | ||
| } | ||
|
|
||
| ss::future<> disk_space_manager::controller() { |
There was a problem hiding this comment.
nit: could we avoid using "controller"? Maybe run_loop or something? Especially if this at some point makes its way into cluster/
| , enable_storage_space_manager( | ||
| *this, | ||
| "enable_storage_space_manager", | ||
| "Enable the storage space manager that coordinates and control space " | ||
| "usage between log data and the cloud storage cache.", | ||
| {.needs_restart = needs_restart::no, .visibility = visibility::user}, | ||
| true) |
There was a problem hiding this comment.
I don't think the behavior you're implementing warrant doing anything differently in the face of skew, but I'm curious if you've given thought to whether this should be configurable/controllable at the node level. A few scenarios come to mind when space usage may vary across nodes
- when there is some hypothetical retention bug that consumes the space on a a single node but not others
- when there is heterogeneous hardware
- when there is severe partition-size skew
Just thinking whether during an incident it'll be desirable to have a way to flip this off for one but not all nodes.
There was a problem hiding this comment.
this makes a lot of sense. i'd even assume that we'd want to be able to turn it off on a per node basis without needing a restart. at least i think that we dont' allow that later feature so we'd need an admin endpoint.
There was a problem hiding this comment.
I've added a ticket to track this here. It's a good idea: #11192
| @@ -1101,4 +1103,79 @@ materialized_segments& remote_partition::materialized() { | |||
| return _api.materialized(); | |||
| } | |||
|
|
|||
| cache_usage remote_partition::get_cache_usage() const { | |||
There was a problem hiding this comment.
Naming: something other than usage will be easier to understand (usually usage means how much space we're using, but this is really asking for target usage, or some other noun if we can think of one)
dotnwat
force-pushed
the
space-management2
branch
from
fb0e895
to
be24755
Compare
Signed-off-by: Noah Watkins <noahwatkins@gmail.com>
A segment may be in legacy mode or chunked mode which will affect the way that it interacts with the cloud cache. This patch adds an interface for exposing this aspect of the state of the segment. Signed-off-by: Noah Watkins <noahwatkins@gmail.com>
Adds an interface for estimating the target minimum / wanted sizes for a remote partition in the cloud storage cache. Signed-off-by: Noah Watkins <noahwatkins@gmail.com>
Signed-off-by: Noah Watkins <noahwatkins@gmail.com>
Introduces a new disk space manager control loop that has access to both the cloud storage cache as well as log storage for the purposes of managing free space. This commit only introduces the control loop and prints storage statistics collected from the subsystem using our new APIs at debug level. Signed-off-by: Noah Watkins <noahwatkins@gmail.com>
dotnwat
force-pushed
the
space-management2
branch
from
be24755
to
ed387c6
Compare
|
Force push 1:
Force push 2:
|
|
Ping @andrwng |
Adds a cloud cache interface for estimating the target minimum for storage size, as well as a control loop skeleton which will be used to manage disk space to be added in subsequent PR.
Backports Required
Release Notes