This is an automated email from the ASF dual-hosted git repository. chhsiao pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 668781708ac80913ad1b8eb336e4c38bff70d842 Author: Chun-Hung Hsiao <[email protected]> AuthorDate: Tue Jun 11 11:33:00 2019 -0700 Moved default constants for CSI RPC retry to a new header. Since the default constants for CSI RPC retry do not depend on CSI versions, these constants are pulled off from version-specific headers to a common header. Review: https://reviews.apache.org/r/71143 --- src/Makefile.am | 3 +- src/csi/constants.hpp | 38 ++++++++++++++++++++++ src/csi/v0_volume_manager.cpp | 5 +-- src/csi/v0_volume_manager_process.hpp | 11 ------- src/csi/v1_volume_manager.cpp | 5 +-- src/csi/v1_volume_manager_process.hpp | 11 ------- .../storage_local_resource_provider_tests.cpp | 9 ++--- 7 files changed, 51 insertions(+), 31 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index 697ab10..f0a83a2 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1588,7 +1588,7 @@ libmesos_no_3rdparty_la_LIBADD += libbuild.la # Convenience library for build the CSI client. noinst_LTLIBRARIES += libcsi.la libcsi_la_SOURCES = \ - csi/types.cpp \ + csi/constants.hpp \ csi/metrics.cpp \ csi/metrics.hpp \ csi/paths.cpp \ @@ -1597,6 +1597,7 @@ libcsi_la_SOURCES = \ csi/service_manager.hpp \ csi/state.hpp \ csi/state.proto \ + csi/types.cpp \ csi/v0.cpp \ csi/v0_client.cpp \ csi/v0_client.hpp \ diff --git a/src/csi/constants.hpp b/src/csi/constants.hpp new file mode 100644 index 0000000..af53cb2 --- /dev/null +++ b/src/csi/constants.hpp @@ -0,0 +1,38 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef __CSI_CONSTANTS_HPP__ +#define __CSI_CONSTANTS_HPP__ + +#include <stout/duration.hpp> + +namespace mesos { +namespace csi { + +// The CSI volume manager initially picks a random amount of time between +// `[0, b]`, where `b = DEFAULT_RPC_RETRY_BACKOFF_FACTOR`, to retry RPC calls. +// Subsequent retries are exponentially backed off based on this interval (e.g., +// 2nd retry uses a random value between `[0, b * 2^1]`, 3rd retry between +// `[0, b * 2^2]`, etc) up to a maximum of `DEFAULT_RPC_RETRY_INTERVAL_MAX`. +// +// TODO(chhsiao): Make the retry parameters configurable. +constexpr Duration DEFAULT_RPC_RETRY_BACKOFF_FACTOR = Seconds(10); +constexpr Duration DEFAULT_RPC_RETRY_INTERVAL_MAX = Minutes(10); + +} // namespace csi { +} // namespace mesos { + +#endif // __CSI_CONSTANTS_HPP__ diff --git a/src/csi/v0_volume_manager.cpp b/src/csi/v0_volume_manager.cpp index e19dc7c..4b056e7 100644 --- a/src/csi/v0_volume_manager.cpp +++ b/src/csi/v0_volume_manager.cpp @@ -40,6 +40,7 @@ #include <stout/try.hpp> #include <stout/unreachable.hpp> +#include "csi/constants.hpp" #include "csi/paths.hpp" #include "csi/v0_client.hpp" #include "csi/v0_utils.hpp" @@ -494,7 +495,7 @@ Future<Response> VolumeManagerProcess::call( const Request& request, const bool retry) // Made immutable in the following mutable lambda. { - Duration maxBackoff = DEFAULT_CSI_RETRY_BACKOFF_FACTOR; + Duration maxBackoff = DEFAULT_RPC_RETRY_BACKOFF_FACTOR; return process::loop( self(), @@ -514,7 +515,7 @@ Future<Response> VolumeManagerProcess::call( ? maxBackoff * (static_cast<double>(os::random()) / RAND_MAX) : Option<Duration>::none(); - maxBackoff = std::min(maxBackoff * 2, DEFAULT_CSI_RETRY_INTERVAL_MAX); + maxBackoff = std::min(maxBackoff * 2, DEFAULT_RPC_RETRY_INTERVAL_MAX); // We dispatch `__call` for testing purpose. return process::dispatch( diff --git a/src/csi/v0_volume_manager_process.hpp b/src/csi/v0_volume_manager_process.hpp index 4cfb5b5..50148ff 100644 --- a/src/csi/v0_volume_manager_process.hpp +++ b/src/csi/v0_volume_manager_process.hpp @@ -55,17 +55,6 @@ namespace mesos { namespace csi { namespace v0 { -// The CSI volume manager initially picks a random amount of time between -// `[0, b]`, where `b = DEFAULT_CSI_RETRY_BACKOFF_FACTOR`, to retry CSI calls. -// Subsequent retries are exponentially backed off based on this interval (e.g., -// 2nd retry uses a random value between `[0, b * 2^1]`, 3rd retry between -// `[0, b * 2^2]`, etc) up to a maximum of `DEFAULT_CSI_RETRY_INTERVAL_MAX`. -// -// TODO(chhsiao): Make the retry parameters configurable. -constexpr Duration DEFAULT_CSI_RETRY_BACKOFF_FACTOR = Seconds(10); -constexpr Duration DEFAULT_CSI_RETRY_INTERVAL_MAX = Minutes(10); - - class VolumeManagerProcess : public process::Process<VolumeManagerProcess> { public: diff --git a/src/csi/v1_volume_manager.cpp b/src/csi/v1_volume_manager.cpp index e7e0329..9e44947 100644 --- a/src/csi/v1_volume_manager.cpp +++ b/src/csi/v1_volume_manager.cpp @@ -41,6 +41,7 @@ #include <stout/try.hpp> #include <stout/unreachable.hpp> +#include "csi/constants.hpp" #include "csi/paths.hpp" #include "csi/v1_client.hpp" #include "csi/v1_utils.hpp" @@ -515,7 +516,7 @@ Future<Response> VolumeManagerProcess::call( const Request& request, const bool retry) // Made immutable in the following mutable lambda. { - Duration maxBackoff = DEFAULT_CSI_RETRY_BACKOFF_FACTOR; + Duration maxBackoff = DEFAULT_RPC_RETRY_BACKOFF_FACTOR; return process::loop( self(), @@ -535,7 +536,7 @@ Future<Response> VolumeManagerProcess::call( ? maxBackoff * (static_cast<double>(os::random()) / RAND_MAX) : Option<Duration>::none(); - maxBackoff = std::min(maxBackoff * 2, DEFAULT_CSI_RETRY_INTERVAL_MAX); + maxBackoff = std::min(maxBackoff * 2, DEFAULT_RPC_RETRY_INTERVAL_MAX); // We dispatch `__call` for testing purpose. return process::dispatch( diff --git a/src/csi/v1_volume_manager_process.hpp b/src/csi/v1_volume_manager_process.hpp index 30788c3..a03e291 100644 --- a/src/csi/v1_volume_manager_process.hpp +++ b/src/csi/v1_volume_manager_process.hpp @@ -55,17 +55,6 @@ namespace mesos { namespace csi { namespace v1 { -// The CSI volume manager initially picks a random amount of time between -// `[0, b]`, where `b = DEFAULT_CSI_RETRY_BACKOFF_FACTOR`, to retry CSI calls. -// Subsequent retries are exponentially backed off based on this interval (e.g., -// 2nd retry uses a random value between `[0, b * 2^1]`, 3rd retry between -// `[0, b * 2^2]`, etc) up to a maximum of `DEFAULT_CSI_RETRY_INTERVAL_MAX`. -// -// TODO(chhsiao): Make the retry parameters configurable. -constexpr Duration DEFAULT_CSI_RETRY_BACKOFF_FACTOR = Seconds(10); -constexpr Duration DEFAULT_CSI_RETRY_INTERVAL_MAX = Minutes(10); - - class VolumeManagerProcess : public process::Process<VolumeManagerProcess> { public: diff --git a/src/tests/storage_local_resource_provider_tests.cpp b/src/tests/storage_local_resource_provider_tests.cpp index 6986126..69b59d4 100644 --- a/src/tests/storage_local_resource_provider_tests.cpp +++ b/src/tests/storage_local_resource_provider_tests.cpp @@ -48,6 +48,7 @@ #include <stout/os/realpath.hpp> +#include "csi/constants.hpp" #include "csi/paths.hpp" #include "csi/state.hpp" #include "csi/v0_volume_manager_process.hpp" @@ -5893,7 +5894,7 @@ TEST_P(StorageLocalResourceProviderTest, RetryRpcWithExponentialBackoff) AWAIT_READY(createVolumeCall) << "Failed to wait for CreateVolume call #1"; - Duration createVolumeBackoff = csi::v0::DEFAULT_CSI_RETRY_BACKOFF_FACTOR; + Duration createVolumeBackoff = csi::DEFAULT_RPC_RETRY_BACKOFF_FACTOR; for (size_t i = 1; i < numRetryableErrors; i++) { // Return `UNAVAILABLE` for subsequent `CreateVolume` calls. @@ -5909,7 +5910,7 @@ TEST_P(StorageLocalResourceProviderTest, RetryRpcWithExponentialBackoff) << "Failed to wait for CreateVolume call #" << (i + 1); createVolumeBackoff = std::min( - createVolumeBackoff * 2, csi::v0::DEFAULT_CSI_RETRY_INTERVAL_MAX); + createVolumeBackoff * 2, csi::DEFAULT_RPC_RETRY_INTERVAL_MAX); } // Return a successful response for the last `CreateVolume` call. @@ -5954,7 +5955,7 @@ TEST_P(StorageLocalResourceProviderTest, RetryRpcWithExponentialBackoff) AWAIT_READY(deleteVolumeCall) << "Failed to wait for DeleteVolume call #1"; - Duration deleteVolumeBackoff = csi::v0::DEFAULT_CSI_RETRY_BACKOFF_FACTOR; + Duration deleteVolumeBackoff = csi::DEFAULT_RPC_RETRY_BACKOFF_FACTOR; for (size_t i = 1; i < numRetryableErrors; i++) { // Return `UNAVAILABLE` for subsequent `DeleteVolume` calls. @@ -5970,7 +5971,7 @@ TEST_P(StorageLocalResourceProviderTest, RetryRpcWithExponentialBackoff) << "Failed to wait for DeleteVolume call #" << (i + 1); deleteVolumeBackoff = std::min( - deleteVolumeBackoff * 2, csi::v0::DEFAULT_CSI_RETRY_INTERVAL_MAX); + deleteVolumeBackoff * 2, csi::DEFAULT_RPC_RETRY_INTERVAL_MAX); } // Return a non-retryable error for the last `DeleteVolume` call.
