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.

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

KafkaListenerEndpointRegistry.stop(Runnable) does not call callback when there are no containers to stop #1134

Closed
wilkinsona opened this issue Jun 21, 2019 · 10 comments · Fixed by #1135

Comments

@wilkinsona
Copy link
Member

commented Jun 21, 2019

Boot's tests for its Kafka auto-configuration are taking 30 seconds each to run as we have a context with a KafkaListenerEndpointRegistry that has no MessageListenerContainers. This causes it to fail to call the callback that's passed into stop(Runnable) so we have to wait for Framework's 30 second timeout to pop.

@artembilan

This comment has been minimized.

Copy link
Member

commented Jun 21, 2019

Good catch, Andy!

Will fix soon...

@artembilan artembilan self-assigned this Jun 21, 2019

@artembilan

This comment has been minimized.

Copy link
Member

commented Jun 21, 2019

Do you need the fix until Spring Boot 1.5.x ?

artembilan added a commit to artembilan/spring-kafka that referenced this issue Jun 21, 2019
spring-projectsGH-1134: Fix `stop(Runnable)` usage
Fixes spring-projects#1134

We always have to call `callback` in the `stop(Runnable)` implementation
independently of the component state

**Cherry-pick until 1.1.x to support Spring Boot 1.5.x**
@wilkinsona

This comment has been minimized.

Copy link
Member Author

commented Jun 21, 2019

Boot 1.5 doesn't appear to be affected. For example, this test takes less than 1 second. That may be due to the tests being different in 1.5, I'm not sure.

@artembilan

This comment has been minimized.

Copy link
Member

commented Jun 21, 2019

Yeah... I think so. We will back-port any way until the version which is pulled by Initializr (seems for me 1.3.x), because we haven't committed into 1.1.x for a while already...

@garyrussell

This comment has been minimized.

Copy link
Member

commented Jun 21, 2019

Yeah I found and fixed this in my experimental reactive @KafkaListener because in that environment, the MLC list was empty.

https://github.com/spring-projects/spring-kafka/pull/1123/files#diff-f23a91d9c56afd55946555b6ec1a150fR331

garyrussell added a commit that referenced this issue Jun 21, 2019
GH-1134: Fix `stop(Runnable)` usage
Fixes #1134

We always have to call `callback` in the `stop(Runnable)` implementation
independently of the component state

**Cherry-pick until 1.1.x to support Spring Boot 1.5.x**
garyrussell added a commit that referenced this issue Jun 21, 2019
GH-1134: Fix `stop(Runnable)` usage
Fixes #1134

We always have to call `callback` in the `stop(Runnable)` implementation
independently of the component state

**Cherry-pick until 1.1.x to support Spring Boot 1.5.x**
garyrussell added a commit that referenced this issue Jun 21, 2019
GH-1134: Fix `stop(Runnable)` usage
Fixes #1134

We always have to call `callback` in the `stop(Runnable)` implementation
independently of the component state

**Cherry-pick until 1.1.x to support Spring Boot 1.5.x**
garyrussell added a commit that referenced this issue Jun 21, 2019
GH-1134: Fix `stop(Runnable)` usage
Fixes #1134

We always have to call `callback` in the `stop(Runnable)` implementation
independently of the component state

**Cherry-pick until 1.1.x to support Spring Boot 1.5.x**
garyrussell added a commit that referenced this issue Jun 21, 2019
GH-1134: Fix `stop(Runnable)` usage
Fixes #1134

We always have to call `callback` in the `stop(Runnable)` implementation
independently of the component state

**Cherry-pick until 1.1.x to support Spring Boot 1.5.x**
@wilkinsona

This comment has been minimized.

Copy link
Member Author

commented Jun 24, 2019

Thanks, both. What's your schedule for the next Spring Kafka maintenance releases? I'd like to switch to snapshots if you have releases planned before the next round of Boot releases. We have 2.2 M5 scheduled for 24 July. Given the Framework releases planned for 17 July, we'll probably have 1.5 and 2.1 releases around then as well.

@wilkinsona

This comment has been minimized.

Copy link
Member Author

commented Jun 24, 2019

I've figured out why we're only seeing the 30 second delay in Boot 2.2.

It is due to #1102. In Boot 2.1 (Spring Kafka 2.2), isRunning() only returns true if one of the MLCs is running. As there are no MLCs in the affected tests, it returns false. As a result, stop(Runnable) is never called and the 30 second delay doesn't occur.

@garyrussell

This comment has been minimized.

Copy link
Member

commented Jun 24, 2019

Thanks Andy, yes we'll have a new milestone (M4). I've scheduled it for July 23 (it's the day before Artem goes on extended PTO, so we may go a few days earlier - as long as SF stays where it is).

@garyrussell

This comment has been minimized.

Copy link
Member

commented Jun 26, 2019

@wilkinsona Do you plan to change the version in s-b-dependencies? (I just hit the problem while testing a patch and I see the version is still M3). Working around it with a version override for now.

Thanks.

@wilkinsona

This comment has been minimized.

Copy link
Member Author

commented Jun 26, 2019

Oops. Thanks for the reminder. I'd intended to move to snapshots and then got distracted by other things. I've just pushed the change: spring-projects/spring-boot@b67fedd.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.