Skip to content

Commit 452bd29

Browse files
laramielcopybara-github
authored andcommitted
When specified, ignore gcs_grpc timeout values below 250ms.
The gcs_grpc.timeout value is not recommended nor is it documented, and the primary purpose is testing. So we're going to limit it to values higher than 250ms for now. PiperOrigin-RevId: 896713295 Change-Id: Ib7695a721eda5efaf3b6edf0c93216205977ebe8
1 parent f0a8ceb commit 452bd29

2 files changed

Lines changed: 18 additions & 11 deletions

File tree

tensorstore/kvstore/gcs_grpc/gcs_grpc.cc

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,6 @@
5353
#include "tensorstore/internal/uri/percent_coder.h"
5454
#include "tensorstore/json_serialization_options.h"
5555
#include "tensorstore/json_serialization_options_base.h"
56-
#include "tensorstore/kvstore/batch_util.h"
5756
#include "tensorstore/kvstore/common_metrics.h"
5857
#include "tensorstore/kvstore/driver.h"
5958
#include "tensorstore/kvstore/gcs/exp_credentials_resource.h"
@@ -74,7 +73,6 @@
7473
#include "tensorstore/kvstore/read_result.h"
7574
#include "tensorstore/kvstore/registry.h"
7675
#include "tensorstore/kvstore/spec.h"
77-
#include "tensorstore/kvstore/supported_features.h"
7876
#include "tensorstore/kvstore/url_registry.h"
7977
#include "tensorstore/util/execution/any_receiver.h"
8078
#include "tensorstore/util/executor.h"
@@ -103,7 +101,6 @@ namespace jb = tensorstore::internal_json_binding;
103101

104102
using ::tensorstore::internal::DataCopyConcurrencyResource;
105103
using ::tensorstore::internal::ScheduleAt;
106-
using ::tensorstore::internal_gcs_grpc::GetSharedStorageStubPool;
107104
using ::tensorstore::internal_storage_gcs::ExperimentalGcsGrpcCredentials;
108105
using ::tensorstore::internal_storage_gcs::GcsUserProjectResource;
109106
using ::tensorstore::internal_storage_gcs::IsValidBucketName;
@@ -153,15 +150,15 @@ TENSORSTORE_DEFINE_JSON_DEFAULT_BINDER(
153150
jb::DefaultInitializedValue())),
154151
jb::Member("timeout",
155152
jb::Projection<&GcsGrpcKeyValueStoreSpecData::timeout>(
156-
jb::DefaultValue<jb::kNeverIncludeDefaults>([](auto* x) {
157-
*x = absl::ZeroDuration();
158-
}))),
153+
jb::DefaultValue<jb::kNeverIncludeDefaults>(
154+
[](absl::Duration* x) {
155+
*x = absl::ZeroDuration();
156+
}))),
159157
jb::Member(
160158
"wait_for_connection",
161159
jb::Projection<&GcsGrpcKeyValueStoreSpecData::wait_for_connection>(
162-
jb::DefaultValue<jb::kNeverIncludeDefaults>([](auto* x) {
163-
*x = absl::ZeroDuration();
164-
}))),
160+
jb::DefaultValue<jb::kNeverIncludeDefaults>(
161+
[](absl::Duration* x) { *x = absl::ZeroDuration(); }))),
165162
jb::Member(
166163
GcsUserProjectResource::id,
167164
jb::Projection<&GcsGrpcKeyValueStoreSpecData::user_project>()),
@@ -173,7 +170,17 @@ TENSORSTORE_DEFINE_JSON_DEFAULT_BINDER(
173170
jb::Member(
174171
ExperimentalGcsGrpcCredentials::id,
175172
jb::Projection<&GcsGrpcKeyValueStoreSpecData::credentials>()), /**/
176-
jb::DiscardExtraMembers));
173+
jb::DiscardExtraMembers,
174+
jb::Initialize([](GcsGrpcKeyValueStoreSpecData* spec) {
175+
if (spec->timeout < absl::Milliseconds(250) &&
176+
spec->timeout != absl::ZeroDuration()) {
177+
spec->timeout = absl::ZeroDuration();
178+
ABSL_LOG_FIRST_N(INFO, 1)
179+
<< "The `gcs_grpc.timeout` should generally not be set. "
180+
"Values less than 250ms are ignored.";
181+
}
182+
return absl::OkStatus();
183+
})));
177184

178185
/// Obtains a `SpecData` representation from an open `Driver`.
179186
absl::Status GcsGrpcKeyValueStore::GetBoundSpecData(

tensorstore/kvstore/gcs_grpc/gcs_grpc_test.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ class GcsGrpcTest : public testing::Test {
102102
return kvstore::Open({{"driver", "gcs_grpc"},
103103
{"endpoint", mock_service_.server_address()},
104104
{"bucket", "bucket"},
105-
{"timeout", "100ms"}})
105+
{"timeout", "250ms"}})
106106
.value();
107107
}
108108

0 commit comments

Comments
 (0)