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

[Bug][Connector-v2][KafkaSource]Fix KafkaConsumerThread exit caused by commit offset error. #4379

Merged
merged 3 commits into from
Mar 24, 2023

Conversation

lightzhao
Copy link
Contributor

Purpose of this pull request

When starting to consume kafka for the first time, since the initialized offset is -1, an exception will occur when submitting the kafka offset, resulting in KafkaConsumerThread not catching the exception, causing the thread to exit and unable to consume subsequent messages.
image

Check list

@lightzhao
Copy link
Contributor Author

Now this running workflow needs to be approved to run? Please approve the operation.

Comment on lines 60 to 65
} catch (InterruptedException e) {
throw new KafkaConnectorException(
KafkaConnectorErrorCode.CONSUME_THREAD_RUN_ERROR, e);
} catch (Exception e) {
throw new KafkaConnectorException(
KafkaConnectorErrorCode.CONSUME_THREAD_RUN_ERROR, e);
Copy link
Member

Choose a reason for hiding this comment

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

If Exception is used here, does the above catch still make sense?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it doesn't make sense, thanks for the reminder.

@lightzhao lightzhao requested a review from liugddx March 21, 2023 11:48
@liugddx
Copy link
Member

liugddx commented Mar 21, 2023

CI is error, there may be some problem to deal with

@lightzhao
Copy link
Contributor Author

CI is error, there may be some problem to deal with

This CI detection feels unstable, and this kind of problem occurs from time to time, and I don’t know how to deal with it. I pulled the latest code and submitted it. Local compilation is also normal. Have you encountered it? How is it handled?

@liugddx
Copy link
Member

liugddx commented Mar 22, 2023

CI is error, there may be some problem to deal with

This CI detection feels unstable, and this kind of problem occurs from time to time, and I don’t know how to deal with it. I pulled the latest code and submitted it. Local compilation is also normal. Have you encountered it? How is it handled?

Wating for #4391 done.

@lightzhao
Copy link
Contributor Author

CI is error, there may be some problem to deal with

This CI detection feels unstable, and this kind of problem occurs from time to time, and I don’t know how to deal with it. I pulled the latest code and submitted it. Local compilation is also normal. Have you encountered it? How is it handled?

Wating for #4391 done.

ok.

@lightzhao
Copy link
Contributor Author

CI is error, there may be some problem to deal with

All checks have passed, PTAL.

@liugddx liugddx merged commit 71f4d0c into apache:dev Mar 24, 2023
@lightzhao lightzhao deleted the kafka-consumer-thread-exception branch March 24, 2023 08:41
MonsterChenzhuo pushed a commit to MonsterChenzhuo/incubator-seatunnel that referenced this pull request Apr 19, 2023
…y commit offset error. (apache#4379)

* Fix KafkaConsumerThread exit caused by commit offset error.

* Remove redundant exception catching.

---------

Co-authored-by: lightzhao <zhaolianyong777@gmail.com>
ic4y pushed a commit to ic4y/incubator-seatunnel that referenced this pull request May 22, 2023
…y commit offset error. (apache#4379)

* Fix KafkaConsumerThread exit caused by commit offset error.

* Remove redundant exception catching.

---------

Co-authored-by: lightzhao <zhaolianyong777@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants