googleapis / googleapis/google-cloud-cpp
Bigtable BulkApply does not respect custom retry policies for individual mutation errors
- Ngôn ngữ chính
- C++
- Star
- 659
- Fork
- 462
- Merge trung bình
- 1 ngày 2 giờ
- Pull request đã merge (30 ngày)
- 89
Mô tả
## Problem
We use the current retry policy (which can be customized by the application) to determine whether stream failures are transient.
However, we use the *default* retry policy to determine whether individual mutation errors are transient. Specifically, we only add mutations with default transient errors to the next batch:
https://github.com/googleapis/google-cloud-cpp/blob/0117ba9d7c97f6b512da5e683015f59a07699d74/google/cloud/bigtable/internal/bulk_mutator.cc#L109-L110
## Implementation notes
- The annoying this is that the `BulkMutatorState` code path is shared by a client with `Options` and one without. So there will not always be a retry policy available. We likely will have to have branching in the common code path, which is a red-flag that it should not be a common code path. :shrug:
- We should only check if the error is transient, with `IsPermanentFailure() const`. We should not be pinging `OnFailure()` for individual mutations. (e.g. consider a `DataLimitedErrorCountPolicy(...)`. We want that to apply to stream failures only, not individual mutations).
https://github.com/googleapis/google-cloud-cpp/blob/0117ba9d7c97f6b512da5e683015f59a07699d74/google/cloud/bigtable/retry_policy.h#L71-L73
- We do not need to check `IsExhausted() const`. That will be done in the DataConnection's retry loop.
https://github.com/googleapis/google-cloud-cpp/blob/0117ba9d7c97f6b512da5e683015f59a07699d74/google/cloud/bigtable/internal/data_connection_impl.cc#L181
Hướng dẫn đóng góp
Đánh giá
Issue này chưa được đánh giá.