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

[JBPM-9520] KafkaServerExtension poll is not freed #2342

Merged
merged 1 commit into from Dec 18, 2020

Conversation

fjtirado
Copy link
Contributor

@fjtirado fjtirado commented Dec 16, 2020

Using read write lock

JIRA:

JBPM-9520

@gmunozfe
Copy link
Member

jenkins retest this

@fjtirado
Copy link
Contributor Author

jenkins test this

if (processMessages()) {
removeTopics(topic2Message, deploymentId, processDefinition.getMessagesDesc());
}
removeTopics(topic2Signal, deploymentId, processDefinition.getSignalsDesc());
Copy link
Contributor Author

@fjtirado fjtirado Dec 17, 2020

Choose a reason for hiding this comment

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

This change tries to cover an edge case. System property enable when onDeploy/onActivate is called, but disable when onUndeploy/onDeactivate is invoked. This will leak to a subscripton "leak". In order to avoid that, at the cost of sacrificing a bit of performance (not much in any case), it is better to clean always

Copy link
Member

Choose a reason for hiding this comment

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

OK, agreed; it's very weird but not impossible. Anyway, performance impact is minimum and removing topics is during undeployment.

@elguardian
Copy link
Member

Jenkins retest this

1 similar comment
@elguardian
Copy link
Member

Jenkins retest this

@fjtirado
Copy link
Contributor Author

jenkins test this (and do not fail with gitub login if possible ;))

@fjtirado
Copy link
Contributor Author

jenkins test this

Copy link
Member

@gmunozfe gmunozfe left a comment

Choose a reason for hiding this comment

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

It worked fine, marvellous work @fjtirado !

if (processMessages()) {
removeTopics(topic2Message, deploymentId, processDefinition.getMessagesDesc());
}
removeTopics(topic2Signal, deploymentId, processDefinition.getSignalsDesc());
Copy link
Member

Choose a reason for hiding this comment

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

OK, agreed; it's very weird but not impossible. Anyway, performance impact is minimum and removing topics is during undeployment.

@fjtirado
Copy link
Contributor Author

jenkins test this

@sonarcloud
Copy link

sonarcloud bot commented Dec 18, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@elguardian elguardian merged commit 4894814 into kiegroup:master Dec 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants