lianetm commented on code in PR #21495:
URL: https://github.com/apache/kafka/pull/21495#discussion_r2843579036
##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/AbstractMembershipManager.java:
##########
@@ -817,11 +817,13 @@ private void transitionToStale() {
* - There are topics that haven't been added to the current assignment
yet, but all their topic IDs
* are missing from the target assignment.
*
- * @param canCommit Controls whether reconciliation can proceed when
auto-commit is enabled.
- * Set to true only when the current offset positions are
safe to commit.
- * If false and auto-commit enabled, the reconciliation
will be skipped.
+ * @param invokedByPoll True if this reconciliation attempt is triggered
by the application thread on consumer.poll().
+ * False if this is triggered by the background
thread on regular manager poll.
+ * In both cases we want to resolve metadata to
unresolved assignments,
+ * but the actual reconciliation (commit, callbacks,
assignment updates)
+ * will only proceed if this is triggered from the
application thread on consumer.poll
*/
- public void maybeReconcile(boolean canCommit) {
+ public void maybeReconcile(boolean invokedByPoll) {
Review Comment:
The only comment related to this: the above explanation stands for the
asyncConsumer, all good there, but not for the ShareConsumer (shareConsumer
does not have this mechanism to wait for the background operations before
fetching). So we can move on with the fix for the AsyncConsumer in this PR, but
I'm leaving the ShareConsumer unchanged.
We need to decide separately if this is really an issue in the ShareConsumer
(not clear to me really, the APIs are very different in this area,
assignment()/seek()/position()). And if we do think we want to avoid assignment
updates without shareConsumer.poll, the ShareConsumer requires a separate fix
(block on SharePoll vs. same indirect waiting as the AsyncConsumer...to assess
and address in another PR if it's really needed)
I created this separate jira to review the ShareConsumer
https://issues.apache.org/jira/browse/KAFKA-20213
cc. @AndrewJSchofield
Makes sense @lucasbru? Updated the PR description to clarify this is fixing
only the AsyncConsumer (where we wanted to improve the behaviour for
assignment/seek/position really)
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
