-
Notifications
You must be signed in to change notification settings - Fork 546
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
Support d2 symlink node in indis flow #933
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bohhyang
force-pushed
the
bohan/supportSymlink
branch
from
September 20, 2023 00:56
1407003
to
072ce1d
Compare
PapaCharlie
requested changes
Sep 28, 2023
d2/src/main/java/com/linkedin/d2/xds/XdsToD2PropertiesAdaptor.java
Outdated
Show resolved
Hide resolved
d2/src/main/java/com/linkedin/d2/xds/XdsToD2PropertiesAdaptor.java
Outdated
Show resolved
Hide resolved
d2/src/main/java/com/linkedin/d2/xds/XdsToD2PropertiesAdaptor.java
Outdated
Show resolved
Hide resolved
d2/src/main/java/com/linkedin/d2/xds/XdsToD2PropertiesAdaptor.java
Outdated
Show resolved
Hide resolved
d2/src/main/java/com/linkedin/d2/xds/XdsToD2PropertiesAdaptor.java
Outdated
Show resolved
Hide resolved
d2/src/main/java/com/linkedin/d2/xds/XdsToD2PropertiesAdaptor.java
Outdated
Show resolved
Hide resolved
d2/src/main/java/com/linkedin/d2/xds/XdsToD2PropertiesAdaptor.java
Outdated
Show resolved
Hide resolved
d2/src/main/java/com/linkedin/d2/xds/XdsToD2PropertiesAdaptor.java
Outdated
Show resolved
Hide resolved
PapaCharlie
approved these changes
Oct 2, 2023
bohhyang
force-pushed
the
bohan/supportSymlink
branch
from
October 2, 2023 18:13
9779117
to
be8c607
Compare
shivamgupta1
approved these changes
Oct 2, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Background
We need to support D2 symlink (d2 single master/primary feature, wiki) in INDIS flow.
The current ZK flow handles symlinks implicitly in SymlinkAwareZookeeper, which captures the data of a symlink node, immediately get the data of the actual master node, and send the fetched data to the symlink node callback. It can't be shared with INDIS flow.
For symlink clusters, the event bus subscribers, like "ClusterLoadBalancerSubscriber", "UriLoadBalancerSubscriber" subscribe to the symlinks ($FooClusterMaster), instead of the actual master node (FooCluster-prod-ltx1), we need to publish to event bus under the symlink names.
For other clusters, we need to publish under its original name. Note that these clusters could be either:
Changes
Subscribe to and handle d2 symlink node data in xDSClient and the xDS-to-d2 data adaptor.
Test Done
build and unit tests.
Verified with Toki qei deployment that the normal service, cluster, and uri nodes can be fetched successfully.
Will need to test fetching the symlink node with d2-proxy after release (due to SI-34767, the snapshot can't build with d2-proxy).