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

instance termination plugin support #1134

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

adyach
Copy link
Member

@adyach adyach commented Dec 23, 2019

One-line summary

it should be possible to notify nakadi about coming instance termination, that it can release used resources and prepare itself for termination. A good example would be subscription connection termination in clean way, that consumer will not wait commit timeout

PR is not compilable due to plugin api change, which is expected here zalando-nakadi/nakadi-plugin-api#10

@RequestMapping(method = GET)
public ResponseEntity<String> healthCheck() {
if (terminationService.isTerminating()) {
return ResponseEntity.status(HttpStatus.IM_A_TEAPOT_418).build();
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this meant to be used by Nakadi Proxy? When maintaining the list of healthy Nakadi instances, those who report 418 during health check will not be added to the final result but at the same time not marked as unhealthy?

@@ -493,11 +505,15 @@ public Builder setKpiCollectionFrequencyMs(final long kpiCollectionFrequencyMs)
return this;
}

public Builder setTerminationService(final TerminationService terminationService) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I see that this is set also in the constructor. Is this used or needed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants