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

cloud_storage/ducktape: Wait for chunk observer to stop #14032

Merged

Conversation

abhijat
Copy link
Contributor

@abhijat abhijat commented Oct 9, 2023

Tests which use chunk observer should wait for it to stop after the stop command is issued, to avoid the underlying set changing while it is being read from.

FIXES #13233

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

Release Notes

  • none

Tests which use chunk observer should wait for it to stop after the stop
command is issued, to avoid the underlying set changing while it is
being read from.
@abhijat
Copy link
Contributor Author

abhijat commented Oct 9, 2023

/ci-repeat 10
skip-units
dt-repeat=100
tests/rptest/tests/cloud_storage_chunk_read_path_test.py::CloudStorageChunkReadTest.test_read_when_cache_smaller_than_segment_size

@abhijat abhijat marked this pull request as ready for review October 10, 2023 05:00
@@ -369,6 +369,9 @@ def test_read_when_cache_smaller_than_segment_size(self):

self._consume_baseline(timeout=180, max_msgs=read_count)
observe_cache_dir.stop()
observe_cache_dir.join(timeout=10)
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: this works, but it's a bit annoying to use. perhaps we could expose an interface a la with ObserveCacheDir(..) as observer: ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, we can make it into a context manager, if we end up adding another call site for this utility I will add it (right now there is just one)

@abhijat abhijat merged commit cdcb1b8 into redpanda-data:dev Oct 11, 2023
10 checks passed
@vbotbuildovich
Copy link
Collaborator

/backport v23.2.x

@gousteris
Copy link
Contributor

/backport v23.2.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
4 participants