Skip to content

Return ConsumerStoppedEvent with Abnormal reason when MLC stopped abn… #3949

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
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Return ConsumerStoppedEvent with Abnormal reason when MLC stopped abn… #3949

wants to merge 1 commit into from
+32 −2

Conversation

AlamuriLokesh
Copy link

…ormally

artembilan
artembilan requested changes Jun 6, 2025
View reviewed changes
Copy link
Member

@artembilan artembilan left a comment

Choose a reason for hiding this comment

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

Signed-off-by: AlamuriLokesh

Please, consider to fix your Git client to provide your official name.
According to DCO, the legal obligations for contributed code has to be from official name, not nicknames.

Otherwise we cannot review and accept your change.

@AlamuriLokesh AlamuriLokesh force-pushed the feature/GH-3948 branch 2 times, most recently from 515211d to e2626ed Compare June 6, 2025 14:54
@AlamuriLokesh
Copy link
Author 

Signed-off-by: AlamuriLokesh

Please, consider to fix your Git client to provide your official name. According to DCO, the legal obligations for contributed code has to be from official name, not nicknames.

Otherwise we cannot review and accept your change.

Could you please review now. I have updated my name as Alamuri Lokesh

artembilan
artembilan requested changes Jun 6, 2025
View reviewed changes
spring-kafka/src/main/java/org/springframework/kafka/event/ConsumerStoppedEvent.java
@@ -43,6 +43,11 @@ public enum Reason {
*/
NORMAL,

/**
* The consumer was stopped because the container was stopped abnormally.
Copy link
Member

Choose a reason for hiding this comment

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

Now this makes sense to me.
Thank you!

Please, add here @since 4.0

sobychacko
sobychacko requested changes Jun 6, 2025
View reviewed changes
Copy link
Contributor

@sobychacko sobychacko left a comment

Choose a reason for hiding this comment

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

Some minor comments. We need to add reference docs for this since this is a new addition.

spring-kafka/src/main/java/org/springframework/kafka/event/ConsumerStoppedEvent.java
@@ -43,6 +43,11 @@ public enum Reason {
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

Add your name as an @author.

spring-kafka/src/main/java/org/springframework/kafka/event/ConsumerStoppedEvent.java
@@ -43,6 +43,11 @@ public enum Reason {
*/
NORMAL,

/**
* The consumer was stopped because the container was stopped abnormally.
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

@since tag.

...ng-kafka/src/main/java/org/springframework/kafka/listener/KafkaMessageListenerContainer.java
@@ -537,6 +537,9 @@ else if (throwable instanceof AuthenticationException || throwable instanceof Au
else if (throwable instanceof NoOffsetForPartitionException) {
reason = Reason.NO_OFFSET;
}
else if (!this.isStoppedNormally()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Add your name as an @author to the class.

Copy link
Author

@AlamuriLokesh AlamuriLokesh Jun 6, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I have already contributed to this file using my personal email Id. Author name is Lokesh Alamuri. Please let me know if some changes are required.

...rc/test/java/org/springframework/kafka/listener/ConcurrentMessageListenerContainerTests.java
@@ -1088,11 +1088,29 @@ protected Consumer<Integer, String> createKafkaConsumer(String groupId, String c
assertThat(container.getContainers()).
doesNotContain(childContainer1);

container.getContainers().forEach(containerForEach -> containerForEach.stop());
KafkaMessageListenerContainer<Integer, String> childContainer0SecRun = container.getContainers().get(0);
Copy link
Contributor

Choose a reason for hiding this comment

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

Add your name as an @author.

Copy link
Author

Choose a reason for hiding this comment

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

I have already contributed to this file using my personal email Id. Author name is Lokesh Alamuri. Please let me know if some changes are required.

@sobychacko sobychacko added this to the 4.0.0-M3 milestone Jun 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sobychacko sobychacko Awaiting requested review from sobychacko

@artembilan artembilan Awaiting requested review from artembilan

Assignees
No one assigned
Labels
type: enhancement
Projects
None yet
Milestone
4.0.0-M3
Development

Successfully merging this pull request may close these issues.
3 participants
@AlamuriLokesh @sobychacko @artembilan