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;
}
