feat: Consistent sharding with bounded loads #16564

akram · 2023-12-07T00:09:33Z

Checklist:

codecov · 2023-12-07T00:18:40Z

Codecov Report

Attention: Patch coverage is 75.60976% with 10 lines in your changes are missing coverage. Please review.

Project coverage is 49.27%. Comparing base (f87897c) to head (9b2ad1f).
Report is 113 commits behind head on master.

❗ Current head 9b2ad1f differs from pull request most recent head ddc0a8e. Consider uploading reports for the commit ddc0a8e to get more accurate results

Files	Patch %	Lines
controller/sharding/sharding.go	75.60%	8 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #16564      +/-   ##
==========================================
- Coverage   49.73%   49.27%   -0.46%     
==========================================
  Files         274      274              
  Lines       48948    48211     -737     
==========================================
- Hits        24343    23755     -588     
+ Misses      22230    22107     -123     
+ Partials     2375     2349      -26

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

akram · 2023-12-07T07:32:30Z

Enhancement proposal is here #16570 and it has been exposed briefly during several calls, but, we will discuss more deeply

controller/sharding/consistent/consistent.go

controller/sharding/sharding.go

Enclavet · 2024-03-11T16:51:19Z

@akram Suggested some changes above. I tested with the various changes with 50 clusters 10k apps randomly distributed across the clusters. Results were consistent with the blog post:

Cluster/Shard reassignments when switching from 10 to 9 replicas:
11 changes - consistent hash
37 changes - round-robin

Max/Min Avg CPU usage across the shards when syncing:
0.77 to 0.44 - consistent hash
1.11 to 0.03 - round-robin (this was a big discrepancy that can occur if one shard is assigned a cluster with very little apps vs the other shards getting a cluster with lots of apps)

Let me know if you have any questions on the changes.

akram · 2024-04-15T20:34:58Z

@ishitasequeira can you PTAL ?

Enclavet · 2024-04-26T04:27:00Z

@akram Tested with your latest commits and everything is consistent with the previous testing/blog post:

Cluster/Shard reassignments when switching from 10 to 9 replicas:
10 changes - consistent hash
37 changes - round-robin

Max/Min Avg CPU usage across the shards when syncing:
0.73 to 0.34 - consistent hash
1.01 to 0.30 - round-robin

Let me know if you have any questions on the testing.

Signed-off-by: Akram Ben Aissi <akram.benaissi@gmail.com>

- The assignment or running of the algorithm has to be consistent across all the clusters. Changed the function to return a map where the consistent hash will be used to build the map - Modifications to the createConsistentHashsingWithBoundLoads function. This will create the map for cluster to shard. Note that the list must be consistent across all shards so that is why the cluster list must be sorted before going through the consistent hash algorithm Signed-off-by: Akram Ben Aissi <akram.benaissi@gmail.com>

ishitasequeira

Overall PR looks good to me!! Left a few nits and a question.

ishitasequeira · 2024-05-08T02:27:01Z

controller/sharding/consistent/consistent.go

+	if avgLoadPerNode == 0 {
+		avgLoadPerNode = 1
+	}
+	avgLoadPerNode = math.Ceil(avgLoadPerNode * 1.25)


Is 1.25 always going to be constant or can this be configurable? Never the less, should we move this value to a constant?

For now I would move this to a constant and not make it configurable. 1.25 is the load factor described by the original paper that the algorithm is based on and was found to be the ideal balance between keeping the shards uniform while also keeping consistency when changing shard numbers.

ishitasequeira · 2024-05-08T02:31:58Z

controller/sharding/consistent/consistent.go

+	if float64(bserver.Load)+1 <= avgLoadPerNode {
+		return true
+	}
+
+	return false


nit:

Suggested change

if float64(bserver.Load)+1 <= avgLoadPerNode {

return true

}

return false

return float64(bserver.Load)+1 <= avgLoadPerNode

ishitasequeira · 2024-05-08T02:43:49Z

common/common.go

+	// ConsistentHashingWithBoundedLoadsAlgorithm uses an algorithm that tries to use an equal distribution accross
+	// all shards but is optimised to handled sharding and/or cluster addings or removal. In case of sharding or
+	// cluster changes, this algorithm minimise the changes between shard and clusters assignments.


nit:

Suggested change

// ConsistentHashingWithBoundedLoadsAlgorithm uses an algorithm that tries to use an equal distribution accross

// all shards but is optimised to handled sharding and/or cluster addings or removal. In case of sharding or

// cluster changes, this algorithm minimise the changes between shard and clusters assignments.

// ConsistentHashingWithBoundedLoadsAlgorithm uses an algorithm that tries to use an equal distribution across

// all shards but is optimised to handle sharding and/or cluster addition or removal. In case of sharding or

// cluster changes, this algorithm minimises the changes between shard and clusters assignments.

akram requested review from a team as code owners December 7, 2023 00:09

akram changed the title ~~Consistent sharding with bounded loads~~ feat: Consistent sharding with bounded loads Dec 7, 2023

akram force-pushed the consistent-sharding-with-bounded-loads-akram branch from 629cf2a to a2714e0 Compare December 7, 2023 00:59

akram marked this pull request as draft December 7, 2023 07:50

akram force-pushed the consistent-sharding-with-bounded-loads-akram branch from a2714e0 to 5350c9c Compare December 11, 2023 13:46

akram marked this pull request as ready for review December 11, 2023 13:48

akram force-pushed the consistent-sharding-with-bounded-loads-akram branch 8 times, most recently from 9d86b5c to 08e7298 Compare January 17, 2024 14:17

akram force-pushed the consistent-sharding-with-bounded-loads-akram branch 4 times, most recently from 5cf46ad to dfc7af1 Compare January 25, 2024 15:50

Enclavet mentioned this pull request Jan 27, 2024

feat: Add app data to sharding cache to allow sharding by apps #17014

Merged

14 tasks

akram force-pushed the consistent-sharding-with-bounded-loads-akram branch 2 times, most recently from e2d5bf7 to d143e9f Compare January 29, 2024 16:00

akram force-pushed the consistent-sharding-with-bounded-loads-akram branch 2 times, most recently from 9b2ad1f to 20581cf Compare February 7, 2024 04:41

akram force-pushed the consistent-sharding-with-bounded-loads-akram branch from 20581cf to ca5fadf Compare February 26, 2024 06:18

akram force-pushed the consistent-sharding-with-bounded-loads-akram branch from ca5fadf to d20f4f3 Compare March 4, 2024 16:24

Enclavet reviewed Mar 11, 2024

View reviewed changes

controller/sharding/consistent/consistent.go Outdated Show resolved Hide resolved

Enclavet reviewed Mar 11, 2024

View reviewed changes

controller/sharding/sharding.go Outdated Show resolved Hide resolved

Enclavet reviewed Mar 11, 2024

View reviewed changes

controller/sharding/sharding.go Outdated Show resolved Hide resolved

akram force-pushed the consistent-sharding-with-bounded-loads-akram branch from d20f4f3 to 104d07d Compare March 26, 2024 05:20

agilgur5 mentioned this pull request Apr 9, 2024

Horizontally Scalable Controller argoproj/argo-workflows#9990

Open

akram force-pushed the consistent-sharding-with-bounded-loads-akram branch 3 times, most recently from 10934e9 to e747864 Compare April 15, 2024 19:09

akram force-pushed the consistent-sharding-with-bounded-loads-akram branch 5 times, most recently from 766419e to 04d265f Compare April 22, 2024 12:12

akram force-pushed the consistent-sharding-with-bounded-loads-akram branch 2 times, most recently from 1cfef01 to 2258479 Compare April 25, 2024 15:09

akram force-pushed the consistent-sharding-with-bounded-loads-akram branch 3 times, most recently from e8a5098 to 8e61149 Compare April 29, 2024 12:47

akram added 2 commits April 30, 2024 10:39

Adds consistent hashing with bound loads sharding algorithm

abc8aed

Signed-off-by: Akram Ben Aissi <akram.benaissi@gmail.com>

akram force-pushed the consistent-sharding-with-bounded-loads-akram branch from 8e61149 to ddc0a8e Compare April 30, 2024 09:39

ishitasequeira reviewed May 8, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Consistent sharding with bounded loads #16564

feat: Consistent sharding with bounded loads #16564

akram commented Dec 7, 2023 •

edited

codecov bot commented Dec 7, 2023 •

edited

akram commented Dec 7, 2023

Enclavet commented Mar 11, 2024 •

edited

akram commented Apr 15, 2024

Enclavet commented Apr 26, 2024

ishitasequeira left a comment

ishitasequeira May 8, 2024

andklee May 9, 2024

ishitasequeira May 8, 2024

ishitasequeira May 8, 2024

feat: Consistent sharding with bounded loads #16564

Are you sure you want to change the base?

feat: Consistent sharding with bounded loads #16564

Conversation

akram commented Dec 7, 2023 • edited

codecov bot commented Dec 7, 2023 • edited

Codecov Report

akram commented Dec 7, 2023

Enclavet commented Mar 11, 2024 • edited

akram commented Apr 15, 2024

Enclavet commented Apr 26, 2024

ishitasequeira left a comment

Choose a reason for hiding this comment

ishitasequeira May 8, 2024

Choose a reason for hiding this comment

andklee May 9, 2024

Choose a reason for hiding this comment

ishitasequeira May 8, 2024

Choose a reason for hiding this comment

ishitasequeira May 8, 2024

Choose a reason for hiding this comment

akram commented Dec 7, 2023 •

edited

codecov bot commented Dec 7, 2023 •

edited

Enclavet commented Mar 11, 2024 •

edited