Skip to content

Commit

Permalink
all: add internal API to disable retry stats (#8510)
Browse files Browse the repository at this point in the history
Resolves b/197648853 for internal performance regression. Reporting retry stats caused significant amount of performance overhead internally.
  • Loading branch information
dapengzhang0 committed Sep 13, 2021
1 parent 9ff5405 commit 7c6f53a
Show file tree
Hide file tree
Showing 9 changed files with 46 additions and 19 deletions.
13 changes: 9 additions & 4 deletions census/src/main/java/io/grpc/census/CensusStatsModule.java
Original file line number Diff line number Diff line change
Expand Up @@ -88,19 +88,21 @@ final class CensusStatsModule {
private final boolean recordStartedRpcs;
private final boolean recordFinishedRpcs;
private final boolean recordRealTimeMetrics;
private final boolean recordRetryMetrics;

/**
* Creates a {@link CensusStatsModule} with the default OpenCensus implementation.
*/
CensusStatsModule(Supplier<Stopwatch> stopwatchSupplier,
boolean propagateTags, boolean recordStartedRpcs, boolean recordFinishedRpcs,
boolean recordRealTimeMetrics) {
boolean recordRealTimeMetrics, boolean recordRetryMetrics) {
this(
Tags.getTagger(),
Tags.getTagPropagationComponent().getBinarySerializer(),
Stats.getStatsRecorder(),
stopwatchSupplier,
propagateTags, recordStartedRpcs, recordFinishedRpcs, recordRealTimeMetrics);
propagateTags, recordStartedRpcs, recordFinishedRpcs, recordRealTimeMetrics,
recordRetryMetrics);
}

/**
Expand All @@ -111,7 +113,7 @@ final class CensusStatsModule {
final TagContextBinarySerializer tagCtxSerializer,
StatsRecorder statsRecorder, Supplier<Stopwatch> stopwatchSupplier,
boolean propagateTags, boolean recordStartedRpcs, boolean recordFinishedRpcs,
boolean recordRealTimeMetrics) {
boolean recordRealTimeMetrics, boolean recordRetryMetrics) {
this.tagger = checkNotNull(tagger, "tagger");
this.statsRecorder = checkNotNull(statsRecorder, "statsRecorder");
checkNotNull(tagCtxSerializer, "tagCtxSerializer");
Expand All @@ -120,6 +122,7 @@ final class CensusStatsModule {
this.recordStartedRpcs = recordStartedRpcs;
this.recordFinishedRpcs = recordFinishedRpcs;
this.recordRealTimeMetrics = recordRealTimeMetrics;
this.recordRetryMetrics = recordRetryMetrics;
this.statsHeader =
Metadata.Key.of("grpc-tags-bin", new Metadata.BinaryMarshaller<TagContext>() {
@Override
Expand Down Expand Up @@ -521,7 +524,9 @@ void recordFinishedCall() {
} else if (inboundMetricTracer != null) {
inboundMetricTracer.recordFinishedAttempt();
}

if (!module.recordRetryMetrics) {
return;
}
long retriesPerCall = 0;
long attempts = attemptsPerCall.get();
if (attempts > 0) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,16 @@ private InternalCensusStatsAccessor() {
public static ClientInterceptor getClientInterceptor(
boolean recordStartedRpcs,
boolean recordFinishedRpcs,
boolean recordRealTimeMetrics) {
boolean recordRealTimeMetrics,
boolean recordRetryMetrics) {
CensusStatsModule censusStats =
new CensusStatsModule(
STOPWATCH_SUPPLIER,
true, /* propagateTags */
recordStartedRpcs,
recordFinishedRpcs,
recordRealTimeMetrics);
recordRealTimeMetrics,
recordRetryMetrics);
return censusStats.getClientInterceptor();
}

Expand All @@ -71,11 +73,13 @@ public static ClientInterceptor getClientInterceptor(
boolean propagateTags,
boolean recordStartedRpcs,
boolean recordFinishedRpcs,
boolean recordRealTimeMetrics) {
boolean recordRealTimeMetrics,
boolean recordRetryMetrics) {
CensusStatsModule censusStats =
new CensusStatsModule(
tagger, tagCtxSerializer, statsRecorder, stopwatchSupplier,
propagateTags, recordStartedRpcs, recordFinishedRpcs, recordRealTimeMetrics);
propagateTags, recordStartedRpcs, recordFinishedRpcs, recordRealTimeMetrics,
recordRetryMetrics);
return censusStats.getClientInterceptor();
}

Expand All @@ -92,7 +96,8 @@ public static ServerStreamTracer.Factory getServerStreamTracerFactory(
true, /* propagateTags */
recordStartedRpcs,
recordFinishedRpcs,
recordRealTimeMetrics);
recordRealTimeMetrics,
false);
return censusStats.getServerTracerFactory();
}

Expand All @@ -111,7 +116,7 @@ public static ServerStreamTracer.Factory getServerStreamTracerFactory(
CensusStatsModule censusStats =
new CensusStatsModule(
tagger, tagCtxSerializer, statsRecorder, stopwatchSupplier,
propagateTags, recordStartedRpcs, recordFinishedRpcs, recordRealTimeMetrics);
propagateTags, recordStartedRpcs, recordFinishedRpcs, recordRealTimeMetrics, false);
return censusStats.getServerTracerFactory();
}
}
12 changes: 6 additions & 6 deletions census/src/test/java/io/grpc/census/CensusModulesTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ public void setUp() throws Exception {
censusStats =
new CensusStatsModule(
tagger, tagCtxSerializer, statsRecorder, fakeClock.getStopwatchSupplier(),
true, true, true, false /* real-time */);
true, true, true, false /* real-time */, true);
censusTracing = new CensusTracingModule(tracer, mockTracingPropagationHandler);
}

Expand Down Expand Up @@ -400,7 +400,7 @@ private void subtestClientBasicStatsDefaultContext(
CensusStatsModule localCensusStats =
new CensusStatsModule(
tagger, tagCtxSerializer, statsRecorder, fakeClock.getStopwatchSupplier(),
true, recordStarts, recordFinishes, recordRealTime);
true, recordStarts, recordFinishes, recordRealTime, true);
CensusStatsModule.CallAttemptsTracerFactory callAttemptsTracerFactory =
new CensusStatsModule.CallAttemptsTracerFactory(
localCensusStats, tagger.empty(), method.getFullMethodName());
Expand Down Expand Up @@ -514,7 +514,7 @@ public void recordRetryStats() {
CensusStatsModule localCensusStats =
new CensusStatsModule(
tagger, tagCtxSerializer, statsRecorder, fakeClock.getStopwatchSupplier(),
true, true, true, true);
true, true, true, true, true);
CensusStatsModule.CallAttemptsTracerFactory callAttemptsTracerFactory =
new CensusStatsModule.CallAttemptsTracerFactory(
localCensusStats, tagger.empty(), method.getFullMethodName());
Expand Down Expand Up @@ -908,7 +908,7 @@ private void subtestStatsHeadersPropagateTags(boolean propagate, boolean recordS
tagCtxSerializer,
statsRecorder,
fakeClock.getStopwatchSupplier(),
propagate, recordStats, recordStats, recordStats);
propagate, recordStats, recordStats, recordStats, recordStats);
Metadata headers = new Metadata();
CensusStatsModule.CallAttemptsTracerFactory callAttemptsTracerFactory =
new CensusStatsModule.CallAttemptsTracerFactory(
Expand Down Expand Up @@ -1168,7 +1168,7 @@ private void subtestServerBasicStatsNoHeaders(
CensusStatsModule localCensusStats =
new CensusStatsModule(
tagger, tagCtxSerializer, statsRecorder, fakeClock.getStopwatchSupplier(),
true, recordStarts, recordFinishes, recordRealTime);
true, recordStarts, recordFinishes, recordRealTime, true);
ServerStreamTracer.Factory tracerFactory = localCensusStats.getServerTracerFactory();
ServerStreamTracer tracer =
tracerFactory.newServerStreamTracer(method.getFullMethodName(), new Metadata());
Expand Down Expand Up @@ -1429,7 +1429,7 @@ public void newTagsPopulateOldViews() throws InterruptedException {

CensusStatsModule localCensusStats = new CensusStatsModule(
tagger, tagCtxSerializer, localStats.getStatsRecorder(), fakeClock.getStopwatchSupplier(),
false, false, true, false /* real-time */);
false, false, true, false /* real-time */, true);

CensusStatsModule.CallAttemptsTracerFactory callAttemptsTracerFactory =
new CensusStatsModule.CallAttemptsTracerFactory(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ public ClientTransportFactory buildClientTransportFactory() {
// https://github.com/grpc/grpc-java/issues/2284
managedChannelImplBuilder.setStatsRecordStartedRpcs(false);
managedChannelImplBuilder.setStatsRecordFinishedRpcs(false);
managedChannelImplBuilder.setStatsRecordRetryMetrics(false);
}

@Internal
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ public static ManagedChannelBuilder<?> forTarget(String target) {
private boolean recordStartedRpcs = true;
private boolean recordFinishedRpcs = true;
private boolean recordRealTimeMetrics = false;
private boolean recordRetryMetrics = true;
private boolean tracingEnabled = true;

/**
Expand Down Expand Up @@ -583,6 +584,10 @@ public void setStatsRecordFinishedRpcs(boolean value) {
public void setStatsRecordRealTimeMetrics(boolean value) {
recordRealTimeMetrics = value;
}

public void setStatsRecordRetryMetrics(boolean value) {
recordRetryMetrics = value;
}

/**
* Disable or enable tracing features. Enabled by default.
Expand Down Expand Up @@ -643,14 +648,16 @@ List<ClientInterceptor> getEffectiveInterceptors() {
"getClientInterceptor",
boolean.class,
boolean.class,
boolean.class,
boolean.class);
statsInterceptor =
(ClientInterceptor) getClientInterceptorMethod
.invoke(
null,
recordStartedRpcs,
recordFinishedRpcs,
recordRealTimeMetrics);
recordRealTimeMetrics,
recordRetryMetrics);
} catch (ClassNotFoundException e) {
// Replace these separate catch statements with multicatch when Android min-API >= 19
log.log(Level.FINE, "Unable to apply census stats", e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,8 @@ protected final ClientInterceptor createCensusStatsClientInterceptor() {
tagger, tagContextBinarySerializer, clientStatsRecorder,
GrpcUtil.STOPWATCH_SUPPLIER,
true, true, true,
/* recordRealTimeMetrics= */ false);
/* recordRealTimeMetrics= */ false,
/* recordRetryMetrics= */ true);
}

protected final ServerStreamTracer.Factory createCustomCensusTracerFactory() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ public void run() {} // no-op
InternalCensusStatsAccessor.getClientInterceptor(
tagger, tagContextBinarySerializer, clientStatsRecorder,
fakeClock.getStopwatchSupplier(), true, true, true,
/* recordRealTimeMetrics= */ true);
/* recordRealTimeMetrics= */ true, /* recordRetryMetrics= */ true);
private final MethodDescriptor<String, Integer> clientStreamingMethod =
MethodDescriptor.<String, Integer>newBuilder()
.setType(MethodType.CLIENT_STREAMING)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,10 @@ public static void setStatsRecordRealTimeMetrics(NettyChannelBuilder builder, bo
builder.setStatsRecordRealTimeMetrics(value);
}

public static void setStatsRecordRetryMetrics(NettyChannelBuilder builder, boolean value) {
builder.setStatsRecordRetryMetrics(value);
}

/**
* Sets {@link io.grpc.Channel} and {@link io.netty.channel.EventLoopGroup} to Nio. A major
* benefit over using setters is gRPC will manage the life cycle of {@link
Expand Down
4 changes: 4 additions & 0 deletions netty/src/main/java/io/grpc/netty/NettyChannelBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -605,6 +605,10 @@ void setStatsRecordRealTimeMetrics(boolean value) {
this.managedChannelImplBuilder.setStatsRecordRealTimeMetrics(value);
}

void setStatsRecordRetryMetrics(boolean value) {
this.managedChannelImplBuilder.setStatsRecordRetryMetrics(value);
}

@VisibleForTesting
NettyChannelBuilder setTransportTracerFactory(TransportTracer.Factory transportTracerFactory) {
this.transportTracerFactory = transportTracerFactory;
Expand Down

0 comments on commit 7c6f53a

Please sign in to comment.