Respect GAPIC deadlines and retry policies by default

[sduskis]

All GAPIC clients use a centralized definition of behavior as defined by a yaml file. The files for Cloud Bigtable Admin is found here and the CBT deta API file is here.

Ideally, there's a reusable mechanism to read these types of YAML files so that other C++ APIs can use this configuration as well.

[igorbernstein2]

Please note that the values in that file have not been tuned yet.

[coryan]

Is there a document describing the design and/or structure of these files?

Also, part of this tasks will be to determine:

• At run-time, where do the files live?
• How does the application (or the library embedded into the application) find them?
• If there are standard answers to those questions used by other languages then we should use the same in C++, otherwise we need to document the answers we provide for the C++ libraries.

[igorbernstein2]

The yaml file is used to populate this proto (which is heavily documented):
https://github.com/googleapis/gapic-generator/blob/master/src/main/proto/com/google/api/codegen/config.proto

That proto (along with the client protos) are used to generate clients. The java gapic clients generate java code to represent the settings:
https://github.com/GoogleCloudPlatform/google-cloud-java/blob/master/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/BigtableStubSettings.java#L247

Then those settings are passed to the stub/client:
https://github.com/GoogleCloudPlatform/google-cloud-java/blob/master/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/BigtableStub.java

I'm not entirely sure how the other languages use the yaml.

[coryan]

Ahh, so at least for Java the values are embedded in the generated code. Well, we can do the same for C++, that would be manual though.

One possibility is to write a script to parse the file extract the relevant values and generate a header file with the constants.

I think the more interesting bit is that the existence of these values prescribes requirements for the implementation.

[igorbernstein2]

Yes, but keep in mind that the java client (and the rest of the gapic clients) gets regenerated from those yaml files from time to time. So I would urge against doing it manually in the mid to long term. I think a script makes sense.

What do you mean about the "rescribes requirements for the implementation"? That each method must be configurable separately or that all clients should have the same values across languages? If the former, I'm not convinced that we need different back off settings for different methods nor exponentially interesting deadlines. But I do think that clients should have a consistent set of values.

As a sidenote, I wanted to point out a subtlety regarding retry settings for read rows. The attempt counts should be relative to the last successful message. I've detailed the semantics here:
googleapis/gax-java#463

[coryan]

What do you mean about the "rescribes requirements for the implementation"?

Well, the fact that there is a parameter called "poll delay multiplier" implies that you should poll, and that the delay is on some kind of exponential backoff (as opposed to constant rate). The fact that there is a parameter called "total poll timeout millis" implies that you give up when a certain amount of time has elapsed, and not a number of retries, or a combination of count of retries and time.

That each method must be configurable separately

That too. The CBT-C++ client has a single configuration per interface (Table or TableAdmin) and they apply to all the methodmember functions of those classes.

[manish-qlogic]

Currently, CBT CPP client allows application developer to change the defaults for their application by compiling with different values (e.g. DEFAULT_INITIAL_DELAY)

@coryan
Do we want to keep this flexibility even after CPP client starts respecting GAPIC limits?

I am thinking of following approach

1. Create python script to parse Admin yaml and Data yaml and generate a Header file.
2. This generated header file will contain a map of constant values to store limits per interface, it can be extended to even store limits per call later.
3. Default construction (e.g. DefaultRPCBackoffPolicy()) will construct and return appropriate policy after fetching values from generated header.
4. Header file needs to be generated on periodic basis.

[coryan]

Do we want to keep this flexibility even after CPP client starts respecting GAPIC limits?

Yes, I think we do. We will eventually get the question of "how can I change the default values", and I would like to be able to give an answer like "add -D...=your_value to the compilation flags", instead of "edit this YAML file, regenerate the header using this script and then you are done."

1. Create python script to parse Admin yaml and Data yaml and generate a Header file.
2. This generated header file will contain a map of constant values to store limits per interface, it can be extended to even store limits per call later.

A map like a std::map<> or some other data structure? Can you tell me what types would be the keys into this map?

Could it be just a header with a number of constants pre-defined, like:

#ifndef BIGTABLE_CLIENT_DEFAULT_INTIAL_DELAY_ADMIN
#define  BIGTABLE_CLIENT_DEFAULT_INTIAL_DELAY_ADMIN /* value from YAML here */
#endif // .... 

Is ugly but flexible enough maybe?

3. Default construction (e.g. DefaultRPCBackoffPolicy()) will construct and return appropriate policy after fetching values from generated header.

That sounds good.

4. Header file needs to be generated on periodic basis.

That sounds good.

[manish-qlogic]

Yes, I think we do. We will eventually get the question of "how can I change the default values", and I would like to be able to give an answer like "add -D...=your_value to the compilation flags", instead of "edit this YAML file, regenerate the header using this script and then you are done."

Sure I will introduce more pre-defined to accomplish this. So one can change default behavior by passing user defined policies at the time of client creation and can also change default values through compilation flags.

Could it be just a header with a number of constants pre-defined, like:
#ifndef BIGTABLE_CLIENT_DEFAULT_INTIAL_DELAY_ADMIN
#define BIGTABLE_CLIENT_DEFAULT_INTIAL_DELAY_ADMIN /* value from YAML here */
#endif // ....

I am thinking of creating a class PolicyDefaults and then deriving TableAdminPolicyDefaults while parsing YAML. An override method can set pre-defineds in derived class. This will lead to much cleaner code in RPCBackoffPolicy and RPCRetryPolicy. I will test this approach and if it is more complex then I will stick to your suggestion of constants pre-defined.