grpc-js: Implement EDS load balancer #1507
Conversation
There was a problem hiding this comment.
I have reviewed the logic of the big updateChild() function. It started with the information received from the watcher this.latestEdsUpdate.endpoints. And then a series of operation to re-arrange that information to a more of a hierarchy/tree that's being understood by the childBalancer via its .updateAddressList(addressList, childConfig, latestAttributes) call.
There is a separate concern back in the XdsClient class, on what if we add a 2nd watcher on the same EdsServiceName. What if the first watcher already received an update, but now the 2nd watcher should have received the same information, but updateEdsNames method won't be called again. In the core XdsClient class there seems to be some caching done.
| this.isWatcherActive = true; | ||
| } | ||
|
|
||
| this.updateChild(); |
There was a problem hiding this comment.
Discussed offline, but perhaps we can consider adding a comment here describing why there is an extra updateChild() call here, in addition to the one inside watcher.onValidUpdate().
| this.xdsClient = attributes.xdsClient; | ||
| this.edsServiceName = lbConfig.eds.edsServiceName ?? lbConfig.eds.cluster; | ||
|
|
||
| if (!this.isWatcherActive) { |
There was a problem hiding this comment.
As discussed offline, what if this function updateAddressList is called with a different lbConfig.eds.edsServiceName? After the first call, this.isWatcherActive is set to true. Will the 2nd call not get this.xdsClient.addEndpointWatcher() called?
Or maybe I have misunderstood the semantics - maybe edsServiceName won't change?
There was a problem hiding this comment.
I have changed this behavior to handle the name changing.
This is implemented on top of #1489. The changes for this PR are the last 2 commits, and they affect the files
load-balancer-eds.tsandload-balancing-config.ts.