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

cst: Change acceptable query miss condition #18574

Conversation

abhijat
Copy link
Contributor

@abhijat abhijat commented May 18, 2024

When a query for an offset misses, it is possible that this happens because the manifest was entirely truncated. Truncation usually happens in two steps, first the start offset is advanced and later on the manifest segments are removed.

If the query falls between these two steps, the current condition to accept a query miss as a non-error may fail, because it uses the kafka log start offset. If the manifest start offset has been advanced beyond the entire manifest span, the kafka offset is now not calculatable, which results in nullopt kafka start, which does not match the condtion and the query miss is recorded as an error.

The change detects the condition where the manifest is fully truncated and the query offset lies within truncated range, if so the query is considered not to have failed.

FIXES #18454

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

Release Notes

  • none

When a query for an offset misses, it is possible that this happens
because the manifest was entirely truncated. Truncation usually happens
in two steps, first the start offset is advanced and later on the
manifest segments are removed.

If the query falls between these two steps, the current condition to
accept a query miss as a non-error may fail, because it uses the kafka
log start offset. If the manifest start offset has been advanced beyond
the entire manifest span, the kafka offset is now not calculatable,
which results in nullopt kafka start, which does not match the condtion
and the query miss is recorded as an error.

The change detects the condition where the manifest is fully truncated
and the query offset lies within truncated range, if so the query is
considered not to have failed.
@abhijat
Copy link
Contributor Author

abhijat commented May 18, 2024

Looking at the merge conflict, the changes in this PR go against the changes in #18097 in terms of introducing lenience when accepting errors, specifically d1543ee

Since the changes in #18097 move towards making the behavior of init_cursor stricter and the changes in this PR make the same behavior more lenient, it makes sense to not go ahead with changes in this PR.

@nvartolomei
Copy link
Contributor

@abhijat I believe I fixed this in #18097

@abhijat
Copy link
Contributor Author

abhijat commented May 20, 2024

@abhijat I believe I fixed this in #18097

Yes, I will close this PR

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.

CI Failure (key symptom) in ShadowIndexingWhileBusyTest.test_create_or_delete_topics_while_busy
2 participants