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

Fix committed offset is always behind the real offset by 1 #33

Merged
merged 2 commits into from
Nov 16, 2020

Conversation

shamanu4
Copy link
Contributor

@shamanu4 shamanu4 commented Nov 16, 2020

Description

This PR fixes a bug, that committed offset is always less than log-end-offset in Kafka by 1 when all messages were processed. This bug causes 2 problems:

  • after the worker restart, the last message from the topic partition was re-send to the consumer. (Was fixed by this check in the _drain_messages function: if committed is None or offset > committed. )
  • if there was an offset reset in the partition to the latest, consumer lag is equal to 0. The next message that appears in this partition will be ignored by the consumer due to the previous check, in the _drain_messages function.

This PR increases any committing offset by 1, so the log-end-offset will be equal to 0 when all messages are read.

Fixes #34

@codecov-io
Copy link

Codecov Report

Merging #33 (361b09d) into master (7493b6e) will not change coverage.
The diff coverage is 0.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #33   +/-   ##
=======================================
  Coverage   63.96%   63.96%           
=======================================
  Files          99       99           
  Lines       10272    10272           
  Branches     1153     1153           
=======================================
  Hits         6570     6570           
  Misses       3547     3547           
  Partials      155      155           
Impacted Files Coverage Δ
faust/transport/consumer.py 40.22% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7493b6e...361b09d. Read the comment docs.

@patkivikram
Copy link
Collaborator

Can you please rebase your branch?

@shamanu4
Copy link
Contributor Author

Can you please rebase your branch?

Done

@patkivikram patkivikram merged commit 18230a7 into faust-streaming:master Nov 16, 2020
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.

First message is ignored after resetting Kafka log offsets to the latest offset
3 participants