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

xds: Support localities in multiple priorities #9683

Merged
merged 3 commits into from Nov 29, 2022

Conversation

temawi
Copy link
Contributor

@temawi temawi commented Nov 8, 2022

Additional logic to support for the same locality appearing under multiple priorities.

@temawi temawi requested a review from ejona86 November 8, 2022 21:20
@temawi temawi force-pushed the locality-in-multiple-pirorities branch from a24705b to e4b7195 Compare November 8, 2022 21:47
Additional logic to support for the same locality appearing under
multiple priorities.
@temawi temawi force-pushed the locality-in-multiple-pirorities branch from e4b7195 to fc7d9ab Compare November 8, 2022 22:15
Copy link
Contributor

@YifeiZhuang YifeiZhuang left a comment

Choose a reason for hiding this comment

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

Sorry I miss the bigger picture why moving the attribute to eag level from locality would support locality in multiple priories? Setting different priority or weight for each endpoint? Brief description of background and what's next would be great!

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.

I didn't look too deeply, but this looks about right. "The shape is right." And it might actually be correct, too.

@temawi
Copy link
Contributor Author

temawi commented Nov 18, 2022

Sorry I miss the bigger picture why moving the attribute to eag level from locality would support locality in multiple priories? Setting different priority or weight for each endpoint? Brief description of background and what's next would be great!

This change came out of a chat where it was realized that to be faithful with the xDS spec we need to support the same locality having different weights if it appears under more than one locality. The current code uses a single map from locality to priority so it would not work in this scenario - locality can only have one weight across all priorities. There are no next steps, just this tweak to the logic.

@temawi temawi merged commit 5cf54f3 into grpc:master Nov 29, 2022
@temawi temawi deleted the locality-in-multiple-pirorities branch November 29, 2022 21:15
temawi added a commit to temawi/grpc-java that referenced this pull request Feb 12, 2023
Additional logic to support for the same locality appearing under
multiple priorities.
temawi added a commit that referenced this pull request Feb 13, 2023
)

* xds: Support localities in multiple priorities (#9683)

Additional logic to support for the same locality appearing under
multiple priorities.

* protobuf: update external javadoc link (#9890) (#9891)
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 1, 2023
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

3 participants