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

[v22.3.x] cloud_storage: Read path hardening #9872

Merged

Conversation

Lazin
Copy link
Contributor

@Lazin Lazin commented Apr 6, 2023

Backports #9865

In the restored partition the delta-offset values of uploaded segments will reset and stop being monotonic. This breaks our read path because the offset_translator_state instance is crated per fetch request and every fetch request can read from more than one segment. If the partition reader reads from two segments and the second segment has delta offset which is lower than delta offset of the previous segment the offset_translator_state will throw.

To avoid this the reader detects this situation and exits early or resets the offset_translator_state.

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
  • v23.1.x
  • v22.3.x
  • v22.2.x

Release Notes

Bug Fixes

  • Fix offset translation failure that could be triggered after topic recovery

Lazin added 2 commits April 6, 2023 11:12
Method removes all items fromt he offset translator.

(cherry picked from commit 26be2ba)
...translation inconsistencies. The problem can arise if the partition
was restored. In this case the segment uploaded by the new cluster will
have 'offset_delta' field smaller than in the segments uploaded by the
old cluster.

The fix updates read path to handle this situation by stopping iteration
when the reader reaches the 'seam' between data uploaded by the old
cluster and the new cluster. The reader uses two strategies:
- if no data was produced by the reader it just resets the ot_state;
- otherwise it stops current reader and returns;

(cherry picked from commit 37d5283)
Copy link
Contributor

@VladLazar VladLazar left a comment

Choose a reason for hiding this comment

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

Looks good, but could we backport the test too?

@Lazin Lazin requested review from VladLazar and andrwng April 6, 2023 11:27
@Lazin
Copy link
Contributor Author

Lazin commented Apr 6, 2023

ScalingUpTest strikes back

@Lazin Lazin merged commit 73e6222 into redpanda-data:v22.3.x Apr 6, 2023
@BenPope BenPope added this to the v22.3.16 milestone Apr 13, 2023
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.

3 participants