[kudu] branch master updated: KUDU-3248 improve replica selection initialization

[alexey]

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