Skip to content

Commit

Permalink
[yugabyte#11989] xCluster: Cleanup tserver_addrs from ProducerEntryPB
Browse files Browse the repository at this point in the history
Summary:
Issue: yugabyte#11989
Field was originally created for consumer to connect to producer tservers. Since currently proxies are setup directly through client, these tserver_addrs are never used.
As a result, deprecate this field.

Test Plan:
Make sure code compiles. Also make sure replication can be set up as usual:
```
./yb_build.sh --cxx-test integration-tests_twodc-test --gtest_filter "*SetupUniverseReplication/*" -n 4
```

Reviewers: rahuldesirazu, nicolas, jhe

Reviewed By: jhe

Subscribers: ybase, bogdan

Differential Revision: https://phabricator.dev.yugabyte.com/D18100
  • Loading branch information
LambdaWL committed Jul 6, 2022
1 parent da07ff6 commit f850ec8
Show file tree
Hide file tree
Showing 4 changed files with 2 additions and 35 deletions.
8 changes: 1 addition & 7 deletions ent/src/yb/master/catalog_manager_ent.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4659,7 +4659,6 @@ Status CatalogManager::InitCDCConsumer(
const std::string& producer_universe_uuid,
std::shared_ptr<CDCRpcTasks> cdc_rpc_tasks) {

std::unordered_set<HostPort, HostPortHash> tserver_addrs;
// Get the tablets in the consumer table.
cdc::ProducerEntryPB producer_entry;
for (const auto& stream_info : consumer_info) {
Expand All @@ -4669,7 +4668,7 @@ Status CatalogManager::InitCDCConsumer(
// Get producer tablets and map them to the consumer tablets
RETURN_NOT_OK(InitCDCStream(
stream_info.producer_table_id, stream_info.consumer_table_id, consumer_tablet_keys,
&tserver_addrs, &stream_entry, cdc_rpc_tasks));
&stream_entry, cdc_rpc_tasks));
(*producer_entry.mutable_stream_map())[stream_info.stream_id] = std::move(stream_entry);
}

Expand All @@ -4681,11 +4680,6 @@ Status CatalogManager::InitCDCConsumer(
HostPortToPB(hp, producer_entry.add_master_addrs());
}

producer_entry.mutable_tserver_addrs()->Reserve(narrow_cast<int>(tserver_addrs.size()));
for (const auto& addr : tserver_addrs) {
HostPortToPB(addr, producer_entry.add_tserver_addrs());
}

auto cluster_config = ClusterConfig();
auto l = cluster_config->LockForWrite();
auto producer_map = l.mutable_data()->pb.mutable_consumer_registry()->mutable_producer_map();
Expand Down
26 changes: 0 additions & 26 deletions ent/src/yb/master/cdc_consumer_registry_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -74,25 +74,6 @@ bool GetProducerTabletKeys(
return true;
}

void PopulateTsAddresses(
const TabletLocationsPB& producer, std::unordered_set<HostPort, HostPortHash>* tserver_addrs) {
// For external CDC Consumers, populate the list of TServers they can connect to as proxies.
for (const auto& replica : producer.replicas()) {
// Use the public IP addresses since we're cross-universe
for (const auto& addr : replica.ts_info().broadcast_addresses()) {
tserver_addrs->insert(HostPortFromPB(addr));
}
// Rarely a viable setup for production replication, but used in testing...
if (replica.ts_info().broadcast_addresses_size() == 0) {
LOG(WARNING) << "No public broadcast addresses found for "
<< replica.ts_info().permanent_uuid() << ". Using private addresses instead.";
for (const auto& addr : replica.ts_info().private_rpc_addresses()) {
tserver_addrs->insert(HostPortFromPB(addr));
}
}
}
}

// We can optimize if we have the same tablet count in the producer and consumer table by
// mapping key ranges to each other. Due to tablet splitting it may be possible that the keys dont
// match even when the count of range is the same. Return true if a mapping was possible.
Expand Down Expand Up @@ -181,7 +162,6 @@ Status InitCDCStream(
const std::string& producer_table_id,
const std::string& consumer_table_id,
const std::map<std::string, KeyRange>& consumer_tablet_keys,
std::unordered_set<HostPort, HostPortHash>* tserver_addrs,
cdc::StreamEntryPB* stream_entry,
std::shared_ptr<CDCRpcTasks>
cdc_rpc_tasks) {
Expand All @@ -199,12 +179,6 @@ Status InitCDCStream(
"For producer table id $0 and consumer table id $1, same num tablets: $2", producer_table_id,
consumer_table_id, stream_entry->local_tserver_optimized());

// Create the mapping between consumer and producer tablets.
for (int i = 0; i < producer_table_locations.size(); i++) {
const auto& producer = producer_table_locations.Get(i);
PopulateTsAddresses(producer, tserver_addrs);
}

return Status::OK();
}

Expand Down
1 change: 0 additions & 1 deletion ent/src/yb/master/cdc_consumer_registry_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ Status InitCDCStream(
const std::string& producer_table_id,
const std::string& consumer_table_id,
const std::map<std::string, KeyRange>& consumer_tablet_keys,
std::unordered_set<HostPort, HostPortHash>* tserver_addrs,
cdc::StreamEntryPB* stream_entry,
std::shared_ptr<CDCRpcTasks> cdc_rpc_tasks);

Expand Down
2 changes: 1 addition & 1 deletion src/yb/cdc/cdc_consumer.proto
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ message ProducerEntryPB {
// Map from stream id to metadata for that stream.
map<string, StreamEntryPB> stream_map = 1;
repeated HostPortPB master_addrs = 2;
repeated HostPortPB tserver_addrs = 3;
repeated HostPortPB DEPRECATED_tserver_addrs = 3; // See issue #11989.
bool disable_stream = 4; // [default = false] implicit in proto3
}

Expand Down

0 comments on commit f850ec8

Please sign in to comment.