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

MINOR: Fix the way total consumed is calculated for verifiable consumer #9143

Conversation

skaundinya15
Copy link
Contributor

Currently the way we calculate the number of total consumed messages for the verifiable consumer overcounts the number of actually consumed messages. This PR is to fix that to ensure we count the number of consumed messages correctly.

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

Copy link
Contributor

@rondagostino rondagostino left a comment

Choose a reason for hiding this comment

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

Wow, nice catch. LGTM.

Copy link
Contributor

@rajinisivaram rajinisivaram left a comment

Choose a reason for hiding this comment

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

@skaundinya15 Thanks for the PR, LGTM

@skaundinya15
Copy link
Contributor Author

Link for system test run for this branch: http://confluent-kafka-branch-builder-system-test-results.s3-us-west-2.amazonaws.com/2020-08-07--001.1596860596--skaundinya15--minor-fix-total-consumed-for-verifiable-consumer--d799e563e/report.html. The one test that might be related is streams_broker_compatibility_test but it looked like it failed on something unrelated, so I am rerunning that test to see if it it's okay.

@skaundinya15
Copy link
Contributor Author

I re-ran the system tests for streams_broker_compatibility_test here: http://confluent-kafka-branch-builder-system-test-results.s3-us-west-2.amazonaws.com/2020-08-10--001.1597088498--skaundinya15--minor-fix-total-consumed-for-verifiable-consumer--d799e563e/report.html and got a green build. Seems like anything dependent on this method/variable checks out ok for this.

@rajinisivaram
Copy link
Contributor

@skaundinya15 Thanks for running the tests, merging to trunk.

@rajinisivaram rajinisivaram merged commit f7a4fe7 into apache:trunk Aug 16, 2020
@skaundinya15 skaundinya15 deleted the minor-fix-total-consumed-for-verifiable-consumer branch August 17, 2020 05:26
urbandan pushed a commit to urbandan/kafka that referenced this pull request Oct 14, 2020
…er (apache#9143)

Reviewers: Ron Dagostino <rdagostino@confluent.io>, Rajini Sivaram <rajinisivaram@googlemail.com>
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