-
Notifications
You must be signed in to change notification settings - Fork 17
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.
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
WIP: Consistent hashing for balancer #258
base: main
Are you sure you want to change the base?
Conversation
auto reals_wlc = reals_wlc_weight.find(key); | ||
if (reals_wlc != reals_wlc_weight.end() && reals_wlc->second.has_value()) | ||
{ | ||
effective_weight = reals_wlc->second.value(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll add comments that I've left at Levon's pr, since you will be merging his commit now:
We will have code duplication here with the following lines:
auto it = reals_enabled.find(key);
if (it != reals_enabled.end())
{
if (it->second.has_value())
{
effective_weight = it->second.value();
}
}
Mayve we can add a private method in balancer_t
, smth like
template <typename Map>
std::optional<typename Map::mapped_type::value_type> get_effective_weight(const Map& map, const balancer::real_key_global_t& key) const {
auto it = map.find(key);
if (it != map.end() && it->second.has_value()) {
return it->second.value();
}
return std::nullopt;
}
And then use it, for example, with compound initialization:
if (auto found_weight = get_effective_weight(reals_wlc_weight, key); found_weight) {
effective_weight = *found_weight;
}
uint32_t effective_weight = weight; | ||
{ | ||
auto it = reals_enabled.find(key); | ||
if (it != reals_enabled.end()) | ||
{ | ||
if (it->second.has_value()) | ||
{ | ||
effective_weight = it->second.value(); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
auto found_weight = get_effective_weight(reals_enabled, key);
uint32_t effective_weight = found_weight ? *found_weight : weight;
auto wlc_ratio = std::max(1.0, wlc_power * (1 - 1.0 * connections * weight_sum / connection_sum / weight)); | ||
auto wlc_weight = (uint32_t)(weight * wlc_ratio); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's obscure. I suggest:
auto scaled_connections = static_cast<double>(connections * weight_sum);
auto scaled_weight = static_cast<double>(connection_sum * weight);
double connection_ratio = scaled_connections / scaled_weight;
double wlc_ratio = std::max(min_ratio, wlc_power * (1.0 - connection_ratio));
auto new_weight = static_cast<uint32_t>(std::round(weight * wlc_ratio));
3a2c877
to
f575ea7
Compare
e8acf78
to
89adefa
Compare
89adefa
to
cd20fd0
Compare
dataplane/globalbase.cpp
Outdated
utils::Deferer defer([]() { YADECAP_MEMORY_BARRIER_COMPILE; }); | ||
for (uint32_t real_idx = service->real_start; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMHO this makes code unnecessary complex. Maybe with more complex logic it will be fine, but here I feel like just doing
//...
if (start == do_not_exceed)
YANET_LOG_ERROR("Balancer service exceeded ring chunk bounds\n");
YADECAP_MEMORY_BARRIER_COMPILE
return start;
}
//...
YADECAP_MEMORY_BARRIER_COMPILE;
return start;
}
is sufficient enough
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to YADECAP_MEMORY_BARRIER_COMPILE
at each function exit path. Using it at each existing function exit doesn't convey this intent to future function changes.
How about using a DEFERED_MEMORY_BARRIER_COMPILE
macro?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, maybe your solution makes more sense that I've thought initially.
smth like this will suffice?
struct MemoryBarrierGuard {
~MemoryBarrierGuard() {
YADECAP_MEMORY_BARRIER_COMPILE;
}
};
And do that at the start of every method:
Result cControlPlane::init(bool use_kernel_interface)
{
MemoryBarrierGuard guard;
//...
a490f75
to
92d2254
Compare
LGTM apart from those three unresolved conversations: |
Integrates consistent hashing library into yanet.
Adds option to create "chash" type balancer services.