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

Use empty list instead of nil when adding services (removes unnecessary churn) #3866

Closed
wants to merge 1 commit into from

Conversation

42wim
Copy link
Contributor

@42wim 42wim commented Feb 6, 2018

This fixes an issue introduced by previous PR #3845

When doing the service checks for services without Service Tags we get []string{} back from master, but the service was locally created with []string(nil) which is again a difference and causes services (without service tags) to get synced again every ~2 minutes

cc: @slackpad

@slackpad slackpad added this to the 1.0.5 milestone Feb 7, 2018
@slackpad
Copy link
Contributor

slackpad commented Feb 7, 2018

Thanks @42wim for this fix! Churn without tags is probably worse than churn with tags, so I think we are going to cut a 1.0.5 release tomorrow with this.

While I was testing I noticed that node-level checks also have churn now as well, so I'm going to expand the unit test coverage and try to nail all the combinations so we know we've got a complete fix for this.

@slackpad
Copy link
Contributor

slackpad commented Feb 7, 2018

Closing this in favor of #3867.

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.

2 participants