Merge pull request #17531 from code-dot-org/revert-17519-netsim-jumphash

Revert "Internet Simulator: Distribute load across shards more evenly with JumpHash"

Gemfile

@@ -28,6 +28,7 @@ gem 'mysql2', '~> 0.3.13'
 # Ref: https://github.com/bdurand/seamless_database_pool/pull/39
 gem 'seamless_database_pool', github: 'wjordan/seamless_database_pool', ref: 'cdo'
 
+gem 'consistent-hashing', '~> 1.0.0'
 gem 'dalli' # memcached
 gem 'dalli-elasticache' # ElastiCache Auto Discovery memcached nodes
 gem 'google_drive'
@@ -37,8 +38,6 @@ gem 'parallel'
 gem 'redis', '~> 3.3.3'
 # Using commit ref on fork until maintainer publishes a new version.
 gem 'redis-slave-read', require: false, github: 'code-dot-org/redis-slave-read', ref: 'cfe1bd0f5cf65eee5b52560139cab133f22cb880'
-gem 'jumphash'
-gem 'xxhash'
 
 gem 'google-api-client'
 gem 'launchy' # Peer dependency of Google::APIClient::InstalledAppFlow

Gemfile.lock

@@ -197,9 +197,12 @@ GEM
       io-like (~> 0.3.0)
     arel (7.1.4)
     ast (2.3.0)
+    atomic (1.1.99)
     attr_required (1.0.1)
     auto_strip_attributes (2.1.0)
       activerecord (>= 3.0)
+    avl_tree (1.2.1)
+      atomic (~> 1.1)
     aws-sdk (2.9.14)
       aws-sdk-resources (= 2.9.14)
     aws-sdk-core (2.9.14)
@@ -252,6 +255,8 @@ GEM
       activerecord (~> 5.0.0)
     concurrent-ruby (1.0.5)
     connection_pool (2.2.1)
+    consistent-hashing (1.0.0)
+      avl_tree (>= 1.1.3)
     crack (0.4.3)
       safe_yaml (~> 1.0.0)
     cucumber (2.4.0)
@@ -398,7 +403,6 @@ GEM
       url_safe_base64
     jsonapi-serializers (1.0.0)
       activesupport
-    jumphash (0.1.0)
     jwt (1.5.4)
     kaminari (0.17.0)
       actionpack (>= 3.0.0)
@@ -740,7 +744,6 @@ GEM
     websocket-extensions (0.1.2)
     xpath (2.0.0)
       nokogiri (~> 1.3)
-    xxhash (0.4.0)
     youtube-dl.rb (0.3.1.2016.08.19)
       cocaine (>= 0.5.4)
 
@@ -767,6 +770,7 @@ DEPENDENCIES
   codemirror-rails
   colorize
   composite_primary_keys
+  consistent-hashing (~> 1.0.0)
   cucumber (~> 2.4.0)
   daemons
   dalli
@@ -800,7 +804,6 @@ DEPENDENCIES
   jbuilder (~> 2.5)
   jquery-rails
   jquery-ui-rails (~> 5.0.3)
-  jumphash
   jwt
   kaminari
   launchy
@@ -882,8 +885,7 @@ DEPENDENCIES
   vcr
   web-console
   webmock
-  xxhash
   youtube-dl.rb
 
 BUNDLED WITH
-   1.15.4
+   1.15.3

shared/middleware/helpers/sharded_redis_factory.rb

@@ -1,6 +1,5 @@
 require 'redis-slave-read'
-require 'jumphash'
-require 'xxhash'
+require 'consistent_hashing'
 
 # Helper for getting a correctly configured Redis client for a particular
 # shard key.  Can be configured for any number of shards, where each shard
@@ -34,16 +33,16 @@ class ShardedRedisFactory
   def initialize(shards, new_redis_proc = proc {|url| Redis.new(url: url)})
     raise ArgumentError.new('Must provide at least one shard') if shards.empty?
     @new_redis_proc = new_redis_proc
-    @shards = shards
+    @ring = ConsistentHashing::Ring.new
+    shards.each {|shard_config| @ring.add shard_config}
   end
 
   # The set of Redis node URLs to be used for the given shard key.
   #
   # @param [String] shard_key
   # @return [Hash<'master':String, 'read_replicas':String[]>]
   def client_for_key(shard_key)
-    index = JumpHash.hash_key(XXhash.xxh64(shard_key), @shards.size)
-    shard_config = @shards[index]
+    shard_config = @ring.node_for shard_key
     client_for_shard_config shard_config
   end
 

shared/test/middleware/helpers/test_sharded_redis_factory.rb

@@ -109,7 +109,7 @@ def test_two_replication_group_config
     assert_equal 'redis://replica1_1', client.slaves[0].url_according_to_test
     assert_equal 'redis://replica1_2', client.slaves[1].url_according_to_test
 
-    client = factory.client_for_key('shard 2')
+    client = factory.client_for_key('second shard')
     assert_equal 'redis://master2', client.master.url_according_to_test
     assert_equal 'redis://replica2_1', client.slaves[0].url_according_to_test
     assert_equal 'redis://replica2_2', client.slaves[1].url_according_to_test