From 4ffd11760995a070f662137de42968093373d5e3 Mon Sep 17 00:00:00 2001
From: Snow Pettersen <snowp@squareup.com>
Date: Sat, 30 Mar 2019 17:28:37 -0400
Subject: [PATCH 1/8] upstream: partition hosts in a single pass

This fixes a performance regression that was introduced when support for
degraded hosts was added: the list of hosts would be iterated over four
times instead of the previous two (one for the hosts list, one for the
hosts per locality list). This PR changes both partition operations to
only iterate over the list of hosts once.

Signed-off-by: Snow Pettersen <snowp@squareup.com>
---
 include/envoy/upstream/upstream.h       | 10 ++++
 source/common/upstream/upstream_impl.cc | 77 +++++++++++++++++--------
 source/common/upstream/upstream_impl.h  | 13 ++++-
 3 files changed, 73 insertions(+), 27 deletions(-)

diff --git a/include/envoy/upstream/upstream.h b/include/envoy/upstream/upstream.h
index e2c2300d6e5d6..cdf776b4818ac 100644
--- a/include/envoy/upstream/upstream.h
+++ b/include/envoy/upstream/upstream.h
@@ -229,6 +229,16 @@ class HostsPerLocality {
   virtual std::shared_ptr<const HostsPerLocality>
   filter(std::function<bool(const Host&)> predicate) const PURE;
 
+  /**
+   * Clone object with multiple filter predicates. Returns a vector of clones, each with host that
+   * match the provided predicates.
+   * @param predicates vector of predicates on Host entries.
+   * @return vector of HostsPerLocalityConstSharedPtr clones of the HostsPerLocality that match
+   *         hosts according to predicates.
+   */
+  virtual std::vector<std::shared_ptr<const HostsPerLocality>>
+  filter(const std::vector<std::function<bool(const Host&)>>& predicates) const PURE;
+
   /**
    * Clone object.
    * @return HostsPerLocalityConstSharedPtr clone of the HostsPerLocality.
diff --git a/source/common/upstream/upstream_impl.cc b/source/common/upstream/upstream_impl.cc
index ef436d8a084c7..1cc27a953ba1c 100644
--- a/source/common/upstream/upstream_impl.cc
+++ b/source/common/upstream/upstream_impl.cc
@@ -272,21 +272,42 @@ void HostImpl::weight(uint32_t new_weight) { weight_ = std::max(1U, std::min(128
 
 HostsPerLocalityConstSharedPtr
 HostsPerLocalityImpl::filter(std::function<bool(const Host&)> predicate) const {
-  auto* filtered_clone = new HostsPerLocalityImpl();
-  HostsPerLocalityConstSharedPtr shared_filtered_clone{filtered_clone};
+  return filter(std::vector<std::function<bool(const Host&)>>{predicate})[0];
+}
+
+std::vector<HostsPerLocalityConstSharedPtr> HostsPerLocalityImpl::filter(
+    const std::vector<std::function<bool(const Host&)>>& predicates) const {
+  // We keep two lists: one for being able to mutate the clone and one for returning to the caller.
+  // Creating them both at the start avoids iterating over the mutable values at the end to convert
+  // them to a const pointer.
+  std::vector<HostsPerLocalityImpl*> mutable_clones;
+  std::vector<HostsPerLocalityConstSharedPtr> filtered_clones;
+
+  for (size_t i = 0; i < predicates.size(); ++i) {
+    mutable_clones.emplace_back(new HostsPerLocalityImpl());
+    filtered_clones.emplace_back(mutable_clones.back());
+    mutable_clones.back()->local_ = local_;
+  }
 
-  filtered_clone->local_ = local_;
   for (const auto& hosts_locality : hosts_per_locality_) {
-    HostVector current_locality_hosts;
+    std::vector<HostVector> current_locality_hosts;
+    current_locality_hosts.resize(predicates.size());
+
+    // Since # of hosts >> # of predicates, we iterate over the hosts in the outer loop.
     for (const auto& host : hosts_locality) {
-      if (predicate(*host)) {
-        current_locality_hosts.emplace_back(host);
+      for (size_t i = 0; i < predicates.size(); ++i) {
+        if (predicates[i](*host)) {
+          current_locality_hosts[i].emplace_back(host);
+        }
       }
     }
-    filtered_clone->hosts_per_locality_.push_back(std::move(current_locality_hosts));
+
+    for (size_t i = 0; i < predicates.size(); ++i) {
+      mutable_clones[i]->hosts_per_locality_.push_back(std::move(current_locality_hosts[0]));
+    }
   }
 
-  return shared_filtered_clone;
+  return filtered_clones;
 }
 
 void HostSetImpl::updateHosts(PrioritySet::UpdateHostsParams&& update_hosts_params,
@@ -414,16 +435,15 @@ HostSetImpl::updateHostsParams(HostVectorConstSharedPtr hosts,
 PrioritySet::UpdateHostsParams
 HostSetImpl::partitionHosts(HostVectorConstSharedPtr hosts,
                             HostsPerLocalityConstSharedPtr hosts_per_locality) {
-  auto healthy_hosts = ClusterImplBase::createHostList(*hosts, Host::Health::Healthy);
-  auto degraded_hosts = ClusterImplBase::createHostList(*hosts, Host::Health::Degraded);
-  auto healthy_hosts_per_locality =
-      ClusterImplBase::createHostLists(*hosts_per_locality, Host::Health::Healthy);
-  auto degraded_hosts_per_locality =
-      ClusterImplBase::createHostLists(*hosts_per_locality, Host::Health::Degraded);
+  auto healthy_and_degraded_hosts = ClusterImplBase::partitionHostList(*hosts);
+  auto healthy_and_degraded_hosts_per_locality =
+      ClusterImplBase::partitionHostsPerLocality(*hosts_per_locality);
 
   return updateHostsParams(std::move(hosts), std::move(hosts_per_locality),
-                           std::move(healthy_hosts), std::move(healthy_hosts_per_locality),
-                           std::move(degraded_hosts), std::move(degraded_hosts_per_locality));
+                           std::move(healthy_and_degraded_hosts.first),
+                           std::move(healthy_and_degraded_hosts_per_locality.first),
+                           std::move(healthy_and_degraded_hosts.second),
+                           std::move(healthy_and_degraded_hosts_per_locality.second));
 }
 
 double HostSetImpl::effectiveLocalityWeight(uint32_t index,
@@ -673,21 +693,30 @@ ClusterImplBase::ClusterImplBase(
       });
 }
 
-HostVectorConstSharedPtr ClusterImplBase::createHostList(const HostVector& hosts,
-                                                         Host::Health health) {
-  HostVectorSharedPtr healthy_list(new HostVector());
+std::pair<HostVectorConstSharedPtr, HostVectorConstSharedPtr>
+ClusterImplBase::partitionHostList(const HostVector& hosts) {
+  auto healthy_list = std::make_shared<HostVector>();
+  auto degraded_list = std::make_shared<HostVector>();
+
   for (const auto& host : hosts) {
-    if (host->health() == health) {
+    if (host->health() == Host::Health::Healthy) {
       healthy_list->emplace_back(host);
     }
+    if (host->health() == Host::Health::Degraded) {
+      degraded_list->emplace_back(host);
+    }
   }
 
-  return healthy_list;
+  return {healthy_list, degraded_list};
 }
 
-HostsPerLocalityConstSharedPtr ClusterImplBase::createHostLists(const HostsPerLocality& hosts,
-                                                                Host::Health health) {
-  return hosts.filter([&health](const Host& host) { return host.health() == health; });
+std::pair<HostsPerLocalityConstSharedPtr, HostsPerLocalityConstSharedPtr>
+ClusterImplBase::partitionHostsPerLocality(const HostsPerLocality& hosts) {
+  auto filtered_clones =
+      hosts.filter({[](const Host& host) { return host.health() == Host::Health::Healthy; },
+                    [](const Host& host) { return host.health() == Host::Health::Degraded; }});
+
+  return {std::move(filtered_clones[0]), std::move(filtered_clones[1])};
 }
 
 bool ClusterInfoImpl::maintenanceMode() const {
diff --git a/source/common/upstream/upstream_impl.h b/source/common/upstream/upstream_impl.h
index 8a9096a2e89d6..8cd747ac82272 100644
--- a/source/common/upstream/upstream_impl.h
+++ b/source/common/upstream/upstream_impl.h
@@ -257,6 +257,8 @@ class HostsPerLocalityImpl : public HostsPerLocality {
   bool hasLocalLocality() const override { return local_; }
   const std::vector<HostVector>& get() const override { return hosts_per_locality_; }
   HostsPerLocalityConstSharedPtr filter(std::function<bool(const Host&)> predicate) const override;
+  std::vector<HostsPerLocalityConstSharedPtr>
+  filter(const std::vector<std::function<bool(const Host&)>>& predicate) const override;
 
   // The const shared pointer for the empty HostsPerLocalityImpl.
   static HostsPerLocalityConstSharedPtr empty() {
@@ -637,9 +639,14 @@ class ClusterImplBase : public Cluster, protected Logger::Loggable<Logger::Id::u
   const Network::Address::InstanceConstSharedPtr
   resolveProtoAddress(const envoy::api::v2::core::Address& address);
 
-  static HostVectorConstSharedPtr createHostList(const HostVector& hosts, Host::Health health);
-  static HostsPerLocalityConstSharedPtr createHostLists(const HostsPerLocality& hosts,
-                                                        Host::Health);
+  // Pratitions the provided list of hosts into two new lists containing the healthy and degraded
+  // hosts respectively.
+  static std::pair<HostVectorConstSharedPtr, HostVectorConstSharedPtr>
+  partitionHostList(const HostVector& hosts);
+  // Pratitions the provided list of hosts per locality into two new lists containing the healthy
+  // and degraded hosts respectively.
+  static std::pair<HostsPerLocalityConstSharedPtr, HostsPerLocalityConstSharedPtr>
+  partitionHostsPerLocality(const HostsPerLocality& hosts);
 
   // Upstream::Cluster
   HealthChecker* healthChecker() override { return health_checker_.get(); }

From cf32ab471f9f738607d0f8931d4f1646ee9cbca7 Mon Sep 17 00:00:00 2001
From: Snow Pettersen <snowp@squareup.com>
Date: Mon, 1 Apr 2019 10:46:15 -0400
Subject: [PATCH 2/8] spelling

Signed-off-by: Snow Pettersen <snowp@squareup.com>
---
 source/common/upstream/upstream_impl.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/source/common/upstream/upstream_impl.h b/source/common/upstream/upstream_impl.h
index 8cd747ac82272..7faf0874b936e 100644
--- a/source/common/upstream/upstream_impl.h
+++ b/source/common/upstream/upstream_impl.h
@@ -639,11 +639,11 @@ class ClusterImplBase : public Cluster, protected Logger::Loggable<Logger::Id::u
   const Network::Address::InstanceConstSharedPtr
   resolveProtoAddress(const envoy::api::v2::core::Address& address);
 
-  // Pratitions the provided list of hosts into two new lists containing the healthy and degraded
+  // Partitions the provided list of hosts into two new lists containing the healthy and degraded
   // hosts respectively.
   static std::pair<HostVectorConstSharedPtr, HostVectorConstSharedPtr>
   partitionHostList(const HostVector& hosts);
-  // Pratitions the provided list of hosts per locality into two new lists containing the healthy
+  // Partitions the provided list of hosts per locality into two new lists containing the healthy
   // and degraded hosts respectively.
   static std::pair<HostsPerLocalityConstSharedPtr, HostsPerLocalityConstSharedPtr>
   partitionHostsPerLocality(const HostsPerLocality& hosts);

From 9435169477caa84cb861447f905133a67d6d77a2 Mon Sep 17 00:00:00 2001
From: Snow Pettersen <snowp@squareup.com>
Date: Sat, 13 Apr 2019 11:58:30 -0400
Subject: [PATCH 3/8] use phantom types, remove single filter function

Signed-off-by: Snow Pettersen <snowp@squareup.com>
---
 include/envoy/upstream/BUILD               |  1 +
 include/envoy/upstream/types.h             |  5 ++++
 include/envoy/upstream/upstream.h          | 21 ++++++--------
 source/common/upstream/upstream_impl.cc    | 33 +++++++++-------------
 source/common/upstream/upstream_impl.h     | 21 +++++++-------
 test/common/upstream/upstream_impl_test.cc | 33 +++++++++++-----------
 6 files changed, 56 insertions(+), 58 deletions(-)

diff --git a/include/envoy/upstream/BUILD b/include/envoy/upstream/BUILD
index 31edcd0dff059..2e0a177ca0a51 100644
--- a/include/envoy/upstream/BUILD
+++ b/include/envoy/upstream/BUILD
@@ -138,6 +138,7 @@ envoy_cc_library(
         ":locality_lib",
         ":resource_manager_interface",
         "//include/envoy/common:callback",
+        "//include/envoy/upstream:types_interface",
         "//include/envoy/config:typed_metadata_interface",
         "//include/envoy/http:codec_interface",
         "//include/envoy/network:connection_interface",
diff --git a/include/envoy/upstream/types.h b/include/envoy/upstream/types.h
index 59bd3de65ffa8..b73e66b220ea5 100644
--- a/include/envoy/upstream/types.h
+++ b/include/envoy/upstream/types.h
@@ -51,5 +51,10 @@ struct HealthyAvailability : PriorityAvailability {
   using PriorityAvailability::PriorityAvailability;
 };
 
+// Phantom type indicating that the type is related to healthy hosts.
+struct Healthy {};
+// Phantom type indicating that the type is related to degraded hosts.
+struct Degraded {};
+
 } // namespace Upstream
 } // namespace Envoy
diff --git a/include/envoy/upstream/upstream.h b/include/envoy/upstream/upstream.h
index cdf776b4818ac..364e08a876e75 100644
--- a/include/envoy/upstream/upstream.h
+++ b/include/envoy/upstream/upstream.h
@@ -20,6 +20,7 @@
 #include "envoy/upstream/health_check_host_monitor.h"
 #include "envoy/upstream/load_balancer_type.h"
 #include "envoy/upstream/locality.h"
+#include "envoy/upstream/types.h"
 #include "envoy/upstream/outlier_detection.h"
 #include "envoy/upstream/resource_manager.h"
 
@@ -191,10 +192,15 @@ class Host : virtual public HostDescription {
 typedef std::shared_ptr<const Host> HostConstSharedPtr;
 
 typedef std::vector<HostSharedPtr> HostVector;
+typedef Phantom<HostVector, Healthy> HealthyHostVector;
+typedef Phantom<HostVector, Degraded> DegradedHostVector;
 typedef std::unordered_map<std::string, Upstream::HostSharedPtr> HostMap;
 typedef std::shared_ptr<HostVector> HostVectorSharedPtr;
 typedef std::shared_ptr<const HostVector> HostVectorConstSharedPtr;
 
+typedef std::shared_ptr<const HealthyHostVector> HealthyHostVectorConstSharedPtr;
+typedef std::shared_ptr<const DegradedHostVector> DegradedHostVectorConstSharedPtr;
+
 typedef std::unique_ptr<HostVector> HostListPtr;
 typedef std::unordered_map<envoy::api::v2::core::Locality, uint32_t, LocalityHash, LocalityEqualTo>
     LocalityWeightsMap;
@@ -220,15 +226,6 @@ class HostsPerLocality {
    */
   virtual const std::vector<HostVector>& get() const PURE;
 
-  /**
-   * Clone object with a filter predicate.
-   * @param predicate on Host entries.
-   * @return HostsPerLocalityConstSharedPtr clone of the HostsPerLocality with only
-   *         hosts according to predicate.
-   */
-  virtual std::shared_ptr<const HostsPerLocality>
-  filter(std::function<bool(const Host&)> predicate) const PURE;
-
   /**
    * Clone object with multiple filter predicates. Returns a vector of clones, each with host that
    * match the provided predicates.
@@ -244,7 +241,7 @@ class HostsPerLocality {
    * @return HostsPerLocalityConstSharedPtr clone of the HostsPerLocality.
    */
   std::shared_ptr<const HostsPerLocality> clone() const {
-    return filter([](const Host&) { return true; });
+    return filter({[](const Host&) { return true; }})[0];
   }
 };
 
@@ -376,8 +373,8 @@ class PrioritySet {
    */
   struct UpdateHostsParams {
     HostVectorConstSharedPtr hosts;
-    HostVectorConstSharedPtr healthy_hosts;
-    HostVectorConstSharedPtr degraded_hosts;
+    HealthyHostVectorConstSharedPtr healthy_hosts;
+    DegradedHostVectorConstSharedPtr degraded_hosts;
     HostsPerLocalityConstSharedPtr hosts_per_locality;
     HostsPerLocalityConstSharedPtr healthy_hosts_per_locality;
     HostsPerLocalityConstSharedPtr degraded_hosts_per_locality;
diff --git a/source/common/upstream/upstream_impl.cc b/source/common/upstream/upstream_impl.cc
index 1cc27a953ba1c..74ff558c1f172 100644
--- a/source/common/upstream/upstream_impl.cc
+++ b/source/common/upstream/upstream_impl.cc
@@ -270,21 +270,16 @@ HostImpl::createConnection(Event::Dispatcher& dispatcher, const ClusterInfo& clu
 
 void HostImpl::weight(uint32_t new_weight) { weight_ = std::max(1U, std::min(128U, new_weight)); }
 
-HostsPerLocalityConstSharedPtr
-HostsPerLocalityImpl::filter(std::function<bool(const Host&)> predicate) const {
-  return filter(std::vector<std::function<bool(const Host&)>>{predicate})[0];
-}
-
 std::vector<HostsPerLocalityConstSharedPtr> HostsPerLocalityImpl::filter(
     const std::vector<std::function<bool(const Host&)>>& predicates) const {
   // We keep two lists: one for being able to mutate the clone and one for returning to the caller.
   // Creating them both at the start avoids iterating over the mutable values at the end to convert
   // them to a const pointer.
-  std::vector<HostsPerLocalityImpl*> mutable_clones;
+  std::vector<std::shared_ptr<HostsPerLocalityImpl>> mutable_clones;
   std::vector<HostsPerLocalityConstSharedPtr> filtered_clones;
 
   for (size_t i = 0; i < predicates.size(); ++i) {
-    mutable_clones.emplace_back(new HostsPerLocalityImpl());
+    mutable_clones.emplace_back(std::make_shared<HostsPerLocalityImpl>());
     filtered_clones.emplace_back(mutable_clones.back());
     mutable_clones.back()->local_ = local_;
   }
@@ -327,10 +322,10 @@ void HostSetImpl::updateHosts(PrioritySet::UpdateHostsParams&& update_hosts_para
   locality_weights_ = std::move(locality_weights);
 
   rebuildLocalityScheduler(healthy_locality_scheduler_, healthy_locality_entries_,
-                           *healthy_hosts_per_locality_, *healthy_hosts_, hosts_per_locality_,
+                           *healthy_hosts_per_locality_, healthy_hosts_->get(), hosts_per_locality_,
                            locality_weights_, overprovisioning_factor_);
   rebuildLocalityScheduler(degraded_locality_scheduler_, degraded_locality_entries_,
-                           *degraded_hosts_per_locality_, *degraded_hosts_, hosts_per_locality_,
+                           *degraded_hosts_per_locality_, degraded_hosts_->get(), hosts_per_locality_,
                            locality_weights_, overprovisioning_factor_);
 
   runUpdateCallbacks(hosts_added, hosts_removed);
@@ -404,25 +399,25 @@ PrioritySet::UpdateHostsParams
 HostSetImpl::updateHostsParams(HostVectorConstSharedPtr hosts,
                                HostsPerLocalityConstSharedPtr hosts_per_locality) {
   return updateHostsParams(std::move(hosts), std::move(hosts_per_locality),
-                           std::make_shared<const HostVector>(), HostsPerLocalityImpl::empty());
+                           std::make_shared<const HealthyHostVector>(), HostsPerLocalityImpl::empty());
 }
 
 PrioritySet::UpdateHostsParams
 HostSetImpl::updateHostsParams(HostVectorConstSharedPtr hosts,
                                HostsPerLocalityConstSharedPtr hosts_per_locality,
-                               HostVectorConstSharedPtr healthy_hosts,
+                               HealthyHostVectorConstSharedPtr healthy_hosts,
                                HostsPerLocalityConstSharedPtr healthy_hosts_per_locality) {
   return updateHostsParams(std::move(hosts), std::move(hosts_per_locality),
                            std::move(healthy_hosts), std::move(healthy_hosts_per_locality),
-                           std::make_shared<const HostVector>(), HostsPerLocalityImpl::empty());
+                           std::make_shared<const DegradedHostVector>(), HostsPerLocalityImpl::empty());
 }
 
 PrioritySet::UpdateHostsParams
 HostSetImpl::updateHostsParams(HostVectorConstSharedPtr hosts,
                                HostsPerLocalityConstSharedPtr hosts_per_locality,
-                               HostVectorConstSharedPtr healthy_hosts,
+                               HealthyHostVectorConstSharedPtr healthy_hosts,
                                HostsPerLocalityConstSharedPtr healthy_hosts_per_locality,
-                               HostVectorConstSharedPtr degraded_hosts,
+                               DegradedHostVectorConstSharedPtr degraded_hosts,
                                HostsPerLocalityConstSharedPtr degraded_hosts_per_locality) {
   return PrioritySet::UpdateHostsParams{std::move(hosts),
                                         std::move(healthy_hosts),
@@ -693,17 +688,17 @@ ClusterImplBase::ClusterImplBase(
       });
 }
 
-std::pair<HostVectorConstSharedPtr, HostVectorConstSharedPtr>
+std::pair<HealthyHostVectorConstSharedPtr, DegradedHostVectorConstSharedPtr>
 ClusterImplBase::partitionHostList(const HostVector& hosts) {
-  auto healthy_list = std::make_shared<HostVector>();
-  auto degraded_list = std::make_shared<HostVector>();
+  auto healthy_list = std::make_shared<HealthyHostVector>();
+  auto degraded_list = std::make_shared<DegradedHostVector>();
 
   for (const auto& host : hosts) {
     if (host->health() == Host::Health::Healthy) {
-      healthy_list->emplace_back(host);
+      healthy_list->get().emplace_back(host);
     }
     if (host->health() == Host::Health::Degraded) {
-      degraded_list->emplace_back(host);
+      degraded_list->get().emplace_back(host);
     }
   }
 
diff --git a/source/common/upstream/upstream_impl.h b/source/common/upstream/upstream_impl.h
index 7faf0874b936e..fcbbb90aa07fb 100644
--- a/source/common/upstream/upstream_impl.h
+++ b/source/common/upstream/upstream_impl.h
@@ -256,7 +256,6 @@ class HostsPerLocalityImpl : public HostsPerLocality {
 
   bool hasLocalLocality() const override { return local_; }
   const std::vector<HostVector>& get() const override { return hosts_per_locality_; }
-  HostsPerLocalityConstSharedPtr filter(std::function<bool(const Host&)> predicate) const override;
   std::vector<HostsPerLocalityConstSharedPtr>
   filter(const std::vector<std::function<bool(const Host&)>>& predicate) const override;
 
@@ -282,8 +281,8 @@ class HostSetImpl : public HostSet {
       : priority_(priority), overprovisioning_factor_(overprovisioning_factor.has_value()
                                                           ? overprovisioning_factor.value()
                                                           : kDefaultOverProvisioningFactor),
-        hosts_(new HostVector()), healthy_hosts_(new HostVector()),
-        degraded_hosts_(new HostVector()) {}
+        hosts_(new HostVector()), healthy_hosts_(new HealthyHostVector()),
+        degraded_hosts_(new DegradedHostVector()) {}
 
   /**
    * Install a callback that will be invoked when the host set membership changes.
@@ -296,8 +295,8 @@ class HostSetImpl : public HostSet {
 
   // Upstream::HostSet
   const HostVector& hosts() const override { return *hosts_; }
-  const HostVector& healthyHosts() const override { return *healthy_hosts_; }
-  const HostVector& degradedHosts() const override { return *degraded_hosts_; }
+  const HostVector& healthyHosts() const override { return healthy_hosts_->get(); }
+  const HostVector& degradedHosts() const override { return degraded_hosts_->get(); }
   const HostsPerLocality& hostsPerLocality() const override { return *hosts_per_locality_; }
   const HostsPerLocality& healthyHostsPerLocality() const override {
     return *healthy_hosts_per_locality_;
@@ -318,14 +317,14 @@ class HostSetImpl : public HostSet {
   static PrioritySet::UpdateHostsParams
   updateHostsParams(HostVectorConstSharedPtr hosts,
                     HostsPerLocalityConstSharedPtr hosts_per_locality,
-                    HostVectorConstSharedPtr healthy_hosts,
+                    HealthyHostVectorConstSharedPtr healthy_hosts,
                     HostsPerLocalityConstSharedPtr healthy_hosts_per_locality);
   static PrioritySet::UpdateHostsParams
   updateHostsParams(HostVectorConstSharedPtr hosts,
                     HostsPerLocalityConstSharedPtr hosts_per_locality,
-                    HostVectorConstSharedPtr healthy_hosts,
+                    HealthyHostVectorConstSharedPtr healthy_hosts,
                     HostsPerLocalityConstSharedPtr healthy_hosts_per_locality,
-                    HostVectorConstSharedPtr degraded_hosts,
+                    DegradedHostVectorConstSharedPtr degraded_hosts,
                     HostsPerLocalityConstSharedPtr degraded_hosts_per_locality);
   static PrioritySet::UpdateHostsParams
   partitionHosts(HostVectorConstSharedPtr hosts, HostsPerLocalityConstSharedPtr hosts_per_locality);
@@ -352,8 +351,8 @@ class HostSetImpl : public HostSet {
   uint32_t priority_;
   uint32_t overprovisioning_factor_;
   HostVectorConstSharedPtr hosts_;
-  HostVectorConstSharedPtr healthy_hosts_;
-  HostVectorConstSharedPtr degraded_hosts_;
+  HealthyHostVectorConstSharedPtr healthy_hosts_;
+  DegradedHostVectorConstSharedPtr degraded_hosts_;
   HostsPerLocalityConstSharedPtr hosts_per_locality_{HostsPerLocalityImpl::empty()};
   HostsPerLocalityConstSharedPtr healthy_hosts_per_locality_{HostsPerLocalityImpl::empty()};
   HostsPerLocalityConstSharedPtr degraded_hosts_per_locality_{HostsPerLocalityImpl::empty()};
@@ -641,7 +640,7 @@ class ClusterImplBase : public Cluster, protected Logger::Loggable<Logger::Id::u
 
   // Partitions the provided list of hosts into two new lists containing the healthy and degraded
   // hosts respectively.
-  static std::pair<HostVectorConstSharedPtr, HostVectorConstSharedPtr>
+  static std::pair<HealthyHostVectorConstSharedPtr, DegradedHostVectorConstSharedPtr>
   partitionHostList(const HostVector& hosts);
   // Partitions the provided list of hosts per locality into two new lists containing the healthy
   // and degraded hosts respectively.
diff --git a/test/common/upstream/upstream_impl_test.cc b/test/common/upstream/upstream_impl_test.cc
index 8042842729993..053ee73e33910 100644
--- a/test/common/upstream/upstream_impl_test.cc
+++ b/test/common/upstream/upstream_impl_test.cc
@@ -69,7 +69,8 @@ std::list<std::string> hostListToAddresses(const HostVector& hosts) {
   return addresses;
 }
 
-std::shared_ptr<const HostVector>
+template<class HostsT=HostVector>
+std::shared_ptr<const HostsT>
 makeHostsFromHostsPerLocality(HostsPerLocalityConstSharedPtr hosts_per_locality) {
   HostVector hosts;
 
@@ -79,7 +80,7 @@ makeHostsFromHostsPerLocality(HostsPerLocalityConstSharedPtr hosts_per_locality)
     }
   }
 
-  return std::make_shared<const HostVector>(hosts);
+  return std::make_shared<const HostsT>(hosts);
 }
 
 struct ResolverData {
@@ -1589,7 +1590,7 @@ class TestBatchUpdateCb : public PrioritySet::BatchUpdateCb {
       HostVector hosts_removed{};
       host_update_cb.updateHosts(
           0,
-          HostSetImpl::updateHostsParams(hosts_, hosts_per_locality_, hosts_, hosts_per_locality_),
+          HostSetImpl::updateHostsParams(hosts_, hosts_per_locality_, std::make_shared<const HealthyHostVector>(*hosts_), hosts_per_locality_),
           {}, hosts_added, hosts_removed, absl::nullopt);
     }
 
@@ -1600,7 +1601,7 @@ class TestBatchUpdateCb : public PrioritySet::BatchUpdateCb {
       HostVector hosts_removed{hosts_->front()};
       host_update_cb.updateHosts(
           1,
-          HostSetImpl::updateHostsParams(empty_hosts, HostsPerLocalityImpl::empty(), empty_hosts,
+          HostSetImpl::updateHostsParams(empty_hosts, HostsPerLocalityImpl::empty(), std::make_shared<const HealthyHostVector>(*empty_hosts),
                                          HostsPerLocalityImpl::empty()),
           {}, hosts_added, hosts_removed, absl::nullopt);
     }
@@ -1653,7 +1654,7 @@ TEST(PrioritySet, Extend) {
     HostVector hosts_removed{};
 
     priority_set.updateHosts(
-        1, HostSetImpl::updateHostsParams(hosts, hosts_per_locality, hosts, hosts_per_locality), {},
+        1, HostSetImpl::updateHostsParams(hosts, hosts_per_locality, std::make_shared<const HealthyHostVector>(*hosts), hosts_per_locality), {},
         hosts_added, hosts_removed, absl::nullopt);
   }
   EXPECT_EQ(1, priority_changes);
@@ -2182,9 +2183,9 @@ TEST(HostsPerLocalityImpl, Filter) {
   {
     std::vector<HostVector> locality_hosts = {{host_0}, {host_1}};
     const auto filtered =
-        HostsPerLocalityImpl(std::move(locality_hosts), false).filter([&host_0](const Host& host) {
+        HostsPerLocalityImpl(std::move(locality_hosts), false).filter({[&host_0](const Host& host) {
           return &host == host_0.get();
-        });
+        }})[0];
     EXPECT_FALSE(filtered->hasLocalLocality());
     const std::vector<HostVector> expected_locality_hosts = {{host_0}, {}};
     EXPECT_EQ(expected_locality_hosts, filtered->get());
@@ -2193,9 +2194,9 @@ TEST(HostsPerLocalityImpl, Filter) {
   {
     std::vector<HostVector> locality_hosts = {{host_0}, {host_1}};
     auto filtered =
-        HostsPerLocalityImpl(std::move(locality_hosts), true).filter([&host_1](const Host& host) {
+        HostsPerLocalityImpl(std::move(locality_hosts), true).filter({[&host_1](const Host& host) {
           return &host == host_1.get();
-        });
+        }})[0];
     EXPECT_TRUE(filtered->hasLocalLocality());
     const std::vector<HostVector> expected_locality_hosts = {{}, {host_1}};
     EXPECT_EQ(expected_locality_hosts, filtered->get());
@@ -2237,7 +2238,7 @@ TEST_F(HostSetImplLocalityTest, EmptyLocality) {
   LocalityWeightsConstSharedPtr locality_weights{new LocalityWeights{1, 1}};
   auto hosts = makeHostsFromHostsPerLocality(hosts_per_locality);
   host_set_.updateHosts(
-      HostSetImpl::updateHostsParams(hosts, hosts_per_locality, hosts, hosts_per_locality),
+      HostSetImpl::updateHostsParams(hosts, hosts_per_locality, std::make_shared<const HealthyHostVector>(*hosts), hosts_per_locality),
       locality_weights, {}, {}, absl::nullopt);
   // Verify that we are not RRing between localities.
   EXPECT_EQ(0, host_set_.chooseHealthyLocality().value());
@@ -2250,7 +2251,7 @@ TEST_F(HostSetImplLocalityTest, AllZeroWeights) {
   LocalityWeightsConstSharedPtr locality_weights{new LocalityWeights{0, 0}};
   auto hosts = makeHostsFromHostsPerLocality(hosts_per_locality);
   host_set_.updateHosts(
-      HostSetImpl::updateHostsParams(hosts, hosts_per_locality, hosts, hosts_per_locality),
+      HostSetImpl::updateHostsParams(hosts, hosts_per_locality, std::make_shared<const HealthyHostVector>(*hosts), hosts_per_locality),
       locality_weights, {}, {});
   EXPECT_FALSE(host_set_.chooseHealthyLocality().has_value());
 }
@@ -2262,7 +2263,7 @@ TEST_F(HostSetImplLocalityTest, Unweighted) {
   LocalityWeightsConstSharedPtr locality_weights{new LocalityWeights{1, 1, 1}};
   auto hosts = makeHostsFromHostsPerLocality(hosts_per_locality);
   host_set_.updateHosts(
-      HostSetImpl::updateHostsParams(hosts, hosts_per_locality, hosts, hosts_per_locality),
+      HostSetImpl::updateHostsParams(hosts, hosts_per_locality, std::make_shared<const HealthyHostVector>(*hosts), hosts_per_locality),
       locality_weights, {}, {}, absl::nullopt);
   EXPECT_EQ(0, host_set_.chooseHealthyLocality().value());
   EXPECT_EQ(1, host_set_.chooseHealthyLocality().value());
@@ -2278,7 +2279,7 @@ TEST_F(HostSetImplLocalityTest, Weighted) {
   LocalityWeightsConstSharedPtr locality_weights{new LocalityWeights{1, 2}};
   auto hosts = makeHostsFromHostsPerLocality(hosts_per_locality);
   host_set_.updateHosts(
-      HostSetImpl::updateHostsParams(hosts, hosts_per_locality, hosts, hosts_per_locality),
+      HostSetImpl::updateHostsParams(hosts, hosts_per_locality, std::make_shared<const HealthyHostVector>(*hosts), hosts_per_locality),
       locality_weights, {}, {}, absl::nullopt);
   EXPECT_EQ(1, host_set_.chooseHealthyLocality().value());
   EXPECT_EQ(0, host_set_.chooseHealthyLocality().value());
@@ -2295,7 +2296,7 @@ TEST_F(HostSetImplLocalityTest, MissingWeight) {
   LocalityWeightsConstSharedPtr locality_weights{new LocalityWeights{1, 0, 1}};
   auto hosts = makeHostsFromHostsPerLocality(hosts_per_locality);
   host_set_.updateHosts(
-      HostSetImpl::updateHostsParams(hosts, hosts_per_locality, hosts, hosts_per_locality),
+      HostSetImpl::updateHostsParams(hosts, hosts_per_locality, std::make_shared<const HealthyHostVector>(*hosts), hosts_per_locality),
       locality_weights, {}, {}, absl::nullopt);
   EXPECT_EQ(0, host_set_.chooseHealthyLocality().value());
   EXPECT_EQ(2, host_set_.chooseHealthyLocality().value());
@@ -2321,7 +2322,7 @@ TEST_F(HostSetImplLocalityTest, UnhealthyFailover) {
     auto hosts = makeHostsFromHostsPerLocality(hosts_per_locality);
     host_set_.updateHosts(
         HostSetImpl::updateHostsParams(hosts, hosts_per_locality,
-                                       makeHostsFromHostsPerLocality(healthy_hosts_per_locality),
+                                       makeHostsFromHostsPerLocality<HealthyHostVector>(healthy_hosts_per_locality),
                                        healthy_hosts_per_locality),
         locality_weights, {}, {}, absl::nullopt);
   };
@@ -2366,7 +2367,7 @@ TEST(OverProvisioningFactorTest, LocalityPickChanges) {
     // Healthy ratio: (1/2, 1).
     HostsPerLocalitySharedPtr healthy_hosts_per_locality =
         makeHostsPerLocality({{hosts[0]}, {hosts[2]}});
-    auto healthy_hosts = makeHostsFromHostsPerLocality(healthy_hosts_per_locality);
+    auto healthy_hosts = makeHostsFromHostsPerLocality<HealthyHostVector>(healthy_hosts_per_locality);
     host_set.updateHosts(HostSetImpl::updateHostsParams(std::make_shared<const HostVector>(hosts),
                                                         hosts_per_locality, healthy_hosts,
                                                         healthy_hosts_per_locality),

From 9032ed705d4a1979e5069d3aa198e8d1b9626781 Mon Sep 17 00:00:00 2001
From: Snow Pettersen <snowp@squareup.com>
Date: Sat, 13 Apr 2019 12:55:55 -0400
Subject: [PATCH 4/8] more phantom types fixes

Signed-off-by: Snow Pettersen <snowp@squareup.com>
---
 .../common/upstream/cluster_manager_impl.cc   |  4 +--
 source/common/upstream/subset_lb.cc           | 18 ++++++------
 .../upstream/cluster_manager_impl_test.cc     | 28 +++++++++----------
 3 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/source/common/upstream/cluster_manager_impl.cc b/source/common/upstream/cluster_manager_impl.cc
index 8880e01ff547f..332e88ed588ce 100644
--- a/source/common/upstream/cluster_manager_impl.cc
+++ b/source/common/upstream/cluster_manager_impl.cc
@@ -678,8 +678,8 @@ void ClusterManagerImpl::postThreadLocalClusterUpdate(const Cluster& cluster, ui
 
   // TODO(htuch): Can we skip these copies by exporting out const shared_ptr from HostSet?
   HostVectorConstSharedPtr hosts_copy(new HostVector(host_set->hosts()));
-  HostVectorConstSharedPtr healthy_hosts_copy(new HostVector(host_set->healthyHosts()));
-  HostVectorConstSharedPtr degraded_hosts_copy(new HostVector(host_set->degradedHosts()));
+  HealthyHostVectorConstSharedPtr healthy_hosts_copy(new HealthyHostVector(host_set->healthyHosts()));
+  DegradedHostVectorConstSharedPtr degraded_hosts_copy(new DegradedHostVector(host_set->degradedHosts()));
   HostsPerLocalityConstSharedPtr hosts_per_locality_copy = host_set->hostsPerLocality().clone();
   HostsPerLocalityConstSharedPtr healthy_hosts_per_locality_copy =
       host_set->healthyHostsPerLocality().clone();
diff --git a/source/common/upstream/subset_lb.cc b/source/common/upstream/subset_lb.cc
index 4c2fdc44ab9b4..0e47d3955410f 100644
--- a/source/common/upstream/subset_lb.cc
+++ b/source/common/upstream/subset_lb.cc
@@ -535,19 +535,19 @@ void SubsetLoadBalancer::HostSubsetImpl::update(const HostVector& hosts_added,
     }
   }
 
-  auto healthy_hosts = std::make_shared<HostVector>();
-  healthy_hosts->reserve(original_host_set_.healthyHosts().size());
+  auto healthy_hosts = std::make_shared<HealthyHostVector>();
+  healthy_hosts->get().reserve(original_host_set_.healthyHosts().size());
   for (const auto& host : original_host_set_.healthyHosts()) {
     if (cached_predicate(*host)) {
-      healthy_hosts->emplace_back(host);
+      healthy_hosts->get().emplace_back(host);
     }
   }
 
-  auto degraded_hosts = std::make_shared<HostVector>();
-  degraded_hosts->reserve(original_host_set_.degradedHosts().size());
+  auto degraded_hosts = std::make_shared<DegradedHostVector>();
+  degraded_hosts->get().reserve(original_host_set_.degradedHosts().size());
   for (const auto& host : original_host_set_.degradedHosts()) {
     if (cached_predicate(*host)) {
-      degraded_hosts->emplace_back(host);
+      degraded_hosts->get().emplace_back(host);
     }
   }
 
@@ -561,13 +561,13 @@ void SubsetLoadBalancer::HostSubsetImpl::update(const HostVector& hosts_added,
     hosts_per_locality = std::make_shared<HostsPerLocalityImpl>(
         *hosts, original_host_set_.hostsPerLocality().hasLocalLocality());
   } else {
-    hosts_per_locality = original_host_set_.hostsPerLocality().filter(cached_predicate);
+    hosts_per_locality = original_host_set_.hostsPerLocality().filter({cached_predicate})[0];
   }
 
   HostsPerLocalityConstSharedPtr healthy_hosts_per_locality =
-      original_host_set_.healthyHostsPerLocality().filter(cached_predicate);
+      original_host_set_.healthyHostsPerLocality().filter({cached_predicate})[0];
   HostsPerLocalityConstSharedPtr degraded_hosts_per_locality =
-      original_host_set_.degradedHostsPerLocality().filter(cached_predicate);
+      original_host_set_.degradedHostsPerLocality().filter({cached_predicate})[0];
 
   // We can use the cached predicate here, since we trust that the hosts in hosts_added were also
   // present in the list of all hosts.
diff --git a/test/common/upstream/cluster_manager_impl_test.cc b/test/common/upstream/cluster_manager_impl_test.cc
index a15b364c8b2d6..09f5fe068cfaf 100644
--- a/test/common/upstream/cluster_manager_impl_test.cc
+++ b/test/common/upstream/cluster_manager_impl_test.cc
@@ -2147,7 +2147,7 @@ TEST_F(ClusterManagerImplTest, MergedUpdates) {
   // The first update should be applied immediately, since it's not mergeable.
   hosts_removed.push_back((*hosts)[0]);
   cluster.prioritySet().updateHosts(
-      0, HostSetImpl::updateHostsParams(hosts, hosts_per_locality, hosts, hosts_per_locality), {},
+      0, HostSetImpl::updateHostsParams(hosts, hosts_per_locality, std::make_shared<const HealthyHostVector>(*hosts), hosts_per_locality), {},
       hosts_added, hosts_removed, absl::nullopt);
   EXPECT_EQ(1, factory_.stats_.counter("cluster_manager.cluster_updated").value());
   EXPECT_EQ(0, factory_.stats_.counter("cluster_manager.cluster_updated_via_merge").value());
@@ -2156,10 +2156,10 @@ TEST_F(ClusterManagerImplTest, MergedUpdates) {
   // These calls should be merged, since there are no added/removed hosts.
   hosts_removed.clear();
   cluster.prioritySet().updateHosts(
-      0, HostSetImpl::updateHostsParams(hosts, hosts_per_locality, hosts, hosts_per_locality), {},
+      0, HostSetImpl::updateHostsParams(hosts, hosts_per_locality, std::make_shared<const HealthyHostVector>(*hosts), hosts_per_locality), {},
       hosts_added, hosts_removed, absl::nullopt);
   cluster.prioritySet().updateHosts(
-      0, HostSetImpl::updateHostsParams(hosts, hosts_per_locality, hosts, hosts_per_locality), {},
+      0, HostSetImpl::updateHostsParams(hosts, hosts_per_locality, std::make_shared<const HealthyHostVector>(*hosts), hosts_per_locality), {},
       hosts_added, hosts_removed, absl::nullopt);
   EXPECT_EQ(1, factory_.stats_.counter("cluster_manager.cluster_updated").value());
   EXPECT_EQ(0, factory_.stats_.counter("cluster_manager.cluster_updated_via_merge").value());
@@ -2175,7 +2175,7 @@ TEST_F(ClusterManagerImplTest, MergedUpdates) {
   hosts_removed.clear();
   hosts_added.push_back((*hosts)[0]);
   cluster.prioritySet().updateHosts(
-      0, HostSetImpl::updateHostsParams(hosts, hosts_per_locality, hosts, hosts_per_locality), {},
+      0, HostSetImpl::updateHostsParams(hosts, hosts_per_locality, std::make_shared<const HealthyHostVector>(*hosts), hosts_per_locality), {},
       hosts_added, hosts_removed, absl::nullopt);
   EXPECT_EQ(2, factory_.stats_.counter("cluster_manager.cluster_updated").value());
   EXPECT_EQ(1, factory_.stats_.counter("cluster_manager.cluster_updated_via_merge").value());
@@ -2186,17 +2186,17 @@ TEST_F(ClusterManagerImplTest, MergedUpdates) {
 
   (*hosts)[0]->metadata(buildMetadata("v1"));
   cluster.prioritySet().updateHosts(
-      0, HostSetImpl::updateHostsParams(hosts, hosts_per_locality, hosts, hosts_per_locality), {},
+      0, HostSetImpl::updateHostsParams(hosts, hosts_per_locality, std::make_shared<const HealthyHostVector>(*hosts), hosts_per_locality), {},
       hosts_added, hosts_removed, absl::nullopt);
 
   (*hosts)[0]->healthFlagSet(Host::HealthFlag::FAILED_EDS_HEALTH);
   cluster.prioritySet().updateHosts(
-      0, HostSetImpl::updateHostsParams(hosts, hosts_per_locality, hosts, hosts_per_locality), {},
+      0, HostSetImpl::updateHostsParams(hosts, hosts_per_locality, std::make_shared<const HealthyHostVector>(*hosts), hosts_per_locality), {},
       hosts_added, hosts_removed, absl::nullopt);
 
   (*hosts)[0]->weight(100);
   cluster.prioritySet().updateHosts(
-      0, HostSetImpl::updateHostsParams(hosts, hosts_per_locality, hosts, hosts_per_locality), {},
+      0, HostSetImpl::updateHostsParams(hosts, hosts_per_locality, std::make_shared<const HealthyHostVector>(*hosts), hosts_per_locality), {},
       hosts_added, hosts_removed, absl::nullopt);
 
   // Updates not delivered yet.
@@ -2207,7 +2207,7 @@ TEST_F(ClusterManagerImplTest, MergedUpdates) {
   // Remove the host again, should cancel the scheduled update and be delivered immediately.
   hosts_removed.push_back((*hosts)[0]);
   cluster.prioritySet().updateHosts(
-      0, HostSetImpl::updateHostsParams(hosts, hosts_per_locality, hosts, hosts_per_locality), {},
+      0, HostSetImpl::updateHostsParams(hosts, hosts_per_locality, std::make_shared<const HealthyHostVector>(*hosts), hosts_per_locality), {},
       hosts_added, hosts_removed, absl::nullopt);
 
   EXPECT_EQ(3, factory_.stats_.counter("cluster_manager.cluster_updated").value());
@@ -2241,7 +2241,7 @@ TEST_F(ClusterManagerImplTest, MergedUpdatesOutOfWindow) {
   // cluster.info()->lbConfig().update_merge_window() in ClusterManagerImpl::scheduleUpdate.
   time_system_.sleep(std::chrono::seconds(60));
   cluster.prioritySet().updateHosts(
-      0, HostSetImpl::updateHostsParams(hosts, hosts_per_locality, hosts, hosts_per_locality), {},
+      0, HostSetImpl::updateHostsParams(hosts, hosts_per_locality, std::make_shared<const HealthyHostVector>(*hosts), hosts_per_locality), {},
       hosts_added, hosts_removed, absl::nullopt);
   EXPECT_EQ(1, factory_.stats_.counter("cluster_manager.cluster_updated").value());
   EXPECT_EQ(0, factory_.stats_.counter("cluster_manager.cluster_updated_via_merge").value());
@@ -2266,7 +2266,7 @@ TEST_F(ClusterManagerImplTest, MergedUpdatesInsideWindow) {
   // default-initialized to a monotonic time of 0, as is SimulatedTimeSystem::monotonic_time_.
   time_system_.sleep(std::chrono::seconds(2));
   cluster.prioritySet().updateHosts(
-      0, HostSetImpl::updateHostsParams(hosts, hosts_per_locality, hosts, hosts_per_locality), {},
+      0, HostSetImpl::updateHostsParams(hosts, hosts_per_locality, std::make_shared<const HealthyHostVector>(*hosts), hosts_per_locality), {},
       hosts_added, hosts_removed, absl::nullopt);
   EXPECT_EQ(0, factory_.stats_.counter("cluster_manager.cluster_updated").value());
   EXPECT_EQ(0, factory_.stats_.counter("cluster_manager.cluster_updated_via_merge").value());
@@ -2299,7 +2299,7 @@ TEST_F(ClusterManagerImplTest, MergedUpdatesOutOfWindowDisabled) {
   // The first update should be applied immediately, because even though it's mergeable
   // and outside a merge window, merging is disabled.
   cluster.prioritySet().updateHosts(
-      0, HostSetImpl::updateHostsParams(hosts, hosts_per_locality, hosts, hosts_per_locality), {},
+      0, HostSetImpl::updateHostsParams(hosts, hosts_per_locality, std::make_shared<const HealthyHostVector>(*hosts), hosts_per_locality), {},
       hosts_added, hosts_removed, absl::nullopt);
   EXPECT_EQ(1, factory_.stats_.counter("cluster_manager.cluster_updated").value());
   EXPECT_EQ(0, factory_.stats_.counter("cluster_manager.cluster_updated_via_merge").value());
@@ -2363,7 +2363,7 @@ TEST_F(ClusterManagerImplTest, MergedUpdatesDestroyedOnUpdate) {
   // The first update should be applied immediately, since it's not mergeable.
   hosts_removed.push_back((*hosts)[0]);
   cluster.prioritySet().updateHosts(
-      0, HostSetImpl::updateHostsParams(hosts, hosts_per_locality, hosts, hosts_per_locality), {},
+      0, HostSetImpl::updateHostsParams(hosts, hosts_per_locality, std::make_shared<const HealthyHostVector>(*hosts), hosts_per_locality), {},
       hosts_added, hosts_removed, absl::nullopt);
   EXPECT_EQ(1, factory_.stats_.counter("cluster_manager.cluster_updated").value());
   EXPECT_EQ(0, factory_.stats_.counter("cluster_manager.cluster_updated_via_merge").value());
@@ -2372,10 +2372,10 @@ TEST_F(ClusterManagerImplTest, MergedUpdatesDestroyedOnUpdate) {
   // These calls should be merged, since there are no added/removed hosts.
   hosts_removed.clear();
   cluster.prioritySet().updateHosts(
-      0, HostSetImpl::updateHostsParams(hosts, hosts_per_locality, hosts, hosts_per_locality), {},
+      0, HostSetImpl::updateHostsParams(hosts, hosts_per_locality, std::make_shared<const HealthyHostVector>(*hosts), hosts_per_locality), {},
       hosts_added, hosts_removed, absl::nullopt);
   cluster.prioritySet().updateHosts(
-      0, HostSetImpl::updateHostsParams(hosts, hosts_per_locality, hosts, hosts_per_locality), {},
+      0, HostSetImpl::updateHostsParams(hosts, hosts_per_locality, std::make_shared<const HealthyHostVector>(*hosts), hosts_per_locality), {},
       hosts_added, hosts_removed, absl::nullopt);
   EXPECT_EQ(1, factory_.stats_.counter("cluster_manager.cluster_updated").value());
   EXPECT_EQ(0, factory_.stats_.counter("cluster_manager.cluster_updated_via_merge").value());

From 44d72314d8892d63cc68dbdeb674a84a966317b1 Mon Sep 17 00:00:00 2001
From: Snow Pettersen <snowp@squareup.com>
Date: Sat, 13 Apr 2019 12:58:09 -0400
Subject: [PATCH 5/8] format

Signed-off-by: Snow Pettersen <snowp@squareup.com>
---
 .../common/upstream/cluster_manager_impl.cc   |  6 +-
 source/common/upstream/upstream_impl.cc       | 10 +-
 .../upstream/cluster_manager_impl_test.cc     | 98 +++++++++++++------
 3 files changed, 80 insertions(+), 34 deletions(-)

diff --git a/source/common/upstream/cluster_manager_impl.cc b/source/common/upstream/cluster_manager_impl.cc
index 332e88ed588ce..c73bf5b639c92 100644
--- a/source/common/upstream/cluster_manager_impl.cc
+++ b/source/common/upstream/cluster_manager_impl.cc
@@ -678,8 +678,10 @@ void ClusterManagerImpl::postThreadLocalClusterUpdate(const Cluster& cluster, ui
 
   // TODO(htuch): Can we skip these copies by exporting out const shared_ptr from HostSet?
   HostVectorConstSharedPtr hosts_copy(new HostVector(host_set->hosts()));
-  HealthyHostVectorConstSharedPtr healthy_hosts_copy(new HealthyHostVector(host_set->healthyHosts()));
-  DegradedHostVectorConstSharedPtr degraded_hosts_copy(new DegradedHostVector(host_set->degradedHosts()));
+  HealthyHostVectorConstSharedPtr healthy_hosts_copy(
+      new HealthyHostVector(host_set->healthyHosts()));
+  DegradedHostVectorConstSharedPtr degraded_hosts_copy(
+      new DegradedHostVector(host_set->degradedHosts()));
   HostsPerLocalityConstSharedPtr hosts_per_locality_copy = host_set->hostsPerLocality().clone();
   HostsPerLocalityConstSharedPtr healthy_hosts_per_locality_copy =
       host_set->healthyHostsPerLocality().clone();
diff --git a/source/common/upstream/upstream_impl.cc b/source/common/upstream/upstream_impl.cc
index 9f17d5c2e7e91..fbc27861a3f4f 100644
--- a/source/common/upstream/upstream_impl.cc
+++ b/source/common/upstream/upstream_impl.cc
@@ -329,8 +329,8 @@ void HostSetImpl::updateHosts(PrioritySet::UpdateHostsParams&& update_hosts_para
                            *healthy_hosts_per_locality_, healthy_hosts_->get(), hosts_per_locality_,
                            locality_weights_, overprovisioning_factor_);
   rebuildLocalityScheduler(degraded_locality_scheduler_, degraded_locality_entries_,
-                           *degraded_hosts_per_locality_, degraded_hosts_->get(), hosts_per_locality_,
-                           locality_weights_, overprovisioning_factor_);
+                           *degraded_hosts_per_locality_, degraded_hosts_->get(),
+                           hosts_per_locality_, locality_weights_, overprovisioning_factor_);
 
   runUpdateCallbacks(hosts_added, hosts_removed);
 }
@@ -403,7 +403,8 @@ PrioritySet::UpdateHostsParams
 HostSetImpl::updateHostsParams(HostVectorConstSharedPtr hosts,
                                HostsPerLocalityConstSharedPtr hosts_per_locality) {
   return updateHostsParams(std::move(hosts), std::move(hosts_per_locality),
-                           std::make_shared<const HealthyHostVector>(), HostsPerLocalityImpl::empty());
+                           std::make_shared<const HealthyHostVector>(),
+                           HostsPerLocalityImpl::empty());
 }
 
 PrioritySet::UpdateHostsParams
@@ -413,7 +414,8 @@ HostSetImpl::updateHostsParams(HostVectorConstSharedPtr hosts,
                                HostsPerLocalityConstSharedPtr healthy_hosts_per_locality) {
   return updateHostsParams(std::move(hosts), std::move(hosts_per_locality),
                            std::move(healthy_hosts), std::move(healthy_hosts_per_locality),
-                           std::make_shared<const DegradedHostVector>(), HostsPerLocalityImpl::empty());
+                           std::make_shared<const DegradedHostVector>(),
+                           HostsPerLocalityImpl::empty());
 }
 
 PrioritySet::UpdateHostsParams
diff --git a/test/common/upstream/cluster_manager_impl_test.cc b/test/common/upstream/cluster_manager_impl_test.cc
index 09f5fe068cfaf..0d56b94d81a7f 100644
--- a/test/common/upstream/cluster_manager_impl_test.cc
+++ b/test/common/upstream/cluster_manager_impl_test.cc
@@ -2147,8 +2147,11 @@ TEST_F(ClusterManagerImplTest, MergedUpdates) {
   // The first update should be applied immediately, since it's not mergeable.
   hosts_removed.push_back((*hosts)[0]);
   cluster.prioritySet().updateHosts(
-      0, HostSetImpl::updateHostsParams(hosts, hosts_per_locality, std::make_shared<const HealthyHostVector>(*hosts), hosts_per_locality), {},
-      hosts_added, hosts_removed, absl::nullopt);
+      0,
+      HostSetImpl::updateHostsParams(hosts, hosts_per_locality,
+                                     std::make_shared<const HealthyHostVector>(*hosts),
+                                     hosts_per_locality),
+      {}, hosts_added, hosts_removed, absl::nullopt);
   EXPECT_EQ(1, factory_.stats_.counter("cluster_manager.cluster_updated").value());
   EXPECT_EQ(0, factory_.stats_.counter("cluster_manager.cluster_updated_via_merge").value());
   EXPECT_EQ(0, factory_.stats_.counter("cluster_manager.update_merge_cancelled").value());
@@ -2156,11 +2159,17 @@ TEST_F(ClusterManagerImplTest, MergedUpdates) {
   // These calls should be merged, since there are no added/removed hosts.
   hosts_removed.clear();
   cluster.prioritySet().updateHosts(
-      0, HostSetImpl::updateHostsParams(hosts, hosts_per_locality, std::make_shared<const HealthyHostVector>(*hosts), hosts_per_locality), {},
-      hosts_added, hosts_removed, absl::nullopt);
+      0,
+      HostSetImpl::updateHostsParams(hosts, hosts_per_locality,
+                                     std::make_shared<const HealthyHostVector>(*hosts),
+                                     hosts_per_locality),
+      {}, hosts_added, hosts_removed, absl::nullopt);
   cluster.prioritySet().updateHosts(
-      0, HostSetImpl::updateHostsParams(hosts, hosts_per_locality, std::make_shared<const HealthyHostVector>(*hosts), hosts_per_locality), {},
-      hosts_added, hosts_removed, absl::nullopt);
+      0,
+      HostSetImpl::updateHostsParams(hosts, hosts_per_locality,
+                                     std::make_shared<const HealthyHostVector>(*hosts),
+                                     hosts_per_locality),
+      {}, hosts_added, hosts_removed, absl::nullopt);
   EXPECT_EQ(1, factory_.stats_.counter("cluster_manager.cluster_updated").value());
   EXPECT_EQ(0, factory_.stats_.counter("cluster_manager.cluster_updated_via_merge").value());
   EXPECT_EQ(0, factory_.stats_.counter("cluster_manager.update_merge_cancelled").value());
@@ -2175,8 +2184,11 @@ TEST_F(ClusterManagerImplTest, MergedUpdates) {
   hosts_removed.clear();
   hosts_added.push_back((*hosts)[0]);
   cluster.prioritySet().updateHosts(
-      0, HostSetImpl::updateHostsParams(hosts, hosts_per_locality, std::make_shared<const HealthyHostVector>(*hosts), hosts_per_locality), {},
-      hosts_added, hosts_removed, absl::nullopt);
+      0,
+      HostSetImpl::updateHostsParams(hosts, hosts_per_locality,
+                                     std::make_shared<const HealthyHostVector>(*hosts),
+                                     hosts_per_locality),
+      {}, hosts_added, hosts_removed, absl::nullopt);
   EXPECT_EQ(2, factory_.stats_.counter("cluster_manager.cluster_updated").value());
   EXPECT_EQ(1, factory_.stats_.counter("cluster_manager.cluster_updated_via_merge").value());
   EXPECT_EQ(0, factory_.stats_.counter("cluster_manager.update_merge_cancelled").value());
@@ -2186,18 +2198,27 @@ TEST_F(ClusterManagerImplTest, MergedUpdates) {
 
   (*hosts)[0]->metadata(buildMetadata("v1"));
   cluster.prioritySet().updateHosts(
-      0, HostSetImpl::updateHostsParams(hosts, hosts_per_locality, std::make_shared<const HealthyHostVector>(*hosts), hosts_per_locality), {},
-      hosts_added, hosts_removed, absl::nullopt);
+      0,
+      HostSetImpl::updateHostsParams(hosts, hosts_per_locality,
+                                     std::make_shared<const HealthyHostVector>(*hosts),
+                                     hosts_per_locality),
+      {}, hosts_added, hosts_removed, absl::nullopt);
 
   (*hosts)[0]->healthFlagSet(Host::HealthFlag::FAILED_EDS_HEALTH);
   cluster.prioritySet().updateHosts(
-      0, HostSetImpl::updateHostsParams(hosts, hosts_per_locality, std::make_shared<const HealthyHostVector>(*hosts), hosts_per_locality), {},
-      hosts_added, hosts_removed, absl::nullopt);
+      0,
+      HostSetImpl::updateHostsParams(hosts, hosts_per_locality,
+                                     std::make_shared<const HealthyHostVector>(*hosts),
+                                     hosts_per_locality),
+      {}, hosts_added, hosts_removed, absl::nullopt);
 
   (*hosts)[0]->weight(100);
   cluster.prioritySet().updateHosts(
-      0, HostSetImpl::updateHostsParams(hosts, hosts_per_locality, std::make_shared<const HealthyHostVector>(*hosts), hosts_per_locality), {},
-      hosts_added, hosts_removed, absl::nullopt);
+      0,
+      HostSetImpl::updateHostsParams(hosts, hosts_per_locality,
+                                     std::make_shared<const HealthyHostVector>(*hosts),
+                                     hosts_per_locality),
+      {}, hosts_added, hosts_removed, absl::nullopt);
 
   // Updates not delivered yet.
   EXPECT_EQ(2, factory_.stats_.counter("cluster_manager.cluster_updated").value());
@@ -2207,8 +2228,11 @@ TEST_F(ClusterManagerImplTest, MergedUpdates) {
   // Remove the host again, should cancel the scheduled update and be delivered immediately.
   hosts_removed.push_back((*hosts)[0]);
   cluster.prioritySet().updateHosts(
-      0, HostSetImpl::updateHostsParams(hosts, hosts_per_locality, std::make_shared<const HealthyHostVector>(*hosts), hosts_per_locality), {},
-      hosts_added, hosts_removed, absl::nullopt);
+      0,
+      HostSetImpl::updateHostsParams(hosts, hosts_per_locality,
+                                     std::make_shared<const HealthyHostVector>(*hosts),
+                                     hosts_per_locality),
+      {}, hosts_added, hosts_removed, absl::nullopt);
 
   EXPECT_EQ(3, factory_.stats_.counter("cluster_manager.cluster_updated").value());
   EXPECT_EQ(1, factory_.stats_.counter("cluster_manager.cluster_updated_via_merge").value());
@@ -2241,8 +2265,11 @@ TEST_F(ClusterManagerImplTest, MergedUpdatesOutOfWindow) {
   // cluster.info()->lbConfig().update_merge_window() in ClusterManagerImpl::scheduleUpdate.
   time_system_.sleep(std::chrono::seconds(60));
   cluster.prioritySet().updateHosts(
-      0, HostSetImpl::updateHostsParams(hosts, hosts_per_locality, std::make_shared<const HealthyHostVector>(*hosts), hosts_per_locality), {},
-      hosts_added, hosts_removed, absl::nullopt);
+      0,
+      HostSetImpl::updateHostsParams(hosts, hosts_per_locality,
+                                     std::make_shared<const HealthyHostVector>(*hosts),
+                                     hosts_per_locality),
+      {}, hosts_added, hosts_removed, absl::nullopt);
   EXPECT_EQ(1, factory_.stats_.counter("cluster_manager.cluster_updated").value());
   EXPECT_EQ(0, factory_.stats_.counter("cluster_manager.cluster_updated_via_merge").value());
   EXPECT_EQ(1, factory_.stats_.counter("cluster_manager.update_out_of_merge_window").value());
@@ -2266,8 +2293,11 @@ TEST_F(ClusterManagerImplTest, MergedUpdatesInsideWindow) {
   // default-initialized to a monotonic time of 0, as is SimulatedTimeSystem::monotonic_time_.
   time_system_.sleep(std::chrono::seconds(2));
   cluster.prioritySet().updateHosts(
-      0, HostSetImpl::updateHostsParams(hosts, hosts_per_locality, std::make_shared<const HealthyHostVector>(*hosts), hosts_per_locality), {},
-      hosts_added, hosts_removed, absl::nullopt);
+      0,
+      HostSetImpl::updateHostsParams(hosts, hosts_per_locality,
+                                     std::make_shared<const HealthyHostVector>(*hosts),
+                                     hosts_per_locality),
+      {}, hosts_added, hosts_removed, absl::nullopt);
   EXPECT_EQ(0, factory_.stats_.counter("cluster_manager.cluster_updated").value());
   EXPECT_EQ(0, factory_.stats_.counter("cluster_manager.cluster_updated_via_merge").value());
   EXPECT_EQ(0, factory_.stats_.counter("cluster_manager.update_out_of_merge_window").value());
@@ -2299,8 +2329,11 @@ TEST_F(ClusterManagerImplTest, MergedUpdatesOutOfWindowDisabled) {
   // The first update should be applied immediately, because even though it's mergeable
   // and outside a merge window, merging is disabled.
   cluster.prioritySet().updateHosts(
-      0, HostSetImpl::updateHostsParams(hosts, hosts_per_locality, std::make_shared<const HealthyHostVector>(*hosts), hosts_per_locality), {},
-      hosts_added, hosts_removed, absl::nullopt);
+      0,
+      HostSetImpl::updateHostsParams(hosts, hosts_per_locality,
+                                     std::make_shared<const HealthyHostVector>(*hosts),
+                                     hosts_per_locality),
+      {}, hosts_added, hosts_removed, absl::nullopt);
   EXPECT_EQ(1, factory_.stats_.counter("cluster_manager.cluster_updated").value());
   EXPECT_EQ(0, factory_.stats_.counter("cluster_manager.cluster_updated_via_merge").value());
   EXPECT_EQ(0, factory_.stats_.counter("cluster_manager.update_out_of_merge_window").value());
@@ -2363,8 +2396,11 @@ TEST_F(ClusterManagerImplTest, MergedUpdatesDestroyedOnUpdate) {
   // The first update should be applied immediately, since it's not mergeable.
   hosts_removed.push_back((*hosts)[0]);
   cluster.prioritySet().updateHosts(
-      0, HostSetImpl::updateHostsParams(hosts, hosts_per_locality, std::make_shared<const HealthyHostVector>(*hosts), hosts_per_locality), {},
-      hosts_added, hosts_removed, absl::nullopt);
+      0,
+      HostSetImpl::updateHostsParams(hosts, hosts_per_locality,
+                                     std::make_shared<const HealthyHostVector>(*hosts),
+                                     hosts_per_locality),
+      {}, hosts_added, hosts_removed, absl::nullopt);
   EXPECT_EQ(1, factory_.stats_.counter("cluster_manager.cluster_updated").value());
   EXPECT_EQ(0, factory_.stats_.counter("cluster_manager.cluster_updated_via_merge").value());
   EXPECT_EQ(0, factory_.stats_.counter("cluster_manager.update_merge_cancelled").value());
@@ -2372,11 +2408,17 @@ TEST_F(ClusterManagerImplTest, MergedUpdatesDestroyedOnUpdate) {
   // These calls should be merged, since there are no added/removed hosts.
   hosts_removed.clear();
   cluster.prioritySet().updateHosts(
-      0, HostSetImpl::updateHostsParams(hosts, hosts_per_locality, std::make_shared<const HealthyHostVector>(*hosts), hosts_per_locality), {},
-      hosts_added, hosts_removed, absl::nullopt);
+      0,
+      HostSetImpl::updateHostsParams(hosts, hosts_per_locality,
+                                     std::make_shared<const HealthyHostVector>(*hosts),
+                                     hosts_per_locality),
+      {}, hosts_added, hosts_removed, absl::nullopt);
   cluster.prioritySet().updateHosts(
-      0, HostSetImpl::updateHostsParams(hosts, hosts_per_locality, std::make_shared<const HealthyHostVector>(*hosts), hosts_per_locality), {},
-      hosts_added, hosts_removed, absl::nullopt);
+      0,
+      HostSetImpl::updateHostsParams(hosts, hosts_per_locality,
+                                     std::make_shared<const HealthyHostVector>(*hosts),
+                                     hosts_per_locality),
+      {}, hosts_added, hosts_removed, absl::nullopt);
   EXPECT_EQ(1, factory_.stats_.counter("cluster_manager.cluster_updated").value());
   EXPECT_EQ(0, factory_.stats_.counter("cluster_manager.cluster_updated_via_merge").value());
   EXPECT_EQ(0, factory_.stats_.counter("cluster_manager.update_merge_cancelled").value());

From 2b55e312685219020b3c32b25bef08025aec7e86 Mon Sep 17 00:00:00 2001
From: Snow Pettersen <snowp@squareup.com>
Date: Sat, 13 Apr 2019 18:40:00 -0400
Subject: [PATCH 6/8] fix more phantom usages

Signed-off-by: Snow Pettersen <snowp@squareup.com>
---
 .../upstream/load_balancer_benchmark.cc       |  7 ++-
 .../upstream/load_balancer_impl_test.cc       | 60 +++++++++++++------
 .../upstream/load_balancer_simulation_test.cc | 15 +++--
 .../upstream/original_dst_cluster_test.cc     |  4 +-
 test/common/upstream/subset_lb_test.cc        | 12 ++--
 5 files changed, 65 insertions(+), 33 deletions(-)

diff --git a/test/common/upstream/load_balancer_benchmark.cc b/test/common/upstream/load_balancer_benchmark.cc
index 7cdade43e5fb9..329e4eef53c4b 100644
--- a/test/common/upstream/load_balancer_benchmark.cc
+++ b/test/common/upstream/load_balancer_benchmark.cc
@@ -29,8 +29,11 @@ class BaseTester {
     }
     HostVectorConstSharedPtr updated_hosts{new HostVector(hosts)};
     priority_set_.updateHosts(
-        0, HostSetImpl::updateHostsParams(updated_hosts, nullptr, updated_hosts, nullptr), {},
-        hosts, {}, absl::nullopt);
+        0,
+        HostSetImpl::updateHostsParams(updated_hosts, nullptr,
+                                       std::make_shared<const HealthyHostVector>(*updated_hosts),
+                                       nullptr),
+        {}, hosts, {}, absl::nullopt);
   }
 
   PrioritySetImpl priority_set_;
diff --git a/test/common/upstream/load_balancer_impl_test.cc b/test/common/upstream/load_balancer_impl_test.cc
index 2d167d09d746b..801064893fa05 100644
--- a/test/common/upstream/load_balancer_impl_test.cc
+++ b/test/common/upstream/load_balancer_impl_test.cc
@@ -555,7 +555,8 @@ TEST_P(FailoverTest, ExtendPrioritiesWithLocalPrioritySet) {
   HostVectorSharedPtr hosts(new HostVector({makeTestHost(info_, "tcp://127.0.0.1:82")}));
   local_priority_set_->updateHosts(
       0,
-      HostSetImpl::updateHostsParams(hosts, HostsPerLocalityImpl::empty(), hosts,
+      HostSetImpl::updateHostsParams(hosts, HostsPerLocalityImpl::empty(),
+                                     std::make_shared<const HealthyHostVector>(*hosts),
                                      HostsPerLocalityImpl::empty()),
       {}, empty_host_vector_, empty_host_vector_, absl::nullopt);
   EXPECT_EQ(tertiary_host_set_.hosts_[0], lb_->chooseHost(nullptr));
@@ -825,8 +826,11 @@ TEST_P(RoundRobinLoadBalancerTest, ZoneAwareSmallCluster) {
   common_config_.mutable_zone_aware_lb_config()->mutable_min_cluster_size()->set_value(7);
   init(true);
   local_priority_set_->updateHosts(
-      0, HostSetImpl::updateHostsParams(hosts, hosts_per_locality, hosts, hosts_per_locality), {},
-      empty_host_vector_, empty_host_vector_, absl::nullopt);
+      0,
+      HostSetImpl::updateHostsParams(hosts, hosts_per_locality,
+                                     std::make_shared<const HealthyHostVector>(*hosts),
+                                     hosts_per_locality),
+      {}, empty_host_vector_, empty_host_vector_, absl::nullopt);
 
   EXPECT_CALL(runtime_.snapshot_, getInteger("upstream.healthy_panic_threshold", 0))
       .WillRepeatedly(Return(50));
@@ -850,8 +854,11 @@ TEST_P(RoundRobinLoadBalancerTest, ZoneAwareSmallCluster) {
       .WillRepeatedly(Return(1));
   // Trigger reload.
   local_priority_set_->updateHosts(
-      0, HostSetImpl::updateHostsParams(hosts, hosts_per_locality, hosts, hosts_per_locality), {},
-      empty_host_vector_, empty_host_vector_, absl::nullopt);
+      0,
+      HostSetImpl::updateHostsParams(hosts, hosts_per_locality,
+                                     std::make_shared<const HealthyHostVector>(*hosts),
+                                     hosts_per_locality),
+      {}, empty_host_vector_, empty_host_vector_, absl::nullopt);
   EXPECT_EQ(hostSet().healthy_hosts_per_locality_->get()[0][0], lb_->chooseHost(nullptr));
 }
 
@@ -876,10 +883,12 @@ TEST_P(RoundRobinLoadBalancerTest, NoZoneAwareDifferentZoneSize) {
   common_config_.mutable_zone_aware_lb_config()->mutable_routing_enabled()->set_value(98);
   common_config_.mutable_zone_aware_lb_config()->mutable_min_cluster_size()->set_value(7);
   init(true);
-  local_priority_set_->updateHosts(0,
-                                   HostSetImpl::updateHostsParams(hosts, local_hosts_per_locality,
-                                                                  hosts, local_hosts_per_locality),
-                                   {}, empty_host_vector_, empty_host_vector_, absl::nullopt);
+  local_priority_set_->updateHosts(
+      0,
+      HostSetImpl::updateHostsParams(hosts, local_hosts_per_locality,
+                                     std::make_shared<const HealthyHostVector>(*hosts),
+                                     local_hosts_per_locality),
+      {}, empty_host_vector_, empty_host_vector_, absl::nullopt);
 
   EXPECT_CALL(runtime_.snapshot_, getInteger("upstream.healthy_panic_threshold", 100))
       .WillRepeatedly(Return(50));
@@ -916,8 +925,11 @@ TEST_P(RoundRobinLoadBalancerTest, ZoneAwareRoutingLargeZoneSwitchOnOff) {
   hostSet().healthy_hosts_per_locality_ = hosts_per_locality;
   init(true);
   local_priority_set_->updateHosts(
-      0, HostSetImpl::updateHostsParams(hosts, hosts_per_locality, hosts, hosts_per_locality), {},
-      empty_host_vector_, empty_host_vector_, absl::nullopt);
+      0,
+      HostSetImpl::updateHostsParams(hosts, hosts_per_locality,
+                                     std::make_shared<const HealthyHostVector>(*hosts),
+                                     hosts_per_locality),
+      {}, empty_host_vector_, empty_host_vector_, absl::nullopt);
 
   // There is only one host in the given zone for zone aware routing.
   EXPECT_EQ(hostSet().healthy_hosts_per_locality_->get()[0][0], lb_->chooseHost(nullptr));
@@ -966,7 +978,8 @@ TEST_P(RoundRobinLoadBalancerTest, ZoneAwareRoutingSmallZone) {
   init(true);
   local_priority_set_->updateHosts(
       0,
-      HostSetImpl::updateHostsParams(local_hosts, local_hosts_per_locality, local_hosts,
+      HostSetImpl::updateHostsParams(local_hosts, local_hosts_per_locality,
+                                     std::make_shared<const HealthyHostVector>(*local_hosts),
                                      local_hosts_per_locality),
       {}, empty_host_vector_, empty_host_vector_, absl::nullopt);
 
@@ -1039,7 +1052,8 @@ TEST_P(RoundRobinLoadBalancerTest, LowPrecisionForDistribution) {
   auto local_hosts_per_locality_shared = makeHostsPerLocality(std::move(local_hosts_per_locality));
   local_priority_set_->updateHosts(
       0,
-      HostSetImpl::updateHostsParams(local_hosts, local_hosts_per_locality_shared, local_hosts,
+      HostSetImpl::updateHostsParams(local_hosts, local_hosts_per_locality_shared,
+                                     std::make_shared<const HealthyHostVector>(*local_hosts),
                                      local_hosts_per_locality_shared),
       {}, empty_host_vector_, empty_host_vector_, absl::nullopt);
 
@@ -1062,8 +1076,11 @@ TEST_P(RoundRobinLoadBalancerTest, NoZoneAwareRoutingOneZone) {
   hostSet().healthy_hosts_per_locality_ = hosts_per_locality;
   init(true);
   local_priority_set_->updateHosts(
-      0, HostSetImpl::updateHostsParams(hosts, hosts_per_locality, hosts, hosts_per_locality), {},
-      empty_host_vector_, empty_host_vector_, absl::nullopt);
+      0,
+      HostSetImpl::updateHostsParams(hosts, hosts_per_locality,
+                                     std::make_shared<const HealthyHostVector>(*hosts),
+                                     hosts_per_locality),
+      {}, empty_host_vector_, empty_host_vector_, absl::nullopt);
   EXPECT_EQ(hostSet().healthy_hosts_[0], lb_->chooseHost(nullptr));
 }
 
@@ -1078,8 +1095,11 @@ TEST_P(RoundRobinLoadBalancerTest, NoZoneAwareRoutingNotHealthy) {
   hostSet().healthy_hosts_per_locality_ = hosts_per_locality;
   init(true);
   local_priority_set_->updateHosts(
-      0, HostSetImpl::updateHostsParams(hosts, hosts_per_locality, hosts, hosts_per_locality), {},
-      empty_host_vector_, empty_host_vector_, absl::nullopt);
+      0,
+      HostSetImpl::updateHostsParams(hosts, hosts_per_locality,
+                                     std::make_shared<const HealthyHostVector>(*hosts),
+                                     hosts_per_locality),
+      {}, empty_host_vector_, empty_host_vector_, absl::nullopt);
 
   // local zone has no healthy hosts, take from the all healthy hosts.
   EXPECT_EQ(hostSet().healthy_hosts_[0], lb_->chooseHost(nullptr));
@@ -1111,7 +1131,8 @@ TEST_P(RoundRobinLoadBalancerTest, NoZoneAwareRoutingLocalEmpty) {
   init(true);
   local_priority_set_->updateHosts(
       0,
-      HostSetImpl::updateHostsParams(local_hosts, local_hosts_per_locality, local_hosts,
+      HostSetImpl::updateHostsParams(local_hosts, local_hosts_per_locality,
+                                     std::make_shared<const HealthyHostVector>(*local_hosts),
                                      local_hosts_per_locality),
       {}, empty_host_vector_, empty_host_vector_, absl::nullopt);
 
@@ -1142,7 +1163,8 @@ TEST_P(RoundRobinLoadBalancerTest, NoZoneAwareRoutingNoLocalLocality) {
   init(true);
   local_priority_set_->updateHosts(
       0,
-      HostSetImpl::updateHostsParams(local_hosts, local_hosts_per_locality, local_hosts,
+      HostSetImpl::updateHostsParams(local_hosts, local_hosts_per_locality,
+                                     std::make_shared<const HealthyHostVector>(*local_hosts),
                                      local_hosts_per_locality),
       {}, empty_host_vector_, empty_host_vector_, absl::nullopt);
 
diff --git a/test/common/upstream/load_balancer_simulation_test.cc b/test/common/upstream/load_balancer_simulation_test.cc
index 7167c9fd404b0..76f820a161bdd 100644
--- a/test/common/upstream/load_balancer_simulation_test.cc
+++ b/test/common/upstream/load_balancer_simulation_test.cc
@@ -54,10 +54,12 @@ TEST(DISABLED_LeastRequestLoadBalancerWeightTest, Weight) {
   }
   HostVectorConstSharedPtr updated_hosts{new HostVector(hosts)};
   HostsPerLocalitySharedPtr updated_locality_hosts{new HostsPerLocalityImpl(hosts)};
-  priority_set.updateHosts(0,
-                           HostSetImpl::updateHostsParams(updated_hosts, updated_locality_hosts,
-                                                          updated_hosts, updated_locality_hosts),
-                           {}, hosts, {}, absl::nullopt);
+  priority_set.updateHosts(
+      0,
+      HostSetImpl::updateHostsParams(updated_hosts, updated_locality_hosts,
+                                     std::make_shared<const HealthyHostVector>(*updated_hosts),
+                                     updated_locality_hosts),
+      {}, hosts, {}, absl::nullopt);
 
   Stats::IsolatedStoreImpl stats_store;
   ClusterStats stats{ClusterInfoImpl::generateStats(stats_store)};
@@ -160,8 +162,9 @@ class DISABLED_SimulationTest : public testing::Test {
       auto per_zone_local_shared = makeHostsPerLocality(std::move(per_zone_local));
       local_priority_set_->updateHosts(
           0,
-          HostSetImpl::updateHostsParams(originating_hosts, per_zone_local_shared,
-                                         originating_hosts, per_zone_local_shared),
+          HostSetImpl::updateHostsParams(
+              originating_hosts, per_zone_local_shared,
+              std::make_shared<const HealthyHostVector>(*originating_hosts), per_zone_local_shared),
           {}, empty_vector_, empty_vector_, absl::nullopt);
 
       HostConstSharedPtr selected = lb.chooseHost(nullptr);
diff --git a/test/common/upstream/original_dst_cluster_test.cc b/test/common/upstream/original_dst_cluster_test.cc
index 79360a0cc48d0..23314f2f25fb3 100644
--- a/test/common/upstream/original_dst_cluster_test.cc
+++ b/test/common/upstream/original_dst_cluster_test.cc
@@ -461,8 +461,8 @@ TEST_F(OriginalDstClusterTest, MultipleClusters) {
         // Update second hostset accordingly;
         HostVectorSharedPtr new_hosts(
             new HostVector(cluster_->prioritySet().hostSetsPerPriority()[0]->hosts()));
-        HostVectorSharedPtr healthy_hosts(
-            new HostVector(cluster_->prioritySet().hostSetsPerPriority()[0]->hosts()));
+        auto healthy_hosts = std::make_shared<const HealthyHostVector>(
+            cluster_->prioritySet().hostSetsPerPriority()[0]->hosts());
         const HostsPerLocalityConstSharedPtr empty_hosts_per_locality{new HostsPerLocalityImpl()};
 
         second.updateHosts(0,
diff --git a/test/common/upstream/subset_lb_test.cc b/test/common/upstream/subset_lb_test.cc
index 6dbd9b62c047e..f2b97e03278d6 100644
--- a/test/common/upstream/subset_lb_test.cc
+++ b/test/common/upstream/subset_lb_test.cc
@@ -216,7 +216,8 @@ class SubsetLoadBalancerTest : public testing::TestWithParam<UpdateOrder> {
 
     local_priority_set_.updateHosts(
         0,
-        HostSetImpl::updateHostsParams(local_hosts_, local_hosts_per_locality_, local_hosts_,
+        HostSetImpl::updateHostsParams(local_hosts_, local_hosts_per_locality_,
+                                       std::make_shared<HealthyHostVector>(*local_hosts_),
                                        local_hosts_per_locality_),
         {}, {}, {}, absl::nullopt);
 
@@ -314,7 +315,8 @@ class SubsetLoadBalancerTest : public testing::TestWithParam<UpdateOrder> {
     if (GetParam() == REMOVES_FIRST && !remove.empty()) {
       local_priority_set_.updateHosts(
           0,
-          HostSetImpl::updateHostsParams(local_hosts_, local_hosts_per_locality_, local_hosts_,
+          HostSetImpl::updateHostsParams(local_hosts_, local_hosts_per_locality_,
+                                         std::make_shared<HealthyHostVector>(*local_hosts_),
                                          local_hosts_per_locality_),
           {}, {}, remove, absl::nullopt);
     }
@@ -330,14 +332,16 @@ class SubsetLoadBalancerTest : public testing::TestWithParam<UpdateOrder> {
       if (!add.empty()) {
         local_priority_set_.updateHosts(
             0,
-            HostSetImpl::updateHostsParams(local_hosts_, local_hosts_per_locality_, local_hosts_,
+            HostSetImpl::updateHostsParams(local_hosts_, local_hosts_per_locality_,
+                                           std::make_shared<HealthyHostVector>(*local_hosts_),
                                            local_hosts_per_locality_),
             {}, add, {}, absl::nullopt);
       }
     } else if (!add.empty() || !remove.empty()) {
       local_priority_set_.updateHosts(
           0,
-          HostSetImpl::updateHostsParams(local_hosts_, local_hosts_per_locality_, local_hosts_,
+          HostSetImpl::updateHostsParams(local_hosts_, local_hosts_per_locality_,
+                                         std::make_shared<const HealthyHostVector>(*local_hosts_),
                                          local_hosts_per_locality_),
           {}, add, remove, absl::nullopt);
     }

From 90a2c86d7c446eef8efeb29fade32d8f22cf8782 Mon Sep 17 00:00:00 2001
From: Snow Pettersen <snowp@squareup.com>
Date: Sun, 14 Apr 2019 17:16:08 -0400
Subject: [PATCH 7/8] format

Signed-off-by: Snow Pettersen <snowp@squareup.com>
---
 include/envoy/upstream/upstream.h          |  2 +-
 test/common/upstream/upstream_impl_test.cc | 61 +++++++++++++---------
 2 files changed, 38 insertions(+), 25 deletions(-)

diff --git a/include/envoy/upstream/upstream.h b/include/envoy/upstream/upstream.h
index 5f12f30d93ce2..f6599c0b7254a 100644
--- a/include/envoy/upstream/upstream.h
+++ b/include/envoy/upstream/upstream.h
@@ -20,9 +20,9 @@
 #include "envoy/upstream/health_check_host_monitor.h"
 #include "envoy/upstream/load_balancer_type.h"
 #include "envoy/upstream/locality.h"
-#include "envoy/upstream/types.h"
 #include "envoy/upstream/outlier_detection.h"
 #include "envoy/upstream/resource_manager.h"
+#include "envoy/upstream/types.h"
 
 #include "absl/types/optional.h"
 
diff --git a/test/common/upstream/upstream_impl_test.cc b/test/common/upstream/upstream_impl_test.cc
index 053ee73e33910..64864f91f29fc 100644
--- a/test/common/upstream/upstream_impl_test.cc
+++ b/test/common/upstream/upstream_impl_test.cc
@@ -69,7 +69,7 @@ std::list<std::string> hostListToAddresses(const HostVector& hosts) {
   return addresses;
 }
 
-template<class HostsT=HostVector>
+template <class HostsT = HostVector>
 std::shared_ptr<const HostsT>
 makeHostsFromHostsPerLocality(HostsPerLocalityConstSharedPtr hosts_per_locality) {
   HostVector hosts;
@@ -1590,7 +1590,9 @@ class TestBatchUpdateCb : public PrioritySet::BatchUpdateCb {
       HostVector hosts_removed{};
       host_update_cb.updateHosts(
           0,
-          HostSetImpl::updateHostsParams(hosts_, hosts_per_locality_, std::make_shared<const HealthyHostVector>(*hosts_), hosts_per_locality_),
+          HostSetImpl::updateHostsParams(hosts_, hosts_per_locality_,
+                                         std::make_shared<const HealthyHostVector>(*hosts_),
+                                         hosts_per_locality_),
           {}, hosts_added, hosts_removed, absl::nullopt);
     }
 
@@ -1601,7 +1603,8 @@ class TestBatchUpdateCb : public PrioritySet::BatchUpdateCb {
       HostVector hosts_removed{hosts_->front()};
       host_update_cb.updateHosts(
           1,
-          HostSetImpl::updateHostsParams(empty_hosts, HostsPerLocalityImpl::empty(), std::make_shared<const HealthyHostVector>(*empty_hosts),
+          HostSetImpl::updateHostsParams(empty_hosts, HostsPerLocalityImpl::empty(),
+                                         std::make_shared<const HealthyHostVector>(*empty_hosts),
                                          HostsPerLocalityImpl::empty()),
           {}, hosts_added, hosts_removed, absl::nullopt);
     }
@@ -1654,8 +1657,11 @@ TEST(PrioritySet, Extend) {
     HostVector hosts_removed{};
 
     priority_set.updateHosts(
-        1, HostSetImpl::updateHostsParams(hosts, hosts_per_locality, std::make_shared<const HealthyHostVector>(*hosts), hosts_per_locality), {},
-        hosts_added, hosts_removed, absl::nullopt);
+        1,
+        HostSetImpl::updateHostsParams(hosts, hosts_per_locality,
+                                       std::make_shared<const HealthyHostVector>(*hosts),
+                                       hosts_per_locality),
+        {}, hosts_added, hosts_removed, absl::nullopt);
   }
   EXPECT_EQ(1, priority_changes);
   EXPECT_EQ(1, membership_changes);
@@ -2237,9 +2243,10 @@ TEST_F(HostSetImplLocalityTest, EmptyLocality) {
       makeHostsPerLocality({{hosts_[0], hosts_[1], hosts_[2]}, {}});
   LocalityWeightsConstSharedPtr locality_weights{new LocalityWeights{1, 1}};
   auto hosts = makeHostsFromHostsPerLocality(hosts_per_locality);
-  host_set_.updateHosts(
-      HostSetImpl::updateHostsParams(hosts, hosts_per_locality, std::make_shared<const HealthyHostVector>(*hosts), hosts_per_locality),
-      locality_weights, {}, {}, absl::nullopt);
+  host_set_.updateHosts(HostSetImpl::updateHostsParams(
+                            hosts, hosts_per_locality,
+                            std::make_shared<const HealthyHostVector>(*hosts), hosts_per_locality),
+                        locality_weights, {}, {}, absl::nullopt);
   // Verify that we are not RRing between localities.
   EXPECT_EQ(0, host_set_.chooseHealthyLocality().value());
   EXPECT_EQ(0, host_set_.chooseHealthyLocality().value());
@@ -2250,9 +2257,10 @@ TEST_F(HostSetImplLocalityTest, AllZeroWeights) {
   HostsPerLocalitySharedPtr hosts_per_locality = makeHostsPerLocality({{hosts_[0]}, {hosts_[1]}});
   LocalityWeightsConstSharedPtr locality_weights{new LocalityWeights{0, 0}};
   auto hosts = makeHostsFromHostsPerLocality(hosts_per_locality);
-  host_set_.updateHosts(
-      HostSetImpl::updateHostsParams(hosts, hosts_per_locality, std::make_shared<const HealthyHostVector>(*hosts), hosts_per_locality),
-      locality_weights, {}, {});
+  host_set_.updateHosts(HostSetImpl::updateHostsParams(
+                            hosts, hosts_per_locality,
+                            std::make_shared<const HealthyHostVector>(*hosts), hosts_per_locality),
+                        locality_weights, {}, {});
   EXPECT_FALSE(host_set_.chooseHealthyLocality().has_value());
 }
 
@@ -2262,9 +2270,10 @@ TEST_F(HostSetImplLocalityTest, Unweighted) {
       makeHostsPerLocality({{hosts_[0]}, {hosts_[1]}, {hosts_[2]}});
   LocalityWeightsConstSharedPtr locality_weights{new LocalityWeights{1, 1, 1}};
   auto hosts = makeHostsFromHostsPerLocality(hosts_per_locality);
-  host_set_.updateHosts(
-      HostSetImpl::updateHostsParams(hosts, hosts_per_locality, std::make_shared<const HealthyHostVector>(*hosts), hosts_per_locality),
-      locality_weights, {}, {}, absl::nullopt);
+  host_set_.updateHosts(HostSetImpl::updateHostsParams(
+                            hosts, hosts_per_locality,
+                            std::make_shared<const HealthyHostVector>(*hosts), hosts_per_locality),
+                        locality_weights, {}, {}, absl::nullopt);
   EXPECT_EQ(0, host_set_.chooseHealthyLocality().value());
   EXPECT_EQ(1, host_set_.chooseHealthyLocality().value());
   EXPECT_EQ(2, host_set_.chooseHealthyLocality().value());
@@ -2278,9 +2287,10 @@ TEST_F(HostSetImplLocalityTest, Weighted) {
   HostsPerLocalitySharedPtr hosts_per_locality = makeHostsPerLocality({{hosts_[0]}, {hosts_[1]}});
   LocalityWeightsConstSharedPtr locality_weights{new LocalityWeights{1, 2}};
   auto hosts = makeHostsFromHostsPerLocality(hosts_per_locality);
-  host_set_.updateHosts(
-      HostSetImpl::updateHostsParams(hosts, hosts_per_locality, std::make_shared<const HealthyHostVector>(*hosts), hosts_per_locality),
-      locality_weights, {}, {}, absl::nullopt);
+  host_set_.updateHosts(HostSetImpl::updateHostsParams(
+                            hosts, hosts_per_locality,
+                            std::make_shared<const HealthyHostVector>(*hosts), hosts_per_locality),
+                        locality_weights, {}, {}, absl::nullopt);
   EXPECT_EQ(1, host_set_.chooseHealthyLocality().value());
   EXPECT_EQ(0, host_set_.chooseHealthyLocality().value());
   EXPECT_EQ(1, host_set_.chooseHealthyLocality().value());
@@ -2295,9 +2305,10 @@ TEST_F(HostSetImplLocalityTest, MissingWeight) {
       makeHostsPerLocality({{hosts_[0]}, {hosts_[1]}, {hosts_[2]}});
   LocalityWeightsConstSharedPtr locality_weights{new LocalityWeights{1, 0, 1}};
   auto hosts = makeHostsFromHostsPerLocality(hosts_per_locality);
-  host_set_.updateHosts(
-      HostSetImpl::updateHostsParams(hosts, hosts_per_locality, std::make_shared<const HealthyHostVector>(*hosts), hosts_per_locality),
-      locality_weights, {}, {}, absl::nullopt);
+  host_set_.updateHosts(HostSetImpl::updateHostsParams(
+                            hosts, hosts_per_locality,
+                            std::make_shared<const HealthyHostVector>(*hosts), hosts_per_locality),
+                        locality_weights, {}, {}, absl::nullopt);
   EXPECT_EQ(0, host_set_.chooseHealthyLocality().value());
   EXPECT_EQ(2, host_set_.chooseHealthyLocality().value());
   EXPECT_EQ(0, host_set_.chooseHealthyLocality().value());
@@ -2321,9 +2332,10 @@ TEST_F(HostSetImplLocalityTest, UnhealthyFailover) {
 
     auto hosts = makeHostsFromHostsPerLocality(hosts_per_locality);
     host_set_.updateHosts(
-        HostSetImpl::updateHostsParams(hosts, hosts_per_locality,
-                                       makeHostsFromHostsPerLocality<HealthyHostVector>(healthy_hosts_per_locality),
-                                       healthy_hosts_per_locality),
+        HostSetImpl::updateHostsParams(
+            hosts, hosts_per_locality,
+            makeHostsFromHostsPerLocality<HealthyHostVector>(healthy_hosts_per_locality),
+            healthy_hosts_per_locality),
         locality_weights, {}, {}, absl::nullopt);
   };
 
@@ -2367,7 +2379,8 @@ TEST(OverProvisioningFactorTest, LocalityPickChanges) {
     // Healthy ratio: (1/2, 1).
     HostsPerLocalitySharedPtr healthy_hosts_per_locality =
         makeHostsPerLocality({{hosts[0]}, {hosts[2]}});
-    auto healthy_hosts = makeHostsFromHostsPerLocality<HealthyHostVector>(healthy_hosts_per_locality);
+    auto healthy_hosts =
+        makeHostsFromHostsPerLocality<HealthyHostVector>(healthy_hosts_per_locality);
     host_set.updateHosts(HostSetImpl::updateHostsParams(std::make_shared<const HostVector>(hosts),
                                                         hosts_per_locality, healthy_hosts,
                                                         healthy_hosts_per_locality),

From 8f285c56cb585ee105b6291a4165378ce7e79317 Mon Sep 17 00:00:00 2001
From: Snow Pettersen <snowp@squareup.com>
Date: Mon, 15 Apr 2019 11:23:25 -0400
Subject: [PATCH 8/8] format fix

Signed-off-by: Snow Pettersen <snowp@squareup.com>
---
 include/envoy/upstream/BUILD | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/envoy/upstream/BUILD b/include/envoy/upstream/BUILD
index 2e0a177ca0a51..e20c85e1d24ec 100644
--- a/include/envoy/upstream/BUILD
+++ b/include/envoy/upstream/BUILD
@@ -138,7 +138,6 @@ envoy_cc_library(
         ":locality_lib",
         ":resource_manager_interface",
         "//include/envoy/common:callback",
-        "//include/envoy/upstream:types_interface",
         "//include/envoy/config:typed_metadata_interface",
         "//include/envoy/http:codec_interface",
         "//include/envoy/network:connection_interface",
@@ -146,6 +145,7 @@ envoy_cc_library(
         "//include/envoy/runtime:runtime_interface",
         "//include/envoy/ssl:context_interface",
         "//include/envoy/ssl:context_manager_interface",
+        "//include/envoy/upstream:types_interface",
     ],
 )