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

Transaction Improvements #656

Closed
garyrussell opened this issue Apr 18, 2018 · 0 comments
Closed

Transaction Improvements #656

garyrussell opened this issue Apr 18, 2018 · 0 comments
Assignees
@garyrussell
Milestone
2.1.6

Comments

@garyrussell
Copy link
Contributor

invokeRecordListenerInTx only seeks the current failed record - it should seek all unprocessed records.

Also, some strategy is needed to discard the poison record and continue. See #653 (comment)
@garyrussell garyrussell mentioned this issue Apr 19, 2018
Closed
@garyrussell garyrussell self-assigned this Apr 19, 2018
@garyrussell garyrussell added this to the 2.1.6 milestone Apr 19, 2018
garyrussell added a commit to garyrussell/spring-kafka that referenced this issue Apr 19, 2018
@garyrussell
spring-projectsGH-656: Fix seek on rollback
53a2a62 
Fixes spring-projects#656
Fixes spring-projects#657

Previously, after a rollback, we only performed a `seek` on the failed record.
We need to seek for all unprocessed records.

Also, when no error handler was provided, and using a batch listener, the
offsets were added to `acks` and incorrectly committed. (spring-projects#657).

Enhance the tests to verify full seeks.
Add a new test to verify the batch listener doesn't commit after a roll back.

**cherry-pick to 2.1.x, 2.0.x** I will backport to 1.3.x after review.
garyrussell added a commit to garyrussell/spring-kafka that referenced this issue Apr 19, 2018
@garyrussell
spring-projectsGH-656: Fix seek on rollback
f79ddd9 
Fixes spring-projects#656
Fixes spring-projects#657

Previously, after a rollback, we only performed a `seek` on the failed record.
We need to seek for all unprocessed records.

Also, when no error handler was provided, and using a batch listener, the
offsets were added to `acks` and incorrectly committed. (spring-projects#657).

Also, if a `ContainerAwareErrorHandler` "handles" the error, the offsets weren't
comitted.

Enhance the tests to verify full seeks.
Add a new test to verify the batch listener doesn't commit after a roll back.

**cherry-pick to 2.1.x, 2.0.x** I will backport to 1.3.x after review.
garyrussell added a commit to garyrussell/spring-kafka that referenced this issue Apr 19, 2018
@garyrussell
spring-projectsGH-656: Fix seek on rollback
5379204 
Fixes spring-projects#656
Fixes spring-projects#657

Previously, after a rollback, we only performed a `seek` on the failed record.
We need to seek for all unprocessed records.

Also, when no error handler was provided, and using a batch listener, the
offsets were added to `acks` and incorrectly committed. (spring-projects#657).

Also, if a `ContainerAwareErrorHandler` "handles" the error, the offsets weren't
comitted.

Enhance the tests to verify full seeks.
Add a new test to verify the batch listener doesn't commit after a roll back.

**cherry-pick to 2.1.x, 2.0.x** I will backport to 1.3.x after review.
@garyrussell garyrussell mentioned this issue Apr 19, 2018
Closed
artembilan pushed a commit that referenced this issue Apr 19, 2018
@garyrussell @artembilan
GH-656: Fix seek on rollback
9612e88 
Fixes #656
Fixes #657

Previously, after a rollback, we only performed a `seek` on the failed record.
We need to seek for all unprocessed records.

Also, when no error handler was provided, and using a batch listener, the
offsets were added to `acks` and incorrectly committed. (#657).

Also, if a `ContainerAwareErrorHandler` "handles" the error, the offsets weren't
committed.

Enhance the tests to verify full seeks.
Add a new test to verify the batch listener doesn't commit after a roll back.

**cherry-pick to 2.1.x, 2.0.x** I will backport to 1.3.x after review.

* Some simple polishing

# Conflicts:
#	spring-kafka/src/main/java/org/springframework/kafka/config/AbstractKafkaListenerContainerFactory.java
#	spring-kafka/src/test/java/org/springframework/kafka/listener/TransactionalContainerTests.java
#	src/reference/asciidoc/whats-new.adoc
artembilan pushed a commit that referenced this issue Apr 19, 2018
@garyrussell @artembilan
GH-656: Fix seek on rollback
439f2a5 
Fixes #656
Fixes #657

Previously, after a rollback, we only performed a `seek` on the failed record.
We need to seek for all unprocessed records.

Also, when no error handler was provided, and using a batch listener, the
offsets were added to `acks` and incorrectly committed. (#657).

Also, if a `ContainerAwareErrorHandler` "handles" the error, the offsets weren't
committed.

Enhance the tests to verify full seeks.
Add a new test to verify the batch listener doesn't commit after a roll back.

**cherry-pick to 2.1.x, 2.0.x** I will backport to 1.3.x after review.

* Some simple polishing

# Conflicts:
#	spring-kafka/src/main/java/org/springframework/kafka/config/AbstractKafkaListenerContainerFactory.java
#	spring-kafka/src/test/java/org/springframework/kafka/listener/TransactionalContainerTests.java
#	src/reference/asciidoc/whats-new.adoc

# Conflicts:
#	spring-kafka/src/main/java/org/springframework/kafka/listener/AbstractMessageListenerContainer.java
#	spring-kafka/src/main/java/org/springframework/kafka/listener/KafkaMessageListenerContainer.java
#	src/reference/asciidoc/kafka.adoc
#	src/reference/asciidoc/whats-new.adoc

* Resolve errors for code which doesn't exist yet
garyrussell added a commit to garyrussell/spring-kafka that referenced this issue Apr 19, 2018
@garyrussell
spring-projectsGH-656: Fix seek on rollback
8ed559d 
Fixes spring-projects#656
Fixes spring-projects#657

Previously, after a rollback, we only performed a `seek` on the failed record.
We need to seek for all unprocessed records.

Also, when no error handler was provided, and using a batch listener, the
offsets were added to `acks` and incorrectly committed. (spring-projects#657).

Also, if a `ContainerAwareErrorHandler` "handles" the error, the offsets weren't
committed.

Enhance the tests to verify full seeks.
Add a new test to verify the batch listener doesn't commit after a roll back.

**cherry-pick to 2.1.x, 2.0.x** I will backport to 1.3.x after review.

* Some simple polishing
@garyrussell garyrussell mentioned this issue Apr 19, 2018
Closed
artembilan pushed a commit that referenced this issue Apr 20, 2018
@garyrussell @artembilan
GH-656: Fix seek on rollback
e826a58 
Fixes #656
Fixes #657

Previously, after a rollback, we only performed a `seek` on the failed record.
We need to seek for all unprocessed records.

Also, when no error handler was provided, and using a batch listener, the
offsets were added to `acks` and incorrectly committed. (#657).

Also, if a `ContainerAwareErrorHandler` "handles" the error, the offsets weren't
committed.

Enhance the tests to verify full seeks.
Add a new test to verify the batch listener doesn't commit after a roll back.

**cherry-pick to 2.1.x, 2.0.x** I will backport to 1.3.x after review.

* Some simple polishing
* Remove `@FunctionalInterface` since it's not for Java 7
* Refactor `DefaultAfterRollbackProcessor` to avoid extra loop
denis554 added a commit to denis554/spring-kafka that referenced this issue Mar 27, 2019
@denis554
GH-656: Fix seek on rollback
843f5dc 
Fixes spring-projects/spring-kafka#656
Fixes spring-projects/spring-kafka#657

Previously, after a rollback, we only performed a `seek` on the failed record.
We need to seek for all unprocessed records.

Also, when no error handler was provided, and using a batch listener, the
offsets were added to `acks` and incorrectly committed. (#657).

Also, if a `ContainerAwareErrorHandler` "handles" the error, the offsets weren't
committed.

Enhance the tests to verify full seeks.
Add a new test to verify the batch listener doesn't commit after a roll back.

**cherry-pick to 2.1.x, 2.0.x** I will backport to 1.3.x after review.

* Some simple polishing
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@garyrussell garyrussell

Labels
None yet
Projects
None yet
Milestone
2.1.6
Development

No branches or pull requests
1 participant
@garyrussell