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

core: fix a bug in health check config propgation. #6804

Merged
merged 1 commit into from Mar 5, 2020

Conversation

zhangkun83
Copy link
Contributor

The condition "effectiveServiceConfig != validServiceConfig" should
have been deleted in commit 2162ad0.

The condition was there before that commit because
NAME_RESOLVER_SERVICE_CONFIG was already in "attrs", thus it needed to
be re-added only if "effectiveServiceConfig" differs from the original
"validServiceConfig".

In contrast, ATTR_HEALTH_CHECKING_CONFIG is not in the original
"attrs" and always needs to be added.

The condition "effectiveServiceConfig != validServiceConfig" should
have been deleted in commit 2162ad0.

The condition was there before that commit because
NAME_RESOLVER_SERVICE_CONFIG was already in "attrs", thus it needed to
be re-added only if "effectiveServiceConfig" differs from the original
"validServiceConfig".

In contrast, ATTR_HEALTH_CHECKING_CONFIG is not in the original
"attrs" and always needs to be added.
Copy link
Member

@ejona86 ejona86 left a comment

Choose a reason for hiding this comment

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

Arg. I was watching that condition earlier, but then didn't check it in the end...

@zhangkun83 zhangkun83 merged commit 4a2c5d6 into grpc:master Mar 5, 2020
@zhangkun83 zhangkun83 deleted the health_check_fix branch March 5, 2020 17:28
dfawley pushed a commit to dfawley/grpc-java that referenced this pull request Jan 15, 2021
The condition "effectiveServiceConfig != validServiceConfig" should
have been deleted in commit 2162ad0.

The condition was there before that commit because
NAME_RESOLVER_SERVICE_CONFIG was already in "attrs", thus it needed to
be re-added only if "effectiveServiceConfig" differs from the original
"validServiceConfig".

In contrast, ATTR_HEALTH_CHECKING_CONFIG is not in the original
"attrs" and always needs to be added.
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants