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

rptest: do not expect cached segment readers at the end of the test #23509

Merged

Conversation

nvartolomei
Copy link
Contributor

This assertion fails from time to time. It seems perfectly fine there not to be any cached readers available.

If there aren't any running queries using readers and we are under memory pressure the next query will likely evict all segment readers to free up resources. Reader eviction (and memory accounting) is asynchronous but the metric/probe update is synchronous.

If we get metrics in-between last query acquiring memory resources (or slightly after) and before the actual physical eviction happened then we expect the metric to be 0.

It is possible for the last query to be aborted altogether in-between requesting resources and actually creating a reader which would also lead to metric being 0.

I consider this acceptable behavior so removing the assert.

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

Release Notes

  • none

This assertion fails from time to time. It seems perfectly fine there
not to be any cached readers available.

If there aren't any running queries using readers and we are under
memory pressure the next query will likely evict all segment readers to
free up resources. Reader eviction (and memory accounting) is
asynchronous but the metric/probe update is synchronous.

If we get metrics in-between last query acquiring memory resources (or
slightly after) and before the actual physical eviction happened then we
expect the metric to be 0.

It is possible for the last query to be aborted altogether in-between
requesting resources and actually creating a reader which would also
lead to metric being 0.

I consider this acceptable behavior so removing the assert.
Copy link
Member

@dotnwat dotnwat left a comment

Choose a reason for hiding this comment

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

makes sense. definitely testing some internal dynamic behavior. i think if want a test specifically for readers cache behavior we should have that separate?

@nvartolomei nvartolomei merged commit bdfbdd1 into redpanda-data:dev Sep 27, 2024
17 checks passed
@vbotbuildovich
Copy link
Collaborator

/backport v24.2.x

@vbotbuildovich
Copy link
Collaborator

/backport v24.1.x

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

Successfully merging this pull request may close these issues.

4 participants