Skip to content

net: Introduce round-robin connection balancing policy#3371

Open
tgrabiec wants to merge 1 commit into
scylladb:masterfrom
tgrabiec:round_robin
Open

net: Introduce round-robin connection balancing policy#3371
tgrabiec wants to merge 1 commit into
scylladb:masterfrom
tgrabiec:round_robin

Conversation

@tgrabiec

Copy link
Copy Markdown
Contributor

Useful for some protocols which assume this policy, like shard-aware CQL protocol, which keeps opening connections until it hits the desired shard. With the current default load balancing strategy, which picks the least-loaded shard, that is not guaranteed to converge and can live-lock if there is connection imbalance.

Useful for some protocols which assume this policy, like shard-aware
CQL protocol, which keeps opening connections until it hits the
desired shard. With the current default load balancing strategy, which
picks the least-loaded shard, that is not guaranteed to converge and
can live-lock if there is connection imbalance.
@tgrabiec

Copy link
Copy Markdown
Contributor Author

Needed by ScyllaDB: scylladb/scylladb#29687

@nyh nyh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good to me, had just some trivial nitpicks (and also there's some interaction with #3355 that somebody will need to fix, sooner or later).

port,
// This algorithm distributes all new connections to listen_options::fixed_cpu shard only.
fixed,
// This algorithm distributes new connections to shards in a round-robin fashion,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nitpick: typo in the commit message title ("balaning").

class conntrack {
class load_balancer {
std::vector<unsigned> _cpu_load;
unsigned _rr_counter = 0;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nitpick: If you're familiar with this code, you can guess that the acronym "rr" refers to round-robin, but it's not obvious. I would add a comment that this variable is used by load_balancing_algorithm::round_robin to choose the next shard.

return cpu;
}
shard_id next_cpu_rr() {
auto cpu = shard_id(_rr_counter % smp::count);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Note that in #3355 @avikivity deprecates smp::count, so this call will need to be fixed (in the same way Avi fixed the lines above).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants