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

KAFKA-2989: system tests should verify partitions consumed after rebalancing #702

Closed
wants to merge 2 commits into from

Conversation

hachikuji
Copy link
Contributor

No description provided.

@@ -123,7 +123,7 @@ class VerifiableConsumer(BackgroundThreadService):
logs = {
"verifiable_consumer_stdout": {
"path": STDOUT_CAPTURE,
"collect_default": False},
"collect_default": True},
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this intentional or just for debugging? Verifiable consumer can generate a huge amount of output, so its pretty risky to save it, especially if we don't have a way to ensure it is at least compressed first. We definitely always need to grab it to do verification, but not necessarily save it by default with the output (which would happen even if the test was successful).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was intentional since it helps debugging quite a bit. The verifiable consumer only logs the start and end offsets of the batches of records it consumes, so the size is pretty reasonable. At the same time, it's probably unnecessary since I already increased offset logging in that other ticket.

@ewencp
Copy link
Contributor

ewencp commented Dec 22, 2015

@hachikuji A bunch of small questions, but overall this looks like a nice cleanup and extension. All the affected tests are passing cleanly for me here.

@hachikuji
Copy link
Contributor Author

@ewencp Ready for another look when you get a chance.

@ewencp
Copy link
Contributor

ewencp commented Dec 23, 2015

LGTM

@asfgit asfgit closed this in 976fa19 Dec 23, 2015
AnGg98 pushed a commit to AnGg98/kafka that referenced this pull request Jul 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants