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

Add Integration tests for the the KIP-966 #15759

Open
wants to merge 2 commits into
base: trunk
Choose a base branch
from

Conversation

CalvinConfluent
Copy link
Contributor

@CalvinConfluent
Copy link
Contributor Author

@mumrah @artemlivshits Can you help take a look?

Comment on lines +264 to +266
Thread.sleep(1000);
records = consumer.poll(Duration.ofSeconds(1L));
assertEquals(0, records.count());
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we should just check that produce with acks=all fails with the not enough replicas error. Trying to consume the records adds time to the test run (the sleep is real time not mock time) and doesn't guarantee that we didn't consume the record yet because HWM didn't move and not for some other reason (e.g. cluster is just slow).

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 is a valid concern. But I don't have an idea how to get around this.
With ack=all, the producer request will be rejected. Then we don't know if restoring the min ISR can unblock the HWM with no offsets to be forwarded.

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