-
Notifications
You must be signed in to change notification settings - Fork 9
Fix addresses_with_failover in cluster requests #750
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
Conversation
src/Interpreters/Cluster.cpp
Outdated
| std::shuffle(shards_info.begin(), shards_info.end(), gen); | ||
| shards_info.resize(max_hosts); | ||
|
|
||
| Addresses all_addresses_; |
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.
Can you put the entire logic that lives inside this if statement in a separate method? IMHO, it is now slightly more complex and it deserves a named method. Perhaps something like:
/*
Shuffle shards_info to avoid picking only the first `max_hosts` hosts. Adjust addresses accordingly.
*/
static void constrainShardInfoAndAddressesToMaxHosts(ShardInfo & shard_info, Addresses & addresses)
{
pcg64_fast gen{randomSeed()};
std::shuffle(shards_info.begin(), shards_info.end(), gen);
shards_info.resize(max_hosts);
// shuffled_addresses must have the same ordering as shards_info, that's why the re-ordering is done manually instead of making a call to `std::shuffle`
Addresses shuffled_addresses;
for (auto & shard_info : shards_info)
{
shuffled_addresses.emplace_back(addresses[shard_info.shard_num - 1]);
shard_info.shard_num = ++shard_num;
}
addresses.swap(shuffled_addresses);
}
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.
Move to separate method.
…lover Fix addresses_with_failover in cluster requests
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Fix filling addresses_with_failover with correct addresses
Documentation entry for user-facing changes
Bug made in #677
addresses_with_failoverfilled only with local addresses instead of all addresses.Should fix flapped stateless test
02804_clusterAllReplicas_insert.