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

@zhangkun83 zhangkun83 commented Mar 5, 2020

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.
@zhangkun83 zhangkun83 requested review from creamsoup and ejona86 Mar 5, 2020
ejona86
ejona86 approved these changes Mar 5, 2020
Copy link
Member

@ejona86 ejona86 left a comment

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
13 checks passed
@zhangkun83 zhangkun83 deleted the health_check_fix branch Mar 5, 2020
dfawley pushed a commit to dfawley/grpc-java that referenced this issue 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