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

Agent state: do not deregister service checks twice #6168

Merged
merged 1 commit into from
Jan 17, 2020

Conversation

ShimmerGlass
Copy link
Contributor

@ShimmerGlass ShimmerGlass commented Jul 18, 2019

Deregistering a service from the catalog automatically deregisters its
checks, however the agent still performs a deregister call for each
service checks even after the service has been deregistered.
With ACLs enabled this results in logs like:
"message:consul: "Catalog.Deregister" RPC failed to server
server_ip:8300: rpc error making call: rpc error making call: Unknown
check 'check_id'"
This change removes associated checks from the agent state when
deregistering a service, which results in less calls to the servers and
supresses the error logs.

@ShimmerGlass
Copy link
Contributor Author

FYI for us this error produces ~5K logs / hour

@ShimmerGlass ShimmerGlass force-pushed the check-deregister branch 2 times, most recently from c7e2567 to 7893220 Compare July 18, 2019 13:55
@ShimmerGlass
Copy link
Contributor Author

Bump, could you take a look at this PR ?

@banks banks added this to the Upcoming milestone Sep 3, 2019
@hanshasselberg hanshasselberg self-assigned this Dec 18, 2019
Copy link
Member

@hanshasselberg hanshasselberg left a comment

Choose a reason for hiding this comment

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

Nice catch! This looks good to me! Could you provide a test that demonstrates deleteService works as expected?

@hanshasselberg hanshasselberg added the waiting-reply Waiting on response from Original Poster or another individual in the thread label Dec 19, 2019
Deregistering a service from the catalog automatically deregisters its
checks, however the agent still performs a deregister call for each
service checks even after the service has been deregistered.
With ACLs enabled this results in logs like:
"message:consul: "Catalog.Deregister" RPC failed to server
server_ip:8300: rpc error making call: rpc error making call: Unknown
check 'check_id'"
This change removes associated checks from the agent state when
deregistering a service, which results in less calls to the servers and
supresses the error logs.
@stale stale bot removed the waiting-reply Waiting on response from Original Poster or another individual in the thread label Jan 13, 2020
@ShimmerGlass
Copy link
Contributor Author

@i0rek There is already many test cases for deregistration. Do you need other ones ?

Copy link
Member

@hanshasselberg hanshasselberg left a comment

Choose a reason for hiding this comment

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

Nice catch! Thanks for your work!

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.

3 participants