-
Notifications
You must be signed in to change notification settings - Fork 3k
Correct setting kafka graceful-shutdown property for dev and test profiles #49861
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
base: main
Are you sure you want to change the base?
Conversation
74af53c
to
e4002a8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
Actually I need to check this, Channel annotations can be injected incoming channels too.
Status for workflow
|
@ozangunalp my understanding is that you decided it was all good, can we approve and merge? (just checking) |
No, please let me check the change locally and get back here asap |
Ok, looking closer at the code, there are multiple things: While we apply Secondly, doing this only based on I think better way is to do it based on A not-so pseudo-code would look like : // visible for testing
void disableGracefulShutdown(List<ConnectorManagedChannelBuildItem> channelsManagedByConnectors,
BuildProducer<RunTimeConfigurationDefaultBuildItem> defaultConfigProducer,
DefaultSerdeDiscoveryState discoveryState) {
for (ConnectorManagedChannelBuildItem managed : channelsManagedByConnectors) {
String channelName = managed.getName();
boolean incoming = managed.getDirection() == INCOMING;
if (!discoveryState.isKafkaConnector(channelsManagedByConnectors, incoming, channelName)) {
continue;
}
String key = getChannelPropertyKey(channelName, incoming ? "graceful-shutdown" : "close-timeout", incoming);
discoveryState.ifNotYetConfigured(key, () -> {
defaultConfigProducer.produce(new RunTimeConfigurationDefaultBuildItem(key, incoming ? "false" : "0"));
});
}
}
@RoMiRoSSaN would this work for you? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Proposed a cleaner way
@ozangunalp Hi! Yes, I think it's not bad. It's just that in the current code, |
Fix for a minor bug that only exists in development or testing mode.
In some cases that are extremely difficult to reproduce, a situation occurs where channels are not formed correctly. For example, channel-in, channel-out are recognized as belonging to 1 specific input channel.
The problem is that 1 common list of
Incoming
,Outgoing
andChannel
annotations was formed, and the graceful-shutdown setting was set for them as for incoming channels.According to the documentation only incoming channels have such a setting, but not for outgoing ones.
Therefore, you can remove the setting of the property for outgoing channels.