google-cloud-cpp icon indicating copy to clipboard operation
google-cloud-cpp copied to clipboard

support per-operation Options defaults

Open devbww opened this issue 2 years ago • 3 comments

We'd like to support default Options on a per-operation basis. For example, if the user does not override a retry or backoff policy, it would be nice for the default policies to depend on the characteristics of the operation. (See spanner_grpc_service_config.json for a specific case where different operations have different timeout/retry specifications.)

We might do this by introducing Options *ServiceOperation*DefaultOptions(Options options), much like the existing Options *Service*DefaultOptions(Options options), and creating the operation OptionsSpan like ...

T Client::Operation(..., Options opts = {}) {
  internal::OptionsSpan span(ServiceDefaults(ServiceOperationDefaults(
      internal::Merge(opts, internal::Merge(this->opts_, conn_->options())))));
  ....
}

Once we do something like that, however, we might as well simplify the default-options functions to simply return Options, and use Merge() as the only means to specify priority. So, ...

T Client::Operation(..., Options opts = {}) {
  internal::OptionsSpan span(internal::Merge(
      internal::Merge(
          internal::Merge(opts, internal::Merge(this->opts_, conn_->options())),
          ServiceOperationDefaults()),
      ServiceDefaults()));
  ....
}

Note: This implies that no defaults should be applied until OptionsSpan time. Note: internal::Merge(this->opts_, conn_->options()) could be precomputed.

But even then there remains the question of how the client and connection options should interact with the defaults. It is probably clear that per-operation options should override client options, which should override connection options, but how should client and connection options interact with service and operation defaults? For example, must all options that have operation defaults be overridden at the operation level?

devbww avatar Mar 01 '22 06:03 devbww

I think the precedence we want is the following, and any code that achieves it is correct:

  1. Custom operation options
  2. Custom client options
  3. Custom connection options
  4. Default operation options
  5. Default client options (if this is even different)
  6. Default connection options (if this is even different)

dbolduc avatar Mar 01 '22 13:03 dbolduc

I agree that this order is the "right" one given the infrastructure we have, and I think it is reflected in the code snippets above (modulo any distinction between "service defaults" and "client/connection defaults").

However, I think there is still a question about how custom client/connection options should interact with per-operation defaults. It seems closer to a bug than a feature to be able to blanket override options that have special, per-operation defaults.

In the case of Spanner, for example, does it ever make any sense to apply a custom client/connection timeout? If not, should we try to disallow that somehow?

devbww avatar Mar 02 '22 21:03 devbww

@devbww and @dbolduc will talk about this again.

coryan avatar Jul 28 '22 18:07 coryan

@devbww and @dbolduc will talk about this again.

We did, and the conclusion was that we don't think we want to support per-operation Options defaults, at least until it is clearer how they might behave and be specified. So, closing for now.

Note: This implies that no defaults should be applied until OptionsSpan time.

But even then there remains the question of how the client and connection options should interact with the defaults. It is probably clear that per-operation options should override client options, which should override connection options, but how should client and connection options interact with service and operation defaults? For example, must all options that have operation defaults be overridden at the operation level?

I think this still summarizes some of the behavioral issues.

devbww avatar Sep 22 '22 21:09 devbww