-
Notifications
You must be signed in to change notification settings - Fork 589
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
storage: various space management non-functional changes #18314
storage: various space management non-functional changes #18314
Commits on May 8, 2024
-
storage: rename log_manager::collection_threshold
I found the name a bit vague in trying to understand what this code was doing. This updates it to lowest_ts_to_retain(), and updates the disk_space_manager to use it rather than duplicating the logic.
Configuration menu - View commit details
-
Copy full SHA for d1ba0ee - Browse repository at this point
Copy the full SHA d1ba0eeView commit details -
resource_mgmt: clarify variable names in manage_disk_space
target_excess -> adjusted_target_excess: I find the name more intuitive, despite being more verbose at call sites, as a reader I prefer the reminder that the value has been tweaked
Configuration menu - View commit details
-
Copy full SHA for 0597b47 - Browse repository at this point
Copy the full SHA 0597b47View commit details -
resource_mgmt: tweak info log in space management
I found the existing logging a bit confusing because "tiered storage retention" isn't widely used verbiage. This tweaks it to refer to this as "space management of tiered storage topics". Also s/recover/remove: it feels a bit clearer, segment removal isn't referred to as "recovering" in other contexts.
Configuration menu - View commit details
-
Copy full SHA for c8eb9e7 - Browse repository at this point
Copy the full SHA c8eb9e7View commit details -
storage: rename maybe_override_retention_config()
The previous name is pretty vague: as a reader it's unclear what it's actually doing other than modifying the config in some way. Updates it to explicitly mention that it is handling local storage overrides. IMO this is easier to reason about when reading the method and its call sites.
Configuration menu - View commit details
-
Copy full SHA for 986264e - Browse repository at this point
Copy the full SHA 986264eView commit details -
storage: rename override_retention_config
Similar to maybe_override_retention_config(), updates override_retention_config() to be called apply_local_storage_overrides().
Configuration menu - View commit details
-
Copy full SHA for fded43a - Browse repository at this point
Copy the full SHA fded43aView commit details -
storage: rename apply_base_overrides
Renames apply_base_overrides() to apply_kafka_retention_overrides(). I think it's a bit easier to infer what the Kafka retention overrides are (i.e. the ones that are Kafka compatible: retention.ms, retention.bytes). Others considered but not taken: - total_retention - full_retention - consumable_retention - topic_retention
Configuration menu - View commit details
-
Copy full SHA for de686ce - Browse repository at this point
Copy the full SHA de686ceView commit details -
storage: refactor retention_adjust_timestamps
Pulls out a couple instances of duplicate code, in an effort to clarify naming of various overrides and adjustments done around retention.
Configuration menu - View commit details
-
Copy full SHA for e81fc13 - Browse repository at this point
Copy the full SHA e81fc13View commit details
Commits on May 10, 2024
-
storage: tweak strict_retention condition for clarity
With the original code, it was easy to get tripped up by the nots and ors. Tweaks the implementation and adds a comment to make this easier to read. No functional changes.
Configuration menu - View commit details
-
Copy full SHA for 6705a4a - Browse repository at this point
Copy the full SHA 6705a4aView commit details -
resource_mgmt: add clarity to info+ logs
We have seen issues where the lack of tiered storage resulted in disk filling up. The info logs didn't make it clear what was happening and it was assumed to be a bug in code, rather than a misconfiguration issue. This commit bumps a log line to info level so that it would have been clear that a specific error path wasn't being taken. Also tweaks wording to makes a stronger connection to tiered storage.
Configuration menu - View commit details
-
Copy full SHA for 9425c18 - Browse repository at this point
Copy the full SHA 9425c18View commit details