[ring hash] revert 64-to-32-bit changes for objc (#33201)

This reverts the changes made to ring_hash in #29872. The comment in the
picker code specifically says not to change these variables to an
unsigned type, but that's exactly what that PR did. I don't know if this
has actually been causing any problems, but given that we duplicated
this algorithm (and that comment) from elsewhere without doing a
detailed analysis of it, it seems prudent to stick with the types that
the original code suggested were important.

To avoid causing problems for ObjC, I have changed this such that
ring_hash is not built unless we are building with xDS support, which we
exclude on mobile. Currently, there is no way to use ring_hash without
xDS, although that might change in the future; if it does, we can deal
with any problems that arise at that point.
pull/33209/head
Mark D. Roth 2 years ago committed by GitHub
parent f3574e3f64
commit 9d03117fbc
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 2
      BUILD
  2. 1
      CMakeLists.txt
  3. 2
      Makefile
  4. 3
      build_autogenerated.yaml
  5. 1
      grpc.gyp
  6. 10
      src/core/ext/filters/client_channel/lb_policy/ring_hash/ring_hash.cc
  7. 2
      src/core/plugin_registry/grpc_plugin_registry.cc
  8. 2
      src/core/plugin_registry/grpc_plugin_registry_extra.cc

@ -585,6 +585,7 @@ GRPC_XDS_TARGETS = [
"//src/core:grpc_lb_policy_xds_cluster_resolver",
"//src/core:grpc_lb_policy_xds_override_host",
"//src/core:grpc_lb_policy_xds_wrr_locality",
"//src/core:grpc_lb_policy_ring_hash",
"//src/core:grpc_resolver_xds",
"//src/core:grpc_resolver_c2p",
"//src/core:grpc_xds_server_config_fetcher",
@ -807,7 +808,6 @@ grpc_cc_library(
"//src/core:grpc_lb_policy_outlier_detection",
"//src/core:grpc_lb_policy_pick_first",
"//src/core:grpc_lb_policy_priority",
"//src/core:grpc_lb_policy_ring_hash",
"//src/core:grpc_lb_policy_round_robin",
"//src/core:grpc_lb_policy_weighted_round_robin",
"//src/core:grpc_lb_policy_weighted_target",

1
CMakeLists.txt generated

@ -2645,7 +2645,6 @@ add_library(grpc_unsecure
src/core/ext/filters/client_channel/lb_policy/outlier_detection/outlier_detection.cc
src/core/ext/filters/client_channel/lb_policy/pick_first/pick_first.cc
src/core/ext/filters/client_channel/lb_policy/priority/priority.cc
src/core/ext/filters/client_channel/lb_policy/ring_hash/ring_hash.cc
src/core/ext/filters/client_channel/lb_policy/rls/rls.cc
src/core/ext/filters/client_channel/lb_policy/round_robin/round_robin.cc
src/core/ext/filters/client_channel/lb_policy/weighted_round_robin/static_stride_scheduler.cc

2
Makefile generated

@ -1879,7 +1879,6 @@ LIBGRPC_UNSECURE_SRC = \
src/core/ext/filters/client_channel/lb_policy/outlier_detection/outlier_detection.cc \
src/core/ext/filters/client_channel/lb_policy/pick_first/pick_first.cc \
src/core/ext/filters/client_channel/lb_policy/priority/priority.cc \
src/core/ext/filters/client_channel/lb_policy/ring_hash/ring_hash.cc \
src/core/ext/filters/client_channel/lb_policy/rls/rls.cc \
src/core/ext/filters/client_channel/lb_policy/round_robin/round_robin.cc \
src/core/ext/filters/client_channel/lb_policy/weighted_round_robin/static_stride_scheduler.cc \
@ -3039,6 +3038,7 @@ ifneq ($(OPENSSL_DEP),)
# This is to ensure the embedded OpenSSL is built beforehand, properly
# installing headers to their final destination on the drive. We need this
# otherwise parallel compilation will fail if a source is compiled first.
src/core/ext/filters/client_channel/lb_policy/ring_hash/ring_hash.cc: $(OPENSSL_DEP)
src/core/ext/filters/client_channel/lb_policy/xds/cds.cc: $(OPENSSL_DEP)
src/core/ext/filters/client_channel/lb_policy/xds/xds_attributes.cc: $(OPENSSL_DEP)
src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_impl.cc: $(OPENSSL_DEP)

@ -1953,7 +1953,6 @@ libs:
- src/core/ext/filters/client_channel/lb_policy/oob_backend_metric.h
- src/core/ext/filters/client_channel/lb_policy/oob_backend_metric_internal.h
- src/core/ext/filters/client_channel/lb_policy/outlier_detection/outlier_detection.h
- src/core/ext/filters/client_channel/lb_policy/ring_hash/ring_hash.h
- src/core/ext/filters/client_channel/lb_policy/subchannel_list.h
- src/core/ext/filters/client_channel/lb_policy/weighted_round_robin/static_stride_scheduler.h
- src/core/ext/filters/client_channel/local_subchannel_pool.h
@ -2332,7 +2331,6 @@ libs:
- src/core/tsi/transport_security.h
- src/core/tsi/transport_security_grpc.h
- src/core/tsi/transport_security_interface.h
- third_party/xxhash/xxhash.h
src:
- src/core/ext/filters/backend_metrics/backend_metric_filter.cc
- src/core/ext/filters/census/grpc_context.cc
@ -2362,7 +2360,6 @@ libs:
- src/core/ext/filters/client_channel/lb_policy/outlier_detection/outlier_detection.cc
- src/core/ext/filters/client_channel/lb_policy/pick_first/pick_first.cc
- src/core/ext/filters/client_channel/lb_policy/priority/priority.cc
- src/core/ext/filters/client_channel/lb_policy/ring_hash/ring_hash.cc
- src/core/ext/filters/client_channel/lb_policy/rls/rls.cc
- src/core/ext/filters/client_channel/lb_policy/round_robin/round_robin.cc
- src/core/ext/filters/client_channel/lb_policy/weighted_round_robin/static_stride_scheduler.cc

1
grpc.gyp generated

@ -1121,7 +1121,6 @@
'src/core/ext/filters/client_channel/lb_policy/outlier_detection/outlier_detection.cc',
'src/core/ext/filters/client_channel/lb_policy/pick_first/pick_first.cc',
'src/core/ext/filters/client_channel/lb_policy/priority/priority.cc',
'src/core/ext/filters/client_channel/lb_policy/ring_hash/ring_hash.cc',
'src/core/ext/filters/client_channel/lb_policy/rls/rls.cc',
'src/core/ext/filters/client_channel/lb_policy/round_robin/round_robin.cc',
'src/core/ext/filters/client_channel/lb_policy/weighted_round_robin/static_stride_scheduler.cc',

@ -360,12 +360,12 @@ RingHash::PickResult RingHash::Picker::Pick(PickArgs args) {
// Ported from https://github.com/RJ/ketama/blob/master/libketama/ketama.c
// (ketama_get_server) NOTE: The algorithm depends on using signed integers
// for lowp, highp, and first_index. Do not change them!
size_t lowp = 0;
size_t highp = ring.size();
size_t first_index = 0;
int64_t lowp = 0;
int64_t highp = ring.size();
int64_t first_index = 0;
while (true) {
first_index = (lowp + highp) / 2;
if (first_index == ring.size()) {
if (first_index == static_cast<int64_t>(ring.size())) {
first_index = 0;
break;
}
@ -509,7 +509,7 @@ RingHash::RingHashSubchannelList::Ring::Ring(
std::ceil(min_normalized_weight * min_ring_size) / min_normalized_weight,
static_cast<double>(max_ring_size));
// Reserve memory for the entire ring up front.
const size_t ring_size = std::ceil(scale);
const uint64_t ring_size = std::ceil(scale);
ring_.reserve(ring_size);
// Populate the hash ring by walking through the (host, weight) pairs in
// normalized_host_weights, and generating (scale * weight) hashes for each

@ -62,7 +62,6 @@ extern void RegisterPickFirstLbPolicy(CoreConfiguration::Builder* builder);
extern void RegisterRoundRobinLbPolicy(CoreConfiguration::Builder* builder);
extern void RegisterWeightedRoundRobinLbPolicy(
CoreConfiguration::Builder* builder);
extern void RegisterRingHashLbPolicy(CoreConfiguration::Builder* builder);
extern void RegisterHttpProxyMapper(CoreConfiguration::Builder* builder);
#ifndef GRPC_NO_RLS
extern void RegisterRlsLbPolicy(CoreConfiguration::Builder* builder);
@ -85,7 +84,6 @@ void BuildCoreConfiguration(CoreConfiguration::Builder* builder) {
RegisterPickFirstLbPolicy(builder);
RegisterRoundRobinLbPolicy(builder);
RegisterWeightedRoundRobinLbPolicy(builder);
RegisterRingHashLbPolicy(builder);
BuildClientChannelConfiguration(builder);
SecurityRegisterHandshakerFactories(builder);
RegisterClientAuthorityFilter(builder);

@ -39,6 +39,7 @@ extern void RegisterXdsClusterResolverLbPolicy(
extern void RegisterXdsOverrideHostLbPolicy(
CoreConfiguration::Builder* builder);
extern void RegisterXdsWrrLocalityLbPolicy(CoreConfiguration::Builder* builder);
extern void RegisterRingHashLbPolicy(CoreConfiguration::Builder* builder);
extern void RegisterFileWatcherCertificateProvider(
CoreConfiguration::Builder* builder);
#endif
@ -60,6 +61,7 @@ void RegisterExtraFilters(CoreConfiguration::Builder* builder) {
RegisterXdsClusterResolverLbPolicy(builder);
RegisterXdsOverrideHostLbPolicy(builder);
RegisterXdsWrrLocalityLbPolicy(builder);
RegisterRingHashLbPolicy(builder);
RegisterFileWatcherCertificateProvider(builder);
#endif
}

Loading…
Cancel
Save