-
Notifications
You must be signed in to change notification settings - Fork 258
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
#6549 Fix kafka consumer initial seek position #6712
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Nigel Jones <nigel.l.jones+git@gmail.com>
planetf1
requested review from
lpalashevski,
mandy-chessell and
davidradl
and removed request for
mandy-chessell
July 8, 2022 12:45
...java/org/odpi/openmetadata/adapters/eventbus/topic/kafka/KafkaOpenMetadataEventConsumer.java
Fixed
Show resolved
Hide resolved
davidradl
reviewed
Jul 8, 2022
...java/org/odpi/openmetadata/adapters/eventbus/topic/kafka/KafkaOpenMetadataEventConsumer.java
Outdated
Show resolved
Hide resolved
...java/org/odpi/openmetadata/adapters/eventbus/topic/kafka/KafkaOpenMetadataEventConsumer.java
Outdated
Show resolved
Hide resolved
...java/org/odpi/openmetadata/adapters/eventbus/topic/kafka/KafkaOpenMetadataEventConsumer.java
Outdated
Show resolved
Hide resolved
Example log with the info messages - covering multiple topics in the same threads, with a mix of threads without messages, and some with
|
...ava/org/odpi/openmetadata/adapters/eventbus/topic/kafka/KafkaOpenMetadataTopicConnector.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Nigel Jones <nigel.l.jones+git@gmail.com>
…ts on topic ->null Signed-off-by: Nigel Jones <nigel.l.jones+git@gmail.com>
Signed-off-by: Nigel Jones <nigel.l.jones+git@gmail.com>
Signed-off-by: Nigel Jones <nigel.l.jones+git@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Signed-off-by: Nigel Jones nigel.l.jones+git@gmail.com
Description
Corrects an issue where Cohort registration events could be missed due to the delay between the kafka topic connector being started (and running asynchronously) and the first poll() being done to receive messages in the case where no topic offset was stored (ie new consumer/install).
Detail of fix
By default Kafka will store the offset that a client last read from for it's consumer group (this should be constant for a persistent server).
Sometimes there is no offset stored. In this case it will default to the 'last' message (kafka properties can set it to first instead)
Unfortunately this setting of 'last' message is done a number of seconds after the Kafka topic connector has started -- not until we get to the first poll() which is typically around 5s later. The result is that we miss messages in this window. In a production environment this probably won't be noticed, but it occurs consistently in our lab/test environments and CTS (before a random 'sleep' was added)
This fix hooks off the 'PartitionAssignment' event -- until then we don't know what offset kafka thinks it wants to use.
We look at the ~ the time the connector was started (actually the constructor was called), and save this as our latest desired read time. When we get the event above IF the current offset is later than this we rewind a little - and this is BEFORE any messages are read by a poll(). We only attempt this once. If anything goes wrong we revert to the default behaviour, and we also only do this on the first partition assignment, since after that kafka should be managing the offset correctly.
Related Issue(s)
Fixes: #6549
Testing
Tested with coco Pharma environment on k8s
verified offset is corrected if older ie
NOTE currently testing the case where we restart a server and consumer group exists
Release Notes & Documentation
Will add ref to release notes
Additional notes