You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by "opershai (via GitHub)" <gi...@apache.org> on 2023/04/25 21:41:19 UTC
[GitHub] [camel] opershai opened a new pull request, #9932: [CAMEL-18985] Updated `SyncCommitManager` to perform offsets auto-commit only if it's enabled.
opershai opened a new pull request, #9932:
URL: https://github.com/apache/camel/pull/9932
Bug CAMEL-18985 is caused by [SyncCommitManager.commit()](https://github.com/apache/camel/blob/camel-3.20.x/components/camel-kafka/src/main/java/org/apache/camel/component/kafka/consumer/SyncCommitManager.java#L48) method which doesn't have logic to check whether auto-commit is enabled. As result of this [KafkaConsumer.commitSync()](https://kafka.apache.org/33/javadoc/org/apache/kafka/clients/consumer/KafkaConsumer.html#commitSync()) method is getting called in scope of the [KafkaFetchRecords.startPolling()](https://github.com/apache/camel/blob/camel-3.20.x/components/camel-kafka/src/main/java/org/apache/camel/component/kafka/KafkaFetchRecords.java#L343) method. The `KafkaConsumer.commitSync()` method commits offsets returned on the *last* `poll()` for all the subscribed list of topics and partitions regardless of the fact whether events have been successfully processed or not. It's undesirable behaviour, because the component is configured with `autoCommitEnable=false`, `allowMan
ualCommit=true` and `breakOnFirstError=true` to guarantee "at least once" events processing.
The fix goes inline with existing logic in the [AsyncCommitManager.commit()](https://github.com/apache/camel/blob/camel-3.20.x/components/camel-kafka/src/main/java/org/apache/camel/component/kafka/consumer/AsyncCommitManager.java#L50) method which respects the auto-commit configuration. Basically, the same logic is added to the [SyncCommitManager.commit()](https://github.com/apache/camel/blob/camel-3.20.x/components/camel-kafka/src/main/java/org/apache/camel/component/kafka/consumer/SyncCommitManager.java#L48) as part of the fix.
Also the auto-commit configuration was respected for sync/async/noop commits in the class `DefaultCommitManager` which was removed as part of the [refactoring](https://github.com/apache/camel/commit/2d46024a7836e7b24dce7257a17d4f9499c48f7b).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] orpiske commented on pull request #9932: [CAMEL-18985] Updated `SyncCommitManager` to perform offsets auto-commit only if it's enabled.
Posted by "orpiske (via GitHub)" <gi...@apache.org>.
orpiske commented on PR #9932:
URL: https://github.com/apache/camel/pull/9932#issuecomment-1523041841
> Do we need this fix in 3.x and main too?
Yes. 3.18, 3.x and 4.x. I'll back-port them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] github-actions[bot] commented on pull request #9932: [CAMEL-18985] Updated `SyncCommitManager` to perform offsets auto-commit only if it's enabled.
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9932:
URL: https://github.com/apache/camel/pull/9932#issuecomment-1522990173
:star2: Thank you for your contribution to the Apache Camel project! :star2:
:warning: Please note that the changes on this PR may be **tested automatically**.
If necessary Apache Camel Committers may access logs and test results in the job summaries!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] orpiske commented on pull request #9932: [CAMEL-18985] Updated `SyncCommitManager` to perform offsets auto-commit only if it's enabled.
Posted by "orpiske (via GitHub)" <gi...@apache.org>.
orpiske commented on PR #9932:
URL: https://github.com/apache/camel/pull/9932#issuecomment-1522988105
LGTM, but I have to test this one manually.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] github-actions[bot] commented on pull request #9932: [CAMEL-18985] Updated `SyncCommitManager` to perform offsets auto-commit only if it's enabled.
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9932:
URL: https://github.com/apache/camel/pull/9932#issuecomment-1522458143
:star2: Thank you for your contribution to the Apache Camel project! :star2:
:warning: Please note that the changes on this PR may be **tested automatically**.
If necessary Apache Camel Committers may access logs and test results in the job summaries!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] opershai commented on pull request #9932: [CAMEL-18985] Updated `SyncCommitManager` to perform offsets auto-commit only if it's enabled.
Posted by "opershai (via GitHub)" <gi...@apache.org>.
opershai commented on PR #9932:
URL: https://github.com/apache/camel/pull/9932#issuecomment-1523067067
Thank you @orpiske and @oscerd !
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] orpiske merged pull request #9932: [CAMEL-18985] Updated `SyncCommitManager` to perform offsets auto-commit only if it's enabled.
Posted by "orpiske (via GitHub)" <gi...@apache.org>.
orpiske merged PR #9932:
URL: https://github.com/apache/camel/pull/9932
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org