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.

Sign up for GitHub

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

Jump to bottom

Fix CommonErrorHandler for asyncAcks and AckMode.MANUAL #2677

Closed
garyrussell opened this issue May 8, 2023 Discussed in #2668 · 0 comments · Fixed by #2678
Closed

Fix CommonErrorHandler for asyncAcks and AckMode.MANUAL #2677

garyrussell opened this issue May 8, 2023 Discussed in #2668 · 0 comments · Fixed by #2678
Assignees
@garyrussell
Labels
backport 2.9.x (obsolete) type: bug
Milestone
3.0.7

Comments

@garyrussell
Copy link
Contributor

Discussed in #2668

Originally posted by linus-learns April 27, 2023
Hi,

I'm running a setup with @KafkaListener annotated consumers using a ConcurrentKafkaListenerContainerFactory setup with asyncAcks and AckMode.MANUAL. I have tried to implement a CommonErrorHandler for my setup with isAckAfterHandle set to true and seeksAfterHandling set to false, but the bookkeeping of the deferred acks does not work, as the the ack after the error handler has been invoked and the broken record handled is not done through the ackInOrder method. This causes the consumer to hang if a poison pill appears in the middle of the batch and higher offset commits are waiting in the ListenerConsumer.
It appears that the ack for the poison pill is never commited until the consumer get's the shutdown signal, as a rebalance causes the broken record to be fetched by the new partition owner but a restart resolves the issue.
Is this a bug, or should I use a different way of handling my errors given my setup?
@garyrussell garyrussell added this to the 3.0.7 milestone May 8, 2023
@garyrussell garyrussell self-assigned this May 8, 2023
garyrussell added a commit to garyrussell/spring-kafka that referenced this issue May 8, 2023
@garyrussell
spring-projectsGH-2677: Fix EH AckAfterHandle With Async Acks
e63fc7f 
Resolves spring-projects#2677

When an error handler handles an error (and `ackAfterHandle` is true),
the ack bypassed the out of order commit logic, causing the consumer
to be paused indefinitely, due to the missing ack.

**cherry-pick to 2.9.x**
@garyrussell garyrussell mentioned this issue May 8, 2023
Merged
artembilan pushed a commit that referenced this issue May 8, 2023
@garyrussell
GH-2677: Fix EH AckAfterHandle With Async Acks
3b01a79 
Resolves #2677

When an error handler handles an error (and `ackAfterHandle` is true),
the ack bypassed the out of order commit logic, causing the consumer
to be paused indefinitely, due to the missing ack.

**cherry-pick to 2.9.x**
@artembilan artembilan mentioned this issue May 8, 2023
Closed
artembilan pushed a commit that referenced this issue May 8, 2023
@garyrussell @artembilan
GH-2677: Fix EH AckAfterHandle With Async Acks
8725b65 
Resolves #2677

When an error handler handles an error (and `ackAfterHandle` is true),
the ack bypassed the out of order commit logic, causing the consumer
to be paused indefinitely, due to the missing ack.

**cherry-pick to 2.9.x**
# Conflicts:
#	spring-kafka/src/main/java/org/springframework/kafka/listener/KafkaMessageListenerContainer.java
@garyrussell garyrussell changed the title CommonErrorHandler for asyncAcks and AckMode.MANUAL Fix CommonErrorHandler for asyncAcks and AckMode.MANUAL May 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@garyrussell garyrussell

Labels
backport 2.9.x (obsolete) type: bug
Projects
None yet
Milestone
3.0.7
Development

Successfully merging a pull request may close this issue.

GH-2677: Fix EH AckAfterHandle With Async Acks garyrussell/spring-kafka
1 participant
@garyrussell