Added Ring hash selection strategy #4831

Bue-von-hon · 2023-04-17T07:38:05Z

Motivation:

this Resolves #4698 issue

Added Ring hash strategy for For a broader selection.

Motification:

Added(RingHashEndpointSelectionStrategy): Implement a ring hash using the md5 hash algorithm and a ConcurrentSkipListMap

Result:

For now on, users will be able to use Ring hashes for load balancing.

Motivation: this Resolves line#4698 issue Added Ring hash strategy for For a broader selection. Motification: - Added(RingHashEndpointSelectionStrategy): Implement a ring hash using the md5 hash algorithm and a ConcurrentSkipListMap Result: For now on, users will be able to use Ring hashes for load balancing.

Bue-von-hon · 2023-04-20T06:15:25Z

Looking at the wighted round robin,
it looks like we need to identify the keys via host and port, am I understanding this correctly?🤔

ikhoon · 2023-04-30T09:02:01Z

core/src/main/java/com/linecorp/armeria/client/endpoint/RingHashEndpointSelectionStrategy.java

+                    final int weight = endpoint.weight();
+                    final String host = endpoint.host();
+                    // If weight is 3, place 3 times in the ring, if weight is 1, place once in the ring
+                    for (int i = 0; i < weight; i++) {


The weight should be normalized to reduce the size of ring. The default weight of an Endpoint is 1000. If all endpoints have the default weight (1000), only one entry for Endpoint should be enough. 30k elements will be added to ring if we have 30 endpoints with 1000 weight which would be a waste of memory.

It is necessary to set the maximum ring size so that the size of the ring is not too large. If the GCM of weights is too small and exceeds the maximum ring size, it seems that you need to divide the value by setting an appropriately large number.

Respecting your feedback, �I have implemented two methods: binary search with normalized weights and GCD.
However, I think a binary search, as implemented in the weighted round robin strategy, is sufficient.
so how about not using GCD but using binary search only?

ikhoon · 2023-04-30T09:06:48Z

core/build.gradle

@@ -100,6 +100,9 @@ dependencies {
    // Caffeine
    implementation libs.caffeine

+    // https://mvnrepository.com/artifact/net.openhft/zero-allocation-hashing
+    implementation 'net.openhft:zero-allocation-hashing:0.16'


What do you think of forking https://github.com/OpenHFT/Zero-Allocation-Hashing/blob/ea/src/main/java/net/openhft/hashing/XxHash.java to our code base or shading and timing the library?

We only use XxHash but Zero-Allocation-Hashing has additional hash functions.

We don't expose a utility library as a transitive dependency in the POM file.

Forked XxHash into our code base.
But due to the complex inheritance, several new files have been created.

trustin

Thanks for your pull request, @Bue-von-hon! Could you add some test cases?

trustin · 2023-05-02T12:31:05Z

core/src/main/java/com/linecorp/armeria/client/endpoint/RingHashEndpointSelectionStrategy.java

@@ -0,0 +1,86 @@
+/*
+ * Copyright 2016 LINE Corporation


Suggested change

* Copyright 2016 LINE Corporation

* Copyright 2023 LINE Corporation

core/src/main/java/com/linecorp/armeria/client/endpoint/RingHashEndpointSelectionStrategy.java

trustin · 2023-05-02T12:33:02Z

core/src/main/java/com/linecorp/armeria/client/endpoint/RingHashEndpointSelectionStrategy.java

+        }
+
+        @Override
+        public @Nullable Endpoint selectNow(ClientRequestContext ctx) {


We probably need @Nullable here because its super class has @Nullable already on this method? (We may want to change this, but at least for now)

removed @nullable annotation!

trustin · 2023-05-02T12:33:53Z

core/src/main/java/com/linecorp/armeria/client/endpoint/RingHashEndpointSelectionStrategy.java

+            final WeightedRingEndpoint weightedRingEndpoint = this.weightedRingEndpoint;
+            return weightedRingEndpoint.select(ctx.endpoint());


Q: Is there any chance of weightedRingEndpoint being null? For example, the listener you added in the construtor might not be notified early enough.

I understand the possibility that it could be null.
I saw code in the WeightedRoundRobinStrategy class that prepared for the same possibility, and I borrowed it.

trustin · 2023-05-02T12:34:51Z

core/src/main/java/com/linecorp/armeria/client/endpoint/RingHashEndpointSelectionStrategy.java

+
+            Endpoint select(Endpoint point) {
+                final int key = getXXHash(point.host());
+                final SortedMap<Integer, Endpoint> tailMap = ring.tailMap(key);


Could we use the Int2ObjectSortedMap instead?

…shEndpointSelectionStrategy.java Co-authored-by: Trustin Lee <[email protected]>

Motivation: Based on Code Review we have problems like below. There was an issue with too many endpoints being placed on the ring, and the WeightedRingEndpoint could be null in the selectNow method. typo in the copyright. unnecessary dependencies in the build.gradle. We only use XxHash but Zero-Allocation-Hashing has additional hash functions. this commit resolves these problems. Modification: Fix(build.gradle): remove zero-allocation-hashing dependency Add(Access.java, ByteBufferAccess.java, CharSequenceAccess.java, LongHashFunction.java, Primitives.java, UnsafeAccess.java, Util.java, XxHash.java): fork Xxhash Related Code to our code base Fix(RingHashEndpointSelectionStrategy.java): fix typo and add a GCD and binary search to find an x that will evenly distribute the endpoints over the ring. Add filtering to use only weights > 0. Prevent weightedRingEndpoint being null. Remove @nullable based on code review. Result: Solved the problem of too many endpoints being placed in a ring with GCD and binary search. Prepared for the possibility of a null WeightedRingEndpoint in the selectNow method. Fixed typo in copyright. Removed unnecessary dependencies in build.gradle by forking only the necessary code.

trustin · 2023-05-08T10:42:42Z

Thanks for continuing updating this PR. Please feel free to let us know when it's ready for reviews. If you feel it's not yet, please consider switching it to a draft PR. 🙇

Bue-von-hon · 2023-05-08T14:14:39Z

Thanks for continuing updating this PR. Please feel free to let us know when it's ready for reviews. If you feel it's not yet, please consider switching it to a draft PR. 🙇

Thanks for pointing out this great feature.
It turned out to be longer than I thought, so I fixed it to draft status

Bue-von-hon

Reviews all comment! PTAL🙏 @trustin @ikhoon

p.s.
i have problem with sun.misc.Unsafe pakage.
This package should be imported, but it didn't exist in our tests.(so that, i impleamated only one test😭)
I'm worried about whether or not to fork this one because I've gotten reviews before saying to be careful about importing dependencies.(in addition I'm not sure how to import dependencies in this project.)

Bue-von-hon · 2023-05-06T10:42:48Z

core/build.gradle

@@ -100,6 +100,9 @@ dependencies {
    // Caffeine
    implementation libs.caffeine

+    // https://mvnrepository.com/artifact/net.openhft/zero-allocation-hashing
+    implementation 'net.openhft:zero-allocation-hashing:0.16'


Forked XxHash into our code base.
But due to the complex inheritance, several new files have been created.

Bue-von-hon · 2023-05-06T10:59:18Z

core/src/main/java/com/linecorp/armeria/client/endpoint/RingHashEndpointSelectionStrategy.java

+            final WeightedRingEndpoint weightedRingEndpoint = this.weightedRingEndpoint;
+            return weightedRingEndpoint.select(ctx.endpoint());


I understand the possibility that it could be null.
I saw code in the WeightedRoundRobinStrategy class that prepared for the same possibility, and I borrowed it.

Bue-von-hon · 2023-05-06T11:10:37Z

core/src/main/java/com/linecorp/armeria/client/endpoint/RingHashEndpointSelectionStrategy.java

+
+            Endpoint select(Endpoint point) {
+                final int key = getXXHash(point.host());
+                final SortedMap<Integer, Endpoint> tailMap = ring.tailMap(key);


Bue-von-hon · 2023-05-07T03:26:28Z

core/src/main/java/com/linecorp/armeria/client/endpoint/RingHashEndpointSelectionStrategy.java

+        }
+
+        @Override
+        public @Nullable Endpoint selectNow(ClientRequestContext ctx) {


removed @nullable annotation!

Bue-von-hon · 2023-05-07T03:34:17Z

core/src/main/java/com/linecorp/armeria/client/endpoint/RingHashEndpointSelectionStrategy.java

+                    final int weight = endpoint.weight();
+                    final String host = endpoint.host();
+                    // If weight is 3, place 3 times in the ring, if weight is 1, place once in the ring
+                    for (int i = 0; i < weight; i++) {


Respecting your feedback, �I have implemented two methods: binary search with normalized weights and GCD.
However, I think a binary search, as implemented in the weighted round robin strategy, is sufficient.
so how about not using GCD but using binary search only?

ikhoon · 2023-05-16T07:09:14Z

core/src/main/java/com/linecorp/armeria/client/endpoint/RingHashEndpointSelectionStrategy.java

+            int getXXHash(String input) {
+                final byte[] inputBytes = input.getBytes(StandardCharsets.UTF_8);
+                final XxHash xxHash = new XxHash();
+                final long hashBytes = xxHash.hash(inputBytes, inputBytes.length);


What do you think of using Hashing.murmur3_32_fixed().hashBytes(inputBytes).asInt() instead at the moment? Many files are forked than I expected.

Adopting XxHash could be considered a separate issue. If we can take advantage of it, we need to shade and trim the library.

Thanks for the kind suggestion. i fixed it!

ikhoon · 2023-05-16T07:15:33Z

core/src/main/java/com/linecorp/armeria/client/endpoint/RingHashEndpointSelectionStrategy.java

+                return gcd;
+            }
+
+            int gcd(int a, int b) {


Should we replace this tail recursion with iteration? Modern IDE supports automatic conversion.

core/src/main/java/com/linecorp/armeria/client/endpoint/RingHashEndpointSelectionStrategy.java

ikhoon · 2023-05-16T07:24:21Z

core/src/main/java/com/linecorp/armeria/client/endpoint/RingHashEndpointSelectionStrategy.java

+                // When the size of the GCD is too small and exceeds the size of the ring,
+                // using binary search for find x where Σ (w[i] / x) ≤ ring_size, w[i] is endpoint's weight at index i
+                List<Integer> arr = new ArrayList<>();
+                for (Endpoint endpoint : this.endpoints) {
+                    int weight = endpoint.weight();
+                    arr.add(weight);
+                }


Don't we need to start else block here?

i added it!

core/src/main/java/com/linecorp/armeria/client/endpoint/RingHashEndpointSelectionStrategy.java

ikhoon · 2023-05-16T07:37:37Z

core/src/main/java/com/linecorp/armeria/client/endpoint/RingHashEndpointSelectionStrategy.java

+                    }
+                }
+
+                // When the size of the GCD is too small and exceeds the size of the ring,


I don't see the size of the ring is a reasonable limitation for the maximum size of the ring. Some users might want to use more memory to get exact weighted balancing.

What do you think of taking the maximum size of the ring as an input parameter?

ikhoon · 2023-05-16T07:39:50Z

...rc/test/java/com/linecorp/armeria/client/endpoint/RingHashEndpointSelectionStrategyTest.java

+    void select() {
+        assertThat(EndpointGroup.of(Endpoint.parse("localhost:1234"),
+                                    Endpoint.parse("localhost:2345"))
+                                .selectNow(ctx)).isNotNull();


Is this selection use RingHashEndpointSelectionStrategy? How do users use RingHashEndpointSelectionStrategy for their load-balancing strategy? 🤔

I think your suggestions would be very convenient for users if implemented.
However, in the existing interface, the size option is not taken into account, which makes the implementation a bit difficult for me.
I would appreciate any ideas on how to incorporate a size option.

…shEndpointSelectionStrategy.java Co-authored-by: Ikhun Um <[email protected]>

Motivation: Removed unnecessary files and added test case and fixed bugs inherent in the algorithm. Removed the use of the xxHash library. Modification: - Fix(EndpointSelectionStrategy): Add static factory methods - Fix(RingHashEndpointSelectionStrategy): Fixed a bug where ring hash select resulted in the same key every time. fixed invalid method names and missing tail recursion and added eles statements - Fix(RingHashEndpointSelectionStrategyTest): Added test case - Remove(Access, CharSequenceAccess, Primitives, UnsafeAccess, XxHash): delete unused file

Bue-von-hon · 2023-11-22T00:32:10Z

gentle ping @trustin @ikhoon @jrhee17 @minwoox

Bue-von-hon · 2023-12-06T07:29:23Z

gentle ping @trustin @ikhoon @jrhee17 @minwoox

Bue-von-hon · 2024-09-15T06:44:25Z

I fixed some minor mistakes, changed the hashing algorithm, and of course shaded the dependencies.
PTAL 🙏

I just realized a mistake in the shading implementation and am fixing it.

Bue-von-hon · 2024-09-15T12:34:13Z

@ikhoon @trustin @minwoox @jrhee17
finally fixed the shading feature!
I actually unzipped it to check it out and it looks like this.
PTAL 🙏

unzip -l build/libs/armeria-shaded-1.30.2-SNAPSHOT.jar | grep openhft
        0  09-15-2024 21:15   com/linecorp/armeria/internal/shaded/openhft/
      256  09-15-2024 21:15   com/linecorp/armeria/internal/shaded/openhft/Access$1.class
     3183  09-15-2024 21:15   com/linecorp/armeria/internal/shaded/openhft/Access$ReverseAccess.class
     3538  09-15-2024 21:15   com/linecorp/armeria/internal/shaded/openhft/Access.class
     1539  09-15-2024 21:15   com/linecorp/armeria/internal/shaded/openhft/LongHashFunction.class
      268  09-15-2024 21:15   com/linecorp/armeria/internal/shaded/openhft/Primitives$1.class
      786  09-15-2024 21:15   com/linecorp/armeria/internal/shaded/openhft/Primitives$ByteOrderHelper.class
      894  09-15-2024 21:15   com/linecorp/armeria/internal/shaded/openhft/Primitives$ByteOrderHelperReverse.class
     1420  09-15-2024 21:15   com/linecorp/armeria/internal/shaded/openhft/Primitives.class
      274  09-15-2024 21:15   com/linecorp/armeria/internal/shaded/openhft/UnsafeAccess$1.class
     1156  09-15-2024 21:15   com/linecorp/armeria/internal/shaded/openhft/UnsafeAccess$OldUnsafeAccessBigEndian.class
     1169  09-15-2024 21:15   com/linecorp/armeria/internal/shaded/openhft/UnsafeAccess$OldUnsafeAccessLittleEndian.class
     4253  09-15-2024 21:15   com/linecorp/armeria/internal/shaded/openhft/UnsafeAccess.class
     1717  09-15-2024 21:15   com/linecorp/armeria/internal/shaded/openhft/XxHash$AsLongHashFunction.class
     2833  09-15-2024 21:15   com/linecorp/armeria/internal/shaded/openhft/XxHash.class

Bue-von-hon · 2024-10-02T11:31:05Z

gentle ping @trustin @ikhoon @jrhee17 @minwoox

Bue-von-hon requested review from ikhoon, jrhee17, minwoox and trustin as code owners April 17, 2023 07:38

Bue-von-hon mentioned this pull request Apr 17, 2023

Add the Ring hash selection strategy #4698

Open

Bue-von-hon added 2 commits April 19, 2023 22:33

Added zero-allocation-hashing

0613dcc

Replace md5 with xxHash

24d14d0

ikhoon reviewed Apr 30, 2023

View reviewed changes

trustin reviewed May 2, 2023

View reviewed changes

Bue-von-hon and others added 3 commits May 6, 2023 19:43

Update core/src/main/java/com/linecorp/armeria/client/endpoint/RingHa…

f51f6b9

…shEndpointSelectionStrategy.java Co-authored-by: Trustin Lee <[email protected]>

Merge remote-tracking branch 'origin/Add-RingHash' into Add-RingHash

08bc7a5

Bue-von-hon marked this pull request as draft May 8, 2023 14:12

Merge branch 'main' into Add-RingHash

dcb9239

Bue-von-hon added 2 commits May 13, 2023 20:41

Add test for Ring hash and Remove unused class

db068ed

Merge remote-tracking branch 'origin/Add-RingHash' into Add-RingHash

c7d55db

Bue-von-hon commented May 13, 2023

View reviewed changes

Bue-von-hon marked this pull request as ready for review May 13, 2023 11:50

ikhoon reviewed May 16, 2023

View reviewed changes

ikhoon added the new feature label May 16, 2023

ikhoon reviewed May 16, 2023

View reviewed changes

Bue-von-hon and others added 4 commits May 30, 2023 16:54

Update core/src/main/java/com/linecorp/armeria/client/endpoint/RingHa…

385b8ce

…shEndpointSelectionStrategy.java Co-authored-by: Ikhun Um <[email protected]>

Update core/src/main/java/com/linecorp/armeria/client/endpoint/RingHa…

43543e1

…shEndpointSelectionStrategy.java Co-authored-by: Ikhun Um <[email protected]>

Update core/src/main/java/com/linecorp/armeria/client/endpoint/RingHa…

5b8951a

…shEndpointSelectionStrategy.java Co-authored-by: Ikhun Um <[email protected]>

Bue-von-hon marked this pull request as draft June 10, 2023 10:00

Bue-von-hon requested review from trustin and ikhoon November 22, 2023 00:32

Bue-von-hon marked this pull request as draft February 28, 2024 01:08

Bue-von-hon added 2 commits March 21, 2024 16:32

Merge branch 'main' into Add-RingHash

629f9a7

Merge branch 'main' into Add-RingHash

a66b122

github-actions bot added the Stale label Apr 29, 2024

Merge branch 'main' into Add-RingHash

54470f8

github-actions bot removed the Stale label Aug 17, 2024

Bue-von-hon and others added 8 commits September 14, 2024 22:19

Merge branch 'line:main' into Add-RingHash

378c683

Improve readability

996f2e7

Sort weights in ascending order

291ac95

Add a separator

7ab979a

Fix typo

d956d18

Fix typo and improve readability

7c1469b

Add nullable

aa9f1a5

Changing the hashing algorithm and shading dependencies

1a64c77

Bue-von-hon marked this pull request as ready for review September 15, 2024 06:42

Fix lint

e83bb86

Bue-von-hon marked this pull request as draft September 15, 2024 11:40

Fix trim and shade

cb048d3

Bue-von-hon marked this pull request as ready for review September 15, 2024 12:34

Merge branch 'main' into Add-RingHash

28faf16

Bue-von-hon added 3 commits October 2, 2024 20:31

Merge branch 'main' into Add-RingHash

0f12d26

Merge branch 'main' into Add-RingHash

8ec1584

Merge branch 'main' into Add-RingHash

1270d43

	* Copyright 2016 LINE Corporation
	* Copyright 2023 LINE Corporation

		final WeightedRingEndpoint weightedRingEndpoint = this.weightedRingEndpoint;
		return weightedRingEndpoint.select(ctx.endpoint());

Added Ring hash selection strategy #4831

Are you sure you want to change the base?

Added Ring hash selection strategy #4831

Conversation

Bue-von-hon commented Apr 17, 2023

Bue-von-hon commented Apr 20, 2023

ikhoon Apr 30, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

trustin left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

trustin commented May 8, 2023

Bue-von-hon commented May 8, 2023

Bue-von-hon left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Bue-von-hon commented Nov 22, 2023

Bue-von-hon commented Dec 6, 2023

Bue-von-hon commented Sep 15, 2024 • edited Loading

Bue-von-hon commented Sep 15, 2024

Bue-von-hon commented Oct 2, 2024

ikhoon Apr 30, 2023 •

edited

Loading

trustin left a comment •

edited

Loading

Bue-von-hon left a comment •

edited

Loading

Bue-von-hon commented Sep 15, 2024 •

edited

Loading