Skip to content

Commit

Permalink
KUDU-3248 improve replica selection initialization
Browse files Browse the repository at this point in the history
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 ccbbfb3.

Change-Id: I11c2a29ef69a8c97c68330d261fdff64accebb0b
Reviewed-on: http://gerrit.cloudera.org:8080/19828
Reviewed-by: Abhishek Chennaka <[email protected]>
Reviewed-by: Wenzhe Zhou <[email protected]>
Tested-by: Alexey Serbin <[email protected]>
  • Loading branch information
alexeyserbin committed May 2, 2023
1 parent e89dfe9 commit 97edafc
Showing 1 changed file with 18 additions and 11 deletions.
29 changes: 18 additions & 11 deletions src/kudu/client/client-internal.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "kudu/client/client-internal.h"

#include <algorithm>
#include <cstddef>
#include <cstdint>
#include <functional>
#include <limits>
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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;
}
Expand Down

0 comments on commit 97edafc

Please sign in to comment.