Skip to content
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

correctly de-dup DNS/SDS hosts based on address #452

Merged
merged 1 commit into from Feb 9, 2017
Merged

correctly de-dup DNS/SDS hosts based on address #452

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 11 additions & 3 deletions source/common/upstream/upstream_impl.cc
View file
Open in desktop
Expand Up @@ -282,13 +282,21 @@ bool BaseDynamicClusterImpl::updateDynamicHostList(const std::vector<HostPtr>& n

// Go through and see if the list we have is different from what we just got. If it is, we
// make a new host list and raise a change notification. This uses an N^2 search given that
// this does not happen very often and the list sizes should be small.
// this does not happen very often and the list sizes should be small. We also check for
// duplicates here. It's possible for DNS to return the same address multiple times, and a bad
// SDS implementation could do the same thing.
std::unordered_set<std::string> host_addresses;
std::vector<HostPtr> final_hosts;
for (HostPtr host : new_hosts) {
if (host_addresses.count(host->address()->asString())) {
continue;
}
host_addresses.emplace(host->address()->asString());

bool found = false;
for (auto i = current_hosts.begin(); i != current_hosts.end();) {
// If we find a host matched based on URL, we keep it. However we do change weight inline so
// do that here.
// If we find a host matched based on address, we keep it. However we do change weight inline
// so do that here.
if (*(*i)->address() == *host->address()) {
if (host->weight() > max_host_weight) {
max_host_weight = host->weight();
Expand Down
3 changes: 2 additions & 1 deletion test/common/upstream/upstream_impl_test.cc
View file
Open in desktop
Expand Up @@ -142,9 +142,10 @@ TEST(StrictDnsClusterImplTest, Basic) {
EXPECT_THAT(std::list<std::string>({"127.0.0.3:11001"}),
ContainerEq(hostListToAddresses(cluster.hosts())));

// Make sure we de-dup the same address.
EXPECT_CALL(*resolver2.timer_, enableTimer(std::chrono::milliseconds(4000)));
EXPECT_CALL(membership_updated, ready());
resolver2.dns_callback_(TestUtility::makeDnsResponse({"10.0.0.1"}));
resolver2.dns_callback_(TestUtility::makeDnsResponse({"10.0.0.1", "10.0.0.1"}));
EXPECT_THAT(std::list<std::string>({"127.0.0.3:11001", "10.0.0.1:11002"}),
ContainerEq(hostListToAddresses(cluster.hosts())));

Expand Down