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

Add parsing of v1 compaction index footers #8555

Merged
merged 2 commits into from
Feb 7, 2023

Conversation

ztlpn
Copy link
Contributor

@ztlpn ztlpn commented Feb 1, 2023

A followup to #7687

An unfortunate side effect of introducing v2 compaction footers was that perfectly good indices with v1 footers would have to be rebuilt and the segment re-compacted (after restart we have to verify index integrity to determine it the segment is already compacted and indices with v1 footers failed the version check). This PR adds compatibility code for v1 footer parsing.

Also added an end-to-end upgrade test that verifies that the new version doesn't re-compact segments processed by the old version, and the old version correctly re-compacts the segments compacted by the new version.

Backports Required

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

Release Notes

  • none

An unfortunate side effect of introducing v2 compaction footers was that
perfectly good indices with v1 footers would have to be rebuilt (after
restart we have to verify index integrity to verify that the segment is
already compacted and indices with v1 footers failed the version check).
This commit adds compatibility code for v1 footer parsing.
map(lambda p: len(p.segments) > count and p.recovered(),
partitions))
class CompactionRecoveryUpgradeTest(RedpandaTest):
OLD_VERSION = (22, 2, 8)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think latest redpanda_installer changes might have included the ability to just say (22, 2) and get the latest version from that series

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But then if we backport and release this changes in a 22.2 release, the test will stop working, right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's true, but I don't think we will backport it to 22.2 -- format changes like this are for major releases.

seg2mtime_1 = get_compacted_segment2mtime(to_restart)
assert len(seg2mtime_1) >= 3

self.installer.install([to_restart], RedpandaInstaller.HEAD)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should upgrade via the intervening versions: jumps straight from 22.2 up to HEAD might not work when head is some future release

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So what is the canonical way of doing this? I've browsed through #7310 , looks like it is still under discussion.

We could test just with upgrading specific versions but then it is kind of useless in dev.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, there is some missing infrastructure here:

I don't think this has to be a blocker for this PR, but it will need revisiting later to make this test sustainable.

jcsp
jcsp previously approved these changes Feb 1, 2023
Copy link
Contributor

@jcsp jcsp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code changes LGTM - left comments on where the upgrade test could be tightened up

@ztlpn ztlpn force-pushed the test-new-compaction-index-footer branch from 9e387de to 467b931 Compare February 6, 2023 23:03
@ztlpn
Copy link
Contributor Author

ztlpn commented Feb 7, 2023

/ci-repeat 1
skip-units
dt-repeat=20
tests/rptest/tests/compaction_recovery_test.py

@ztlpn ztlpn requested a review from jcsp February 7, 2023 13:25
@ztlpn
Copy link
Contributor Author

ztlpn commented Feb 7, 2023

Force-push: deflake the ducktape test.

@jcsp jcsp merged commit 1d371cf into redpanda-data:dev Feb 7, 2023
@ztlpn ztlpn deleted the test-new-compaction-index-footer branch November 27, 2023 13:26
This pull request was closed.
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