Skip to content
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

[v24.3.x] storage: always schedule adjacent segment compaction #24956

Open
wants to merge 4 commits into
base: v24.3.x
Choose a base branch
from

Conversation

vbotbuildovich
Copy link
Collaborator

@vbotbuildovich vbotbuildovich commented Jan 28, 2025

Backport of PR #24874

Also pulls in #24880 which is a dependency of this change

@vbotbuildovich vbotbuildovich added this to the v24.3.x-next milestone Jan 28, 2025
@vbotbuildovich vbotbuildovich added the kind/backport PRs targeting a stable branch label Jan 28, 2025
@vbotbuildovich
Copy link
Collaborator Author

CI test results

test results on build#61273
test_id test_kind job_url test_status passed
rptest.tests.archive_retention_test.CloudArchiveRetentionTest.test_delete.cloud_storage_type=CloudStorageType.ABS.retention_type=retention.bytes ducktape https://buildkite.com/redpanda/redpanda/builds/61273#0194ab6b-eef6-4e28-a83a-8724bd481ce4 FLAKY 1/2
rptest.tests.consumer_group_balancing_test.ConsumerGroupBalancingTest.test_coordinator_nodes_balance ducktape https://buildkite.com/redpanda/redpanda/builds/61273#0194ab6b-eef6-4e28-a83a-8724bd481ce4 FLAKY 1/2
rptest.tests.consumer_group_balancing_test.ConsumerGroupBalancingTest.test_coordinator_nodes_balance ducktape https://buildkite.com/redpanda/redpanda/builds/61273#0194ab70-5a68-4034-8c28-bb3fd6635e09 FLAKY 1/2
rptest.transactions.producers_api_test.ProducersAdminAPITest.test_producers_state_api_during_load ducktape https://buildkite.com/redpanda/redpanda/builds/61273#0194ab6b-eef6-4e28-a83a-8724bd481ce4 FLAKY 1/2
storage_e2e_single_thread_rpunit.storage_e2e_single_thread_rpunit unit https://buildkite.com/redpanda/redpanda/builds/61273#0194ab29-bd26-4c20-8cf1-1d821aec3bec FAIL 0/2
storage_e2e_single_thread_rpunit.storage_e2e_single_thread_rpunit unit https://buildkite.com/redpanda/redpanda/builds/61273#0194ab29-bd27-4317-b345-53ef1c52c303 FAIL 0/2
storage_single_thread_rpunit.storage_single_thread_rpunit unit https://buildkite.com/redpanda/redpanda/builds/61273#0194ab29-bd26-4c20-8cf1-1d821aec3bec FLAKY 1/2

The test itself has many steps that make it hard to follow what has
happened. Adds some log messages to indicate different steps in the
test.

(cherry picked from commit 67689fa)
Adds a static constructor to return an optional interval, which per the
class comments, should represent an empty interval.

I intend on using this for some upcoming bounds checks.

(cherry picked from commit 8514bfd)
The methods to find offset range size currently use segment bounds to
determine whether a given offset is available to be queried. This
doesn't account for the case when the segment set contains offsets that
don't fall in the log's offset range (e.g. follow a delete records
request that trims mid-segment).

This commit adds appropriate bounds checks to both methods.

With an upcoming change to merge compact after windowed compaction,
test_offset_range_size2_compacted would fail because it would prefix
truncate mid-segment following a merge compaction, and then trip over
this, hitting an unexpected exception when creating a reader:

```
std::runtime_error: Reader cannot read before start of the log 0 < 887
```

(cherry picked from commit 69e4666)
We previously fell back on adjacent segment compaction only if there was
no new data to compact. In some situations, we've seen the rate of
incoming data outpace the compaction interval, causing segments to pile
up without ever being merged.

This change tweaks the logic to always run adjacent segment compaction
after running sliding window compaction.

Along the way, a couple tests needed to be tweaked to handle the fact
that housekeeping now may merge segments.

(cherry picked from commit 08d0433)
@andrwng andrwng force-pushed the backport-pr-24874-v24.3.x-55 branch from 48f35d7 to 01d95ed Compare February 10, 2025 21:34
@piyushredpanda
Copy link
Contributor

@andrwng need your review or someone else's review to merge this

@andrwng andrwng requested a review from dotnwat February 14, 2025 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/redpanda kind/backport PRs targeting a stable branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants