Skip to content
This repository has been archived by the owner on Jun 28, 2022. It is now read-only.

Commit

Permalink
fix: grpc_service_config timeout settings (#3200)
Browse files Browse the repository at this point in the history
* fix: set max & initial timeouts to total timeout

When using grpc_service_config the max & initial rpc timeout
should be the same as the total timeout. This essentially
renders the hedging logic unused because the initial timeout
is the limit. This means one RPC is made rather than several
smaller ones that are backed off until the total is reached.

* switch showcase to gapic v2 & grpc_service_config
* retry mapping: fix NPE when a Service is not present in protos
* add blockTimeout showcase test
* add valid block showcase test
* update gapic-showcase image for java to v0.10.1
* lengthen delay on flaky showcase java streaming tests
  • Loading branch information
noahdietz authored May 27, 2020
1 parent 8d362b4 commit 1a9835a
Show file tree
Hide file tree
Showing 7 changed files with 73 additions and 154 deletions.
9 changes: 7 additions & 2 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@ jobs:
- image: circleci/openjdk:8-jdk
working_directory: /tmp/workspace
environment:
SHOWCASE_VERSION: 0.7.0
SHOWCASE_VERSION: 0.10.1
steps:
- checkout:
path: gapic-generator
Expand All @@ -326,13 +326,18 @@ jobs:
git clone --single-branch https://github.com/googleapis/googleapis.git googleapis
- run:
name: Setup showcase protos.
# Remove the other showcase protos, testing only uses echo.proto
command: |
mkdir showcase-protos
curl -sL https://github.com/googleapis/gapic-showcase/releases/download/v${SHOWCASE_VERSION}/gapic-showcase-${SHOWCASE_VERSION}-protos.tar.gz | tar -C showcase-protos -xz
curl -sL https://github.com/googleapis/gapic-showcase/releases/download/v${SHOWCASE_VERSION}/showcase_grpc_service_config.json > showcase-protos/google/showcase/v1beta1/showcase_grpc_service_config.json
TEST_SRC="gapic-generator/src/test/java/com/google/api/codegen/testsrc/showcase"
cp ${TEST_SRC}/showcase.yaml showcase-protos/google/showcase/
cp ${TEST_SRC}/showcase_gapic.yaml showcase-protos/google/showcase/v1beta1/
cp ${TEST_SRC}/artman_showcase.yaml showcase-protos/google/showcase/
rm showcase-protos/google/showcase/v1beta1/identity.proto
rm showcase-protos/google/showcase/v1beta1/messaging.proto
rm showcase-protos/google/showcase/v1beta1/testing.proto
- run:
name: Build and install local toolkit
Expand Down Expand Up @@ -528,7 +533,7 @@ jobs:
<<: *anchor_artman_vars
docker:
- image: circleci/openjdk:8-jdk
- image: gcr.io/gapic-images/gapic-showcase:0.2.0
- image: gcr.io/gapic-images/gapic-showcase:0.10.1
steps:
- attach_workspace:
at: workspace
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,11 @@
import com.google.api.gax.rpc.ClientStream;
import com.google.api.gax.rpc.ServerStream;
import com.google.api.gax.rpc.StreamController;
import com.google.protobuf.Duration;
import com.google.rpc.Code;
import com.google.rpc.Status;
import com.google.showcase.v1beta1.BlockRequest;
import com.google.showcase.v1beta1.BlockResponse;
import com.google.showcase.v1beta1.EchoClient;
import com.google.showcase.v1beta1.EchoRequest;
import com.google.showcase.v1beta1.EchoResponse;
Expand Down Expand Up @@ -172,7 +175,7 @@ public void onCompleted() {
}
requestStream.onCompleted();

latch.await(2, TimeUnit.SECONDS);
latch.await(7, TimeUnit.SECONDS);

assertThat(collections).containsExactly("a b c done");
}
Expand Down Expand Up @@ -224,8 +227,35 @@ public void onComplete() {
}
});

latch.await(2, TimeUnit.SECONDS);
latch.await(7, TimeUnit.SECONDS);

assertThat(responses).containsExactlyElementsIn(inputs).inOrder();
}

@Test(expected = StatusRuntimeException.class)
public void blockTimeout() {
try {
client.block(
BlockRequest.newBuilder()
// Set a longer timeout than the 5 seconds specified in the grpc_service_config.
.setResponseDelay(Duration.newBuilder().setSeconds(10L).build())
.build());
} catch (Exception e) {
assertThat(e.getCause()).isInstanceOf(StatusRuntimeException.class);
StatusRuntimeException error = (StatusRuntimeException) e.getCause();
assertThat(error.getStatus().getCode().value()).isEqualTo(Code.DEADLINE_EXCEEDED_VALUE);
throw error;
}
}

@Test
public void block() {
BlockResponse result =
client.block(
BlockRequest.newBuilder()
.setResponseDelay(Duration.newBuilder().setSeconds(2L).build())
.setSuccess(BlockResponse.newBuilder().setContent("Hello, World!").build())
.build());
assertThat(result.getContent()).isEqualTo("Hello, World!");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -139,9 +139,13 @@ private static void applyRetryPolicyToName(
return;
}

// apply the RetryPolicy to all methods in the service interface that don't already have an
// entry
// Apply the RetryPolicy to all methods in the service interface that don't already have an
// entry. If the Service is not present in the protos, skip it.
Interface interProto = protoInterfaces.get(service);
if (interProto == null) {
return;
}

for (Method methodProto : interProto.getMethods()) {
String fullName = methodProto.getFullName();
methodCodesMap.putIfAbsent(fullName, codesName);
Expand All @@ -155,7 +159,10 @@ private static RetryParamsDefinitionProto.Builder retryPolicyToParamsBuilder(
.setMaxRetryDelayMillis(Durations.toMillis(retryPolicy.getMaxBackoff()))
.setInitialRetryDelayMillis(Durations.toMillis(retryPolicy.getInitialBackoff()))
.setRetryDelayMultiplier(convertFloatToDouble(retryPolicy.getBackoffMultiplier()))
.setRpcTimeoutMultiplier(1.0)
.setTotalTimeoutMillis(timeout)
.setMaxRpcTimeoutMillis(timeout)
.setInitialRpcTimeoutMillis(timeout)
.setName(policyName);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14487,13 +14487,20 @@ public class LibraryServiceStubSettings extends StubSettings<LibraryServiceStubS
.setInitialRetryDelay(Duration.ofMillis(200L))
.setRetryDelayMultiplier(1.3)
.setMaxRetryDelay(Duration.ofMillis(35000L))
.setInitialRpcTimeout(Duration.ofMillis(60000L))
.setRpcTimeoutMultiplier(1.0)
.setMaxRpcTimeout(Duration.ofMillis(60000L))
.setTotalTimeout(Duration.ofMillis(60000L))
.build();
definitions.put("retry_policy_1_params", settings);
settings = RetrySettings.newBuilder()
.setRpcTimeoutMultiplier(1.0)
.build();
definitions.put("no_retry_params", settings);
settings = RetrySettings.newBuilder()
.setInitialRpcTimeout(Duration.ofMillis(60000L))
.setRpcTimeoutMultiplier(1.0)
.setMaxRpcTimeout(Duration.ofMillis(60000L))
.setTotalTimeout(Duration.ofMillis(60000L))
.build();
definitions.put("no_retry_1_params", settings);
Expand Down Expand Up @@ -15491,9 +15498,13 @@ public class MyProtoStubSettings extends StubSettings<MyProtoStubSettings> {
ImmutableMap.Builder<String, RetrySettings> definitions = ImmutableMap.builder();
RetrySettings settings = null;
settings = RetrySettings.newBuilder()
.setRpcTimeoutMultiplier(1.0)
.build();
definitions.put("no_retry_params", settings);
settings = RetrySettings.newBuilder()
.setInitialRpcTimeout(Duration.ofMillis(60000L))
.setRpcTimeoutMultiplier(1.0)
.setMaxRpcTimeout(Duration.ofMillis(60000L))
.setTotalTimeout(Duration.ofMillis(60000L))
.build();
definitions.put("no_retry_2_params", settings);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6089,27 +6089,27 @@ class MyProtoClient extends MyProtoGapicClient
"initial_retry_delay_millis": 200,
"retry_delay_multiplier": 1.3,
"max_retry_delay_millis": 35000,
"initial_rpc_timeout_millis": 0,
"rpc_timeout_multiplier": 0.0,
"max_rpc_timeout_millis": 0,
"initial_rpc_timeout_millis": 60000,
"rpc_timeout_multiplier": 1.0,
"max_rpc_timeout_millis": 60000,
"total_timeout_millis": 60000
},
"no_retry_params": {
"initial_retry_delay_millis": 0,
"retry_delay_multiplier": 0.0,
"max_retry_delay_millis": 0,
"initial_rpc_timeout_millis": 0,
"rpc_timeout_multiplier": 0.0,
"rpc_timeout_multiplier": 1.0,
"max_rpc_timeout_millis": 0,
"total_timeout_millis": 0
},
"no_retry_1_params": {
"initial_retry_delay_millis": 0,
"retry_delay_multiplier": 0.0,
"max_retry_delay_millis": 0,
"initial_rpc_timeout_millis": 0,
"rpc_timeout_multiplier": 0.0,
"max_rpc_timeout_millis": 0,
"initial_rpc_timeout_millis": 60000,
"rpc_timeout_multiplier": 1.0,
"max_rpc_timeout_millis": 60000,
"total_timeout_millis": 60000
}
},
Expand Down Expand Up @@ -6740,17 +6740,17 @@ return [
"retry_delay_multiplier": 0.0,
"max_retry_delay_millis": 0,
"initial_rpc_timeout_millis": 0,
"rpc_timeout_multiplier": 0.0,
"rpc_timeout_multiplier": 1.0,
"max_rpc_timeout_millis": 0,
"total_timeout_millis": 0
},
"no_retry_2_params": {
"initial_retry_delay_millis": 0,
"retry_delay_multiplier": 0.0,
"max_retry_delay_millis": 0,
"initial_rpc_timeout_millis": 0,
"rpc_timeout_multiplier": 0.0,
"max_rpc_timeout_millis": 0,
"initial_rpc_timeout_millis": 60000,
"rpc_timeout_multiplier": 1.0,
"max_rpc_timeout_millis": 60000,
"total_timeout_millis": 60000
}
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ common:
organization_name: google
service_yaml: showcase.yaml
gapic_yaml: v1beta1/showcase_gapic.yaml
grpc_service_config: v1beta1/showcase_grpc_service_config.json
proto_package: google.showcase.v1beta1
src_proto_paths:
- v1beta1
proto_deps:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
# retry_codes_name, required_fields, flattening, and timeout properties cannot
# be precisely decided by the tooling and may require some configuration.
type: com.google.api.codegen.ConfigProto
config_schema_version: 1.0.0
config_schema_version: 2.0.0
# The settings of generated code in a specific language.
language_settings:
java:
Expand All @@ -20,139 +20,3 @@ language_settings:
package_name: Google\Showcase\V1beta1
nodejs:
package_name: showcase.v1beta1
# The configuration for the license header to put on generated files.
license_header:
# The file containing the copyright line(s).
copyright_file: copyright-google.txt
# The file containing the raw license header without any copyright line(s).
license_file: license-header-apache-2.0.txt
# A list of API interface configurations.
interfaces:
# The fully qualified name of the API interface.
- name: google.showcase.v1beta1.Echo
# A list of resource collection configurations.
# Consists of a name_pattern and an entity_name.
# The name_pattern is a pattern to describe the names of the resources of this
# collection, using the platform's conventions for URI patterns. A generator
# may use this to generate methods to compose and decompose such names. The
# pattern should use named placeholders as in `shelves/{shelf}/books/{book}`;
# those will be taken as hints for the parameter names of the generated
# methods. If empty, no name methods are generated.
# The entity_name is the name to be used as a basis for generated methods and
# classes.
# Definition for retryable codes.
retry_codes_def:
- name: idempotent
retry_codes:
- UNAVAILABLE
- DEADLINE_EXCEEDED
- name: non_idempotent
retry_codes: []
# Definition for retry/backoff parameters.
retry_params_def:
- name: default
initial_retry_delay_millis: 100
retry_delay_multiplier: 1.3
max_retry_delay_millis: 60000
initial_rpc_timeout_millis: 20000
rpc_timeout_multiplier: 1
max_rpc_timeout_millis: 20000
total_timeout_millis: 600000
# A list of method configurations.
# Common properties:
#
# name - The simple name of the method.
#
# flattening - Specifies the configuration for parameter flattening.
# Describes the parameter groups for which a generator should produce method
# overloads which allow a client to directly pass request message fields as
# method parameters. This information may or may not be used, depending on
# the target language.
# Consists of groups, which each represent a list of parameters to be
# flattened. Each parameter listed must be a field of the request message.
#
# required_fields - Fields that are always required for a request to be
# valid.
#
# resource_name_treatment - An enum that specifies how to treat the resource
# name formats defined in the field_name_patterns and
# response_field_name_patterns fields.
# UNSET: default value
# NONE: the collection configs will not be used by the generated code.
# VALIDATE: string fields will be validated by the client against the
# specified resource name formats.
# STATIC_TYPES: the client will use generated types for resource names.
#
# page_streaming - Specifies the configuration for paging.
# Describes information for generating a method which transforms a paging
# list RPC into a stream of resources.
# Consists of a request and a response.
# The request specifies request information of the list method. It defines
# which fields match the paging pattern in the request. The request consists
# of a page_size_field and a token_field. The page_size_field is the name of
# the optional field specifying the maximum number of elements to be
# returned in the response. The token_field is the name of the field in the
# request containing the page token.
# The response specifies response information of the list method. It defines
# which fields match the paging pattern in the response. The response
# consists of a token_field and a resources_field. The token_field is the
# name of the field in the response containing the next page token. The
# resources_field is the name of the field in the response containing the
# list of resources belonging to the page.
#
# retry_codes_name - Specifies the configuration for retryable codes. The
# name must be defined in interfaces.retry_codes_def.
#
# retry_params_name - Specifies the configuration for retry/backoff
# parameters. The name must be defined in interfaces.retry_params_def.
#
# field_name_patterns - Maps the field name of the request type to
# entity_name of interfaces.collections.
# Specifies the string pattern that the field must follow.
#
# timeout_millis - Specifies the default timeout for a non-retrying call. If
# the call is retrying, refer to retry_params_name instead.
methods:
- name: Echo
retry_codes_name: idempotent
retry_params_name: default
timeout_millis: 60000
- name: Expand
flattening:
groups:
- parameters:
- content
- error
retry_codes_name: idempotent
retry_params_name: default
timeout_millis: 60000
- name: PagedExpand
page_streaming:
request:
page_size_field: page_size
token_field: page_token
response:
token_field: next_page_token
resources_field: responses
retry_codes_name: idempotent
retry_params_name: default
timeout_millis: 60000
- name: Collect
retry_codes_name: non_idempotent
retry_params_name: default
timeout_millis: 60000
- name: Chat
retry_codes_name: idempotent
retry_params_name: default
timeout_millis: 60000
- name: Wait
retry_codes_name: idempotent
retry_params_name: default
timeout_millis: 60000
long_running:
return_type: google.showcase.v1beta1.WaitResponse
metadata_type: google.showcase.v1beta1.WaitMetadata
initial_poll_delay_millis: 20000
poll_delay_multiplier: 1.5
max_poll_delay_millis: 45000
total_poll_timeout_millis: 86400000

0 comments on commit 1a9835a

Please sign in to comment.