Skip to content

Commit

Permalink
[xDS] avoid creating duplicate hierarchical path attribute values (gr…
Browse files Browse the repository at this point in the history
  • Loading branch information
markdroth authored Aug 21, 2023
1 parent 1e818c9 commit c0e2fa6
Show file tree
Hide file tree
Showing 26 changed files with 319 additions and 143 deletions.
1 change: 1 addition & 0 deletions BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -1585,6 +1585,7 @@ grpc_cc_library(
"//src/core:promise_status",
"//src/core:race",
"//src/core:ref_counted",
"//src/core:ref_counted_string",
"//src/core:resolved_address",
"//src/core:resource_quota",
"//src/core:resource_quota_trace",
Expand Down
5 changes: 5 additions & 0 deletions CMakeLists.txt

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions Makefile

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions Package.swift

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions build_autogenerated.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions config.m4

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions config.w32

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions gRPC-C++.podspec

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions gRPC-Core.podspec

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions grpc.gemspec

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions grpc.gyp

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions package.xml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

20 changes: 20 additions & 0 deletions src/core/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -960,6 +960,23 @@ grpc_cc_library(
],
)

grpc_cc_library(
name = "ref_counted_string",
srcs = [
"lib/gprpp/ref_counted_string.cc",
],
hdrs = [
"lib/gprpp/ref_counted_string.h",
],
external_deps = ["absl/strings"],
language = "c++",
deps = [
"ref_counted",
"//:gpr",
"//:ref_counted_ptr",
],
)

grpc_cc_library(
name = "uuid_v4",
srcs = ["lib/gprpp/uuid_v4.cc"],
Expand Down Expand Up @@ -2680,6 +2697,7 @@ grpc_cc_library(
"channel_stack_type",
"dual_ref_counted",
"ref_counted",
"ref_counted_string",
"time",
"useful",
"//:channel_arg_names",
Expand Down Expand Up @@ -4499,6 +4517,7 @@ grpc_cc_library(
"lb_policy_factory",
"lb_policy_registry",
"pollset_set",
"ref_counted_string",
"validation_errors",
"//:channel_arg_names",
"//:config",
Expand Down Expand Up @@ -4653,6 +4672,7 @@ grpc_cc_library(
deps = [
"channel_args",
"ref_counted",
"ref_counted_string",
"//:gpr_platform",
"//:ref_counted_ptr",
"//:server_address",
Expand Down
15 changes: 10 additions & 5 deletions src/core/ext/filters/client_channel/lb_policy/address_filtering.cc
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ int HierarchicalPathArg::ChannelArgsCompare(const HierarchicalPathArg* a,
const HierarchicalPathArg* b) {
for (size_t i = 0; i < a->path_.size(); ++i) {
if (b->path_.size() == i) return 1;
int r = a->path_[i].compare(b->path_[i]);
int r = a->path_[i].as_string_view().compare(b->path_[i].as_string_view());
if (r != 0) return r;
}
if (b->path_.size() > a->path_.size()) return -1;
Expand All @@ -47,19 +47,24 @@ absl::StatusOr<HierarchicalAddressMap> MakeHierarchicalAddressMap(
const absl::StatusOr<ServerAddressList>& addresses) {
if (!addresses.ok()) return addresses.status();
HierarchicalAddressMap result;
RefCountedPtr<HierarchicalPathArg> remaining_path_attr;
for (const ServerAddress& address : *addresses) {
const auto* path_arg = address.args().GetObject<HierarchicalPathArg>();
if (path_arg == nullptr) continue;
const std::vector<std::string>& path = path_arg->path();
const std::vector<RefCountedStringValue>& path = path_arg->path();
auto it = path.begin();
if (it == path.end()) continue;
ServerAddressList& target_list = result[*it];
ChannelArgs args = address.args();
++it;
if (it != path.end()) {
std::vector<std::string> remaining_path(it, path.end());
args = args.SetObject(
MakeRefCounted<HierarchicalPathArg>(std::move(remaining_path)));
std::vector<RefCountedStringValue> remaining_path(it, path.end());
if (remaining_path_attr == nullptr ||
remaining_path_attr->path() != remaining_path) {
remaining_path_attr =
MakeRefCounted<HierarchicalPathArg>(std::move(remaining_path));
}
args = args.SetObject(remaining_path_attr);
}
target_list.emplace_back(address.address(), args);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,14 @@
#include <grpc/support/port_platform.h>

#include <map>
#include <string>
#include <utility>
#include <vector>

#include "absl/status/statusor.h"
#include "absl/strings/string_view.h"

#include "src/core/lib/gprpp/ref_counted.h"
#include "src/core/lib/gprpp/ref_counted_string.h"
#include "src/core/lib/resolver/server_address.h"

// The resolver returns a flat list of addresses. When a hierarchy of
Expand Down Expand Up @@ -88,23 +88,25 @@ namespace grpc_core {
// to be associated with the address.
class HierarchicalPathArg : public RefCounted<HierarchicalPathArg> {
public:
explicit HierarchicalPathArg(std::vector<std::string> path)
explicit HierarchicalPathArg(std::vector<RefCountedStringValue> path)
: path_(std::move(path)) {}

// Channel arg traits methods.
static absl::string_view ChannelArgName();
static int ChannelArgsCompare(const HierarchicalPathArg* a,
const HierarchicalPathArg* b);

const std::vector<std::string>& path() const { return path_; }
const std::vector<RefCountedStringValue>& path() const { return path_; }

private:
std::vector<std::string> path_;
std::vector<RefCountedStringValue> path_;
};

// A map from the next path element to the addresses that fall under
// that path element.
using HierarchicalAddressMap = std::map<std::string, ServerAddressList>;
using HierarchicalAddressMap =
std::map<RefCountedStringValue, ServerAddressList,
RefCountedStringValueLessThan>;

// Splits up the addresses into a separate list for each child.
absl::StatusOr<HierarchicalAddressMap> MakeHierarchicalAddressMap(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -681,7 +681,12 @@ absl::Status PriorityLb::ChildPriority::UpdateLocked(
UpdateArgs update_args;
update_args.config = std::move(config);
if (priority_policy_->addresses_.ok()) {
update_args.addresses = (*priority_policy_->addresses_)[name_];
auto it = priority_policy_->addresses_->find(name_);
if (it == priority_policy_->addresses_->end()) {
update_args.addresses.emplace();
} else {
update_args.addresses = it->second;
}
} else {
update_args.addresses = priority_policy_->addresses_.status();
}
Expand Down
Loading

0 comments on commit c0e2fa6

Please sign in to comment.