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: handle 100% drop for fallback mode #5784
xds: handle 100% drop for fallback mode #5784
Conversation
dapengzhang0
commented
May 23, 2019
- Cancel fallback timer and/or exit fallback mode once receiving an EDS response indicating 100% drop.
- Also update balancing state once receiving the first EDS response with drop information when the channel is at the initial IDLE state.
…andle-drop-for-fallback
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with minor comments.
@@ -285,6 +285,9 @@ private void updatePicker(ConnectivityState state, List<WeightedChildPicker> ch | |||
|
|||
if (!dropOverloads.isEmpty()) { | |||
picker = new DroppablePicker(dropOverloads, picker, random); | |||
if (state == null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should probably add a @Nullable
to the state
argument
@@ -274,8 +278,17 @@ public PickResult pickSubchannel(PickSubchannelArgs args) { | |||
verify(random, never()).nextInt(1000_000); | |||
} | |||
|
|||
@Test | |||
public void updateLoaclityStore_withEmptyDropList() { | |||
localityStore.updateDropPercentage(ImmutableList.<DropOverload>of()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't empty list the default value already? Should updateLoaclityStore()
be renamed to updateLoaclityStore_withEmptyDropList()
?
verify(localityStore, times(2)).updateLocalityStore(localityEndpointsMappingCaptor.capture()); | ||
verify(adsStreamCallback, times(1)).onWorking(); | ||
verify(adsStreamCallback, never()).onAllDrop(); | ||
verify(adsStreamCallback, never()).onError(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can the above two lines be replaced by verifyNoMoreInteractions()
? (Ditto for the same cases below)