Skip to content

experimental: Add Maglev support #35430

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Nov 22, 2024
Merged

experimental: Add Maglev support #35430

merged 1 commit into from
Nov 22, 2024

Conversation

DamianSawicki
Copy link
Contributor

@DamianSawicki DamianSawicki commented Oct 18, 2024 •
edited
Loading

This adds support for the Maglev load-balancing algorithm to the experimental StateDB load balancing.

Update: In addition to the original goal of the PR, which is contained in the first commit, @joamaki added additional commit(s) improving the existing Maglev implementation (support for concurrency, migration to hive cells, and better data structures to avoid rewriting data from one to another).

Please ensure your pull request adheres to the following guidelines:

  • For first time contributors, read Submitting a pull request
  • All code is covered by unit and/or runtime tests where feasible.
  • All commits contain a well written commit description including a title,
    description and a Fixes: #XXX line if the commit addresses a particular
    GitHub issue.
  • If your commit description contains a Fixes: <commit-id> tag, then
    please add the commit author[s] as reviewer[s] to this issue.
  • All commits are signed off. See the section Developer’s Certificate of Origin
  • Provide a title or release-note blurb suitable for the release notes.
  • Are you a user of Cilium? Please add yourself to the Users doc
  • Thanks for contributing!

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Oct 18, 2024
@github-actions github-actions bot added the kind/community-contribution This was a contribution made by a community member. label Oct 18, 2024
@DamianSawicki
Copy link
Contributor Author

@joamaki Please have a look.

@robscott The PR is not ready for review yet, but just FYI.

joamaki
joamaki reviewed Oct 21, 2024
View reviewed changes
joamaki
joamaki reviewed Oct 21, 2024
View reviewed changes
@DamianSawicki DamianSawicki force-pushed the maglevInStateDB branch 2 times, most recently from 4ec17c8 to 99d9f5f Compare October 21, 2024 16:04
@brb brb mentioned this pull request Oct 23, 2024
Merged
joamaki
joamaki reviewed Oct 25, 2024
View reviewed changes
@maintainer-s-little-helper

This comment was marked as outdated.

Sign in to view
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Oct 25, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Oct 25, 2024
@DamianSawicki DamianSawicki marked this pull request as ready for review October 25, 2024 14:35
@DamianSawicki DamianSawicki requested review from a team as code owners October 25, 2024 14:35
@joamaki joamaki added the release-note/misc This PR makes changes that have no direct user impact. label Oct 28, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Oct 28, 2024
@joamaki joamaki enabled auto-merge October 28, 2024 07:25
@joamaki
Copy link
Contributor

joamaki commented Oct 28, 2024

/test

auto-merge was automatically disabled October 28, 2024 11:43

Head branch was pushed to by a user without write access

@joamaki
Copy link
Contributor

joamaki commented Oct 28, 2024

/test

1 similar comment
@ovidiutirla
Copy link
Contributor

/test

@DamianSawicki DamianSawicki force-pushed the maglevInStateDB branch 3 times, most recently from 8d5dcca to 809a790 Compare November 21, 2024 18:03
@DamianSawicki
Copy link
Contributor Author

Ok, I removed commits that have been copied to #35885, addressed minor comments from derailed@ and myself, and rebased onto main.

@DamianSawicki
Copy link
Contributor Author

/test

@DamianSawicki DamianSawicki marked this pull request as ready for review November 21, 2024 18:13
@DamianSawicki
Copy link
Contributor Author

/test

2 similar comments
@DamianSawicki
Copy link
Contributor Author

/test

@DamianSawicki
Copy link
Contributor Author

/test

@DamianSawicki DamianSawicki requested review from a team as code owners November 22, 2024 10:11
@DamianSawicki
Copy link
Contributor Author

Similar as in #35810 (comment), the latest review requests to cilium/envoy and cilium/sig-servicemesh are for this change in pkg/ciliumenvoyconfig/exp_test.go, which is a subset of the changes in pkg/loadbalancer/experimental/script_test.go.

@DamianSawicki
Copy link
Contributor Author

/test

@DamianSawicki
experimental: Add Maglev support
c0e5099 
This adds support for the Maglev load-balancing algorithm to the
experimental StateDB load balancing.

Signed-off-by: Damian Sawicki <dsawicki@google.com>
Signed-off-by: Jussi Maki <jussi@isovalent.com>
@DamianSawicki
Copy link
Contributor Author

The last push is just squashing all commits into one. All tests were already green.

jrajahalme
jrajahalme approved these changes Nov 22, 2024
View reviewed changes
Copy link
Member

@jrajahalme jrajahalme left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Envoy related change seems OK :-)

@joamaki
Copy link
Contributor

joamaki commented Nov 22, 2024

/test

@joamaki joamaki enabled auto-merge November 22, 2024 14:51
@DamianSawicki
Copy link
Contributor Author

/ci-clustermesh
/ci-e2e-upgrade
/ci-ginkgo
/ci-integration

@DamianSawicki
Copy link
Contributor Author

/ci-clustermesh

@DamianSawicki
Copy link
Contributor Author

/ci-e2e-upgrade

@DamianSawicki
Copy link
Contributor Author

/ci-ginkgo

@DamianSawicki
Copy link
Contributor Author

/ci-integration

@joamaki joamaki added this pull request to the merge queue Nov 22, 2024
Merged via the queue into cilium:main with commit afd62a4 Nov 22, 2024
64 checks passed
@harsimran-pabla harsimran-pabla mentioned this pull request Nov 29, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joamaki joamaki joamaki approved these changes

@derailed derailed derailed approved these changes

@dylandreimerink dylandreimerink dylandreimerink approved these changes

@jrajahalme jrajahalme jrajahalme approved these changes

Assignees
No one assigned
Labels
kind/community-contribution This was a contribution made by a community member. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@DamianSawicki @joamaki @ovidiutirla @derailed @dylandreimerink @jrajahalme @julianwiedmann