mirror of https://github.com/grpc/grpc-java.git
retries:Remove early commit for transparent retries with none remaining. (#10066)
* retries:Remove early commit for transparent retries when no retries or no hedging remain. Fixes #10011
This commit is contained in:
parent
b43ddc2886
commit
7255c8dfa8
|
|
@ -932,29 +932,15 @@ abstract class RetriableStream<ReqT> implements ClientStream {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
if (isHedging) {
|
if (isHedging) {
|
||||||
boolean commit = false;
|
|
||||||
synchronized (lock) {
|
synchronized (lock) {
|
||||||
// Although this operation is not done atomically with
|
// Although this operation is not done atomically with
|
||||||
// noMoreTransparentRetry.compareAndSet(false, true), it does not change the size() of
|
// noMoreTransparentRetry.compareAndSet(false, true), it does not change the size() of
|
||||||
// activeHedges, so neither does it affect the commitment decision of other threads,
|
// activeHedges, so neither does it affect the commitment decision of other threads,
|
||||||
// nor do the commitment decision making threads affect itself.
|
// nor do the commitment decision making threads affect itself.
|
||||||
state = state.replaceActiveHedge(substream, newSubstream);
|
state = state.replaceActiveHedge(substream, newSubstream);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// optimization for early commit
|
|
||||||
if (!hasPotentialHedging(state)
|
|
||||||
&& state.activeHedges.size() == 1) {
|
|
||||||
commit = true;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
if (commit) {
|
|
||||||
commitAndRun(newSubstream);
|
|
||||||
}
|
|
||||||
} else {
|
|
||||||
if (retryPolicy == null || retryPolicy.maxAttempts == 1) {
|
|
||||||
// optimization for early commit
|
|
||||||
commitAndRun(newSubstream);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
callExecutor.execute(new Runnable() {
|
callExecutor.execute(new Runnable() {
|
||||||
@Override
|
@Override
|
||||||
public void run() {
|
public void run() {
|
||||||
|
|
|
||||||
|
|
@ -1971,7 +1971,7 @@ public class RetriableStreamTest {
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void noRetry_transparentRetry_earlyCommit() {
|
public void noRetry_transparentRetry_noEarlyCommit() {
|
||||||
ClientStream mockStream1 = mock(ClientStream.class);
|
ClientStream mockStream1 = mock(ClientStream.class);
|
||||||
ClientStream mockStream2 = mock(ClientStream.class);
|
ClientStream mockStream2 = mock(ClientStream.class);
|
||||||
InOrder inOrder = inOrder(retriableStreamRecorder, mockStream1, mockStream2);
|
InOrder inOrder = inOrder(retriableStreamRecorder, mockStream1, mockStream2);
|
||||||
|
|
@ -1999,7 +1999,7 @@ public class RetriableStreamTest {
|
||||||
inOrder.verify(retriableStreamRecorder).newSubstream(0);
|
inOrder.verify(retriableStreamRecorder).newSubstream(0);
|
||||||
ArgumentCaptor<ClientStreamListener> sublistenerCaptor2 =
|
ArgumentCaptor<ClientStreamListener> sublistenerCaptor2 =
|
||||||
ArgumentCaptor.forClass(ClientStreamListener.class);
|
ArgumentCaptor.forClass(ClientStreamListener.class);
|
||||||
inOrder.verify(retriableStreamRecorder).postCommit();
|
verify(retriableStreamRecorder, never()).postCommit();
|
||||||
inOrder.verify(mockStream2).start(sublistenerCaptor2.capture());
|
inOrder.verify(mockStream2).start(sublistenerCaptor2.capture());
|
||||||
inOrder.verify(mockStream2).isReady();
|
inOrder.verify(mockStream2).isReady();
|
||||||
inOrder.verifyNoMoreInteractions();
|
inOrder.verifyNoMoreInteractions();
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue