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

Attempt to fix 'Local: No offset stored message' from Kafka #42391

Merged
merged 3 commits into from
Feb 13, 2023

Conversation

filimonov
Copy link
Contributor

@filimonov filimonov commented Oct 17, 2022

Changelog category (leave one):

  • Not for changelog

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):

Attempt to fix 'Local: No offset stored message' from Kafka
See #12295

Test is needed

Information about CI checks: https://clickhouse.com/docs/en/development/continuous-integration/

@robot-ch-test-poll2 robot-ch-test-poll2 added the pr-improvement Pull request with some product improvements label Oct 17, 2022
@filimonov filimonov marked this pull request as draft October 17, 2022 12:45
@filimonov filimonov added the can be tested Allows running workflows for external contributors label Oct 17, 2022
@alexey-milovidov alexey-milovidov marked this pull request as ready for review January 29, 2023 17:03
Copy link
Member

@alexey-milovidov alexey-milovidov left a comment

Choose a reason for hiding this comment

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

The original exception should be saved for rethrowing.

Currently, the code is throwing a new exception and losing the context of the original.

@alexey-milovidov alexey-milovidov self-assigned this Jan 29, 2023
@filimonov
Copy link
Contributor Author

filimonov commented Jan 31, 2023

That particular exception is nothing what user can fix / is interested in. It's rather assert / logical error.

There is a code around that should avoid doing any commits if there no offset stored.

I think it's actually some internal issue in librdkafka (we have some offsets stored, but maybe not for every partition, and commit returns the exception from the partitions which had nothing to commit).

Actually that needs a test, to be sure that other partitions were properly committed.

Or we need to refactor code to work with every partition isolately (that can also can potentially resolve few other minor issues)

@alexey-milovidov
Copy link
Member

Or we need to refactor code to work with every partition isolately

This is what we should've done five years ago.

@alexey-milovidov alexey-milovidov merged commit 1b912c0 into master Feb 13, 2023
@alexey-milovidov alexey-milovidov deleted the filimonov-kafka-Local-No-offset-stored branch February 13, 2023 07:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
can be tested Allows running workflows for external contributors pr-improvement Pull request with some product improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants