This is an automated email from the ASF dual-hosted git repository. alexey pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/kudu.git
The following commit(s) were added to refs/heads/master by this push: new 97edafcde KUDU-3248 improve replica selection initialization 97edafcde is described below commit 97edafcde5de684688516ab713307721c36349c3 Author: Alexey Serbin <ale...@apache.org> AuthorDate: Mon May 1 15:34:17 2023 -0700 KUDU-3248 improve replica selection initialization Since the original implementation stored the random choice for replica selection integer in a variable that was initialized statically, the corresponding calls to libstdc++/libc++ runtime had been issued before the process called the main() function. That means some SSE4.2-specific instructions might be called since libkudu_client is unconditionally compiled with -msse4.2 flag, and there'd been no chance to call KuduClientBuilder::Build() that would verify the required features are present by calling CheckCPUFlags(). As a result, an attempt to run an application linked with kudu_client library at a machine lacking SSE4.2 support would result in a crash with SIGILL signal and a stack trace like below: #0 0x00007fc4b1b58162 in std::mersenne_twister_engine<...>::_M_gen_rand at include/c++/7.5.0/bits/random.tcc:408 #1 std::mersenne_twister_engine<...>::operator() at include/c++/7.5.0/bits/random.tcc:459 #2 0x00007fc4b1b1d65d in kudu::client::(anonymous namespace)::InitRandomSelectionInt at ../../../../../src/kudu/client/client-internal.cc:196 #3 0x00007fc4b1b1d6ef in __static_initialization_and_destruction_0 at ../../../../../src/kudu/client/client-internal.cc:198 #4 _GLOBAL__sub_I_client_internal.cc(void) at ../../../../../src/kudu/client/client-internal.cc:871 This patch addresses that deficiency, so now instead of unexpectedly crashing, the application would return an error upon at attempt to create an instance of KuduClient object. This is a follow-up to ccbbfb3006314f2c37f3a40bfec355db9fc90e02. Change-Id: I11c2a29ef69a8c97c68330d261fdff64accebb0b Reviewed-on: http://gerrit.cloudera.org:8080/19828 Reviewed-by: Abhishek Chennaka <achenn...@cloudera.com> Reviewed-by: Wenzhe Zhou <wz...@cloudera.com> Tested-by: Alexey Serbin <ale...@apache.org> --- src/kudu/client/client-internal.cc | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/src/kudu/client/client-internal.cc b/src/kudu/client/client-internal.cc index e4faa05db..f5c54a064 100644 --- a/src/kudu/client/client-internal.cc +++ b/src/kudu/client/client-internal.cc @@ -18,6 +18,7 @@ #include "kudu/client/client-internal.h" #include <algorithm> +#include <cstddef> #include <cstdint> #include <functional> #include <limits> @@ -188,17 +189,22 @@ KuduClient::Data::~Data() { namespace { -// This random integer is used when making any random choice for replica -// selection. It is static to provide a deterministic selection for any given -// process and therefore also better cache affinity while ensuring that we can +// This utility function returns an integer used when making any random choice +// for replica selection. It has the sematics of a per-process constant to +// provide deterministic selection for any given process that creates Kudu +// clients and therefore also better cache affinity while ensuring that we can // still benefit from spreading the load across replicas for other processes // and applications. -int InitRandomSelectionInt() { - std::random_device rdev; - std::mt19937 gen(rdev()); - return gen(); +uint32_t GetReplicaRandomSelection() { + // Initialization of function-local statics is guaranteed to occur only once + // even when called from multiple threads, and may be more efficient than + // the equivalent code using std::call_once [1]. + // + // [1] 'Notes' section at https://en.cppreference.com/w/cpp/thread/call_once + static const uint32_t kRandomSelection = + std::mt19937 {std::random_device {}()}(); + return kRandomSelection; } -static const int kRandomSelectionInt = InitRandomSelectionInt(); } // anonymous namespace @@ -267,12 +273,13 @@ RemoteTabletServer* KuduClient::Data::SelectTServer( same_location.push_back(rts); } } + static const size_t kRandomSelection = GetReplicaRandomSelection(); if (!local.empty()) { - ret = local[kRandomSelectionInt % local.size()]; + ret = local[kRandomSelection % local.size()]; } else if (!same_location.empty()) { - ret = same_location[kRandomSelectionInt % same_location.size()]; + ret = same_location[kRandomSelection % same_location.size()]; } else if (!filtered.empty()) { - ret = filtered[kRandomSelectionInt % filtered.size()]; + ret = filtered[kRandomSelection % filtered.size()]; } break; }