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.2.x] storage: always schedule adjacent segment compaction #25160

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

Conversation

andrwng
Copy link
Contributor

@andrwng andrwng commented Feb 24, 2025

Backport of PR #24874

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

Backports Required

  • none - not a bug fix
  • none - this is a backport
  • none - issue does not exist in previous branches
  • none - papercut/not impactful enough to backport
  • v24.3.x
  • v24.2.x
  • v24.1.x

Release Notes

Improvements

  • Redpanda will now schedule local segment merges of compacted topics, even when windowed compaction has occurred in a given housekeeping round. This ensures progress in reducing segment count in compacted topics with high produce traffic.

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)
CONFLICT:
- this branch had code that takes a range instead of just the compaction
  config for compact_adjacent_segments()

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.2.x branch from c9c6a23 to 1eb7c2d Compare February 24, 2025 23:08
@andrwng andrwng self-assigned this Feb 24, 2025
@vbotbuildovich
Copy link
Collaborator

vbotbuildovich commented Feb 25, 2025

Retry command for Build#62227

please wait until all jobs are finished before running the slash command


/ci-repeat 1
tests/rptest/tests/idempotency_test.py::IdempotencyWriteCachingTest.test_idempotent_producers_write_caching
tests/rptest/tests/e2e_iam_role_test.py::ShortLivedCredentialsTests.test_short_lived_credentials

@vbotbuildovich
Copy link
Collaborator

CI test results

test results on build#62227
test_id test_kind job_url test_status passed
gtest_raft_rpunit.gtest_raft_rpunit unit https://buildkite.com/redpanda/redpanda/builds/62227#01953a3c-0647-4239-96ec-55095711d80f FLAKY 1/2
rptest.tests.consumer_group_balancing_test.ConsumerGroupBalancingTest.test_coordinator_nodes_balance ducktape https://buildkite.com/redpanda/redpanda/builds/62227#01953a85-bc5d-450e-a612-0f339898d367 FLAKY 5/6
rptest.tests.e2e_iam_role_test.ShortLivedCredentialsTests.test_short_lived_credentials ducktape https://buildkite.com/redpanda/redpanda/builds/62227#01953a85-bc5d-450e-a612-0f339898d367 FAIL 0/1
rptest.tests.idempotency_test.IdempotencyWriteCachingTest.test_idempotent_producers_write_caching ducktape https://buildkite.com/redpanda/redpanda/builds/62227#01953a85-6923-4643-9e27-b9ea7ffacbb9 FAIL 0/1
rptest.tests.upgrade_test.UpgradeBackToBackTest.test_upgrade_with_all_workloads.single_upgrade=False ducktape https://buildkite.com/redpanda/redpanda/builds/62227#01953a85-6922-48ac-8ab8-50afd8f00d91 FLAKY 5/6
storage_e2e_single_thread_rpunit.storage_e2e_single_thread_rpunit unit https://buildkite.com/redpanda/redpanda/builds/62227#01953a3c-0647-4239-96ec-55095711d80f FAIL 0/2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants