[yugabyte#6696] Small master perf tweaks

Summary:
Found several small perf opportunities during a test on an overloaded master, using perf.

> + 4.10% yb::QLRowBlock::Extend
During the `system.partitions` queries, noticed we did not have reserve semantics at the
QLRowBlock level.

```
- 2.98% yb::master::CatalogManager::GetTabletLocations
  - 2.87% yb::master::CatalogManager::BuildLocationsForTablet
      + 1.02% yb::master::TSInformationPB::TSInformationPB
```
This was some older refactor on the TSDescriptor which added the ability to pass the info object
as a shared_ptr, but for some reason, we were dereferencing it in this API...

One stack I forgot to save was for CopyRegistration, which used to do a pass by value, so it
could then use PB::Swap. Turns out that was more expensive than just passing by reference and
setting the 3 mutable fields explicitly, as then we could continue using the arena allocation!

Sometimes, we actually use a lot of CPU just to report the slow query logging, due to constructing
stack traces! Added a gfag to control the time cutoff, so we could disable this in prod, if needed.
Flag and defaults: `master_log_lock_warning_ms=100`

Test Plan: Jenkins

Reviewers: sergei, rahuldesirazu, nicolas

Reviewed By: nicolas

Subscribers: ybase

Differential Revision: https://phabricator.dev.yugabyte.com/D10145

src/yb/common/ql_rowblock.cc

@@ -111,6 +111,9 @@ QLRow& QLRowBlock::Extend() {
  rows_.emplace_back(schema_);
  return rows_.back();
 }
+void QLRowBlock::Reserve(size_t size) {
+ rows_.reserve(size);
+}
 
 Status QLRowBlock::AddRow(const QLRow& row) {
  // TODO: check for schema compatibility between QLRow and QLRowBlock.

src/yb/common/ql_rowblock.h

@@ -100,6 +100,9 @@ class QLRowBlock {
  // Extend row block by 1 emtpy row and return the new row.
  QLRow& Extend();
 
+ // Optimization to reserve memory for up to this many rows.
+ void Reserve(size_t size);
+
  // Add a row to the rowblock.
  CHECKED_STATUS AddRow(const QLRow& row);
 

src/yb/master/catalog_manager.cc

@@ -7949,14 +7949,13 @@ Status CatalogManager::BuildLocationsForTablet(const scoped_refptr<TabletInfo>&
  TabletLocationsPB_ReplicaPB* replica_pb = locs_pb->add_replicas();
  replica_pb->set_role(replica.second.role);
  replica_pb->set_member_type(replica.second.member_type);
- TSInformationPB tsinfo_pb = *replica.second.ts_desc->GetTSInformationPB();
+ auto tsinfo_pb = replica.second.ts_desc->GetTSInformationPB();
 
  TSInfoPB* out_ts_info = replica_pb->mutable_ts_info();
- out_ts_info->set_permanent_uuid(tsinfo_pb.tserver_instance().permanent_uuid());
- TakeRegistration(tsinfo_pb.mutable_registration()->mutable_common(), out_ts_info);
- out_ts_info->set_placement_uuid(tsinfo_pb.registration().common().placement_uuid());
- *out_ts_info->mutable_capabilities() = std::move(
- *tsinfo_pb.mutable_registration()->mutable_capabilities());
+ out_ts_info->set_permanent_uuid(tsinfo_pb->tserver_instance().permanent_uuid());
+ CopyRegistration(tsinfo_pb->registration().common(), out_ts_info);
+ out_ts_info->set_placement_uuid(tsinfo_pb->registration().common().placement_uuid());
+ *out_ts_info->mutable_capabilities() = tsinfo_pb->registration().capabilities();
  }
  return Status::OK();
  }

src/yb/master/master_util.cc

@@ -98,8 +98,10 @@ void TakeRegistration(consensus::RaftPeerPB* source, TSInfoPB* dest) {
  dest->mutable_cloud_info()->Swap(source->mutable_cloud_info());
 }
 
-void CopyRegistration(consensus::RaftPeerPB source, TSInfoPB* dest) {
- TakeRegistration(&source, dest);
+void CopyRegistration(const consensus::RaftPeerPB& source, TSInfoPB* dest) {
+ dest->mutable_private_rpc_addresses()->CopyFrom(source.last_known_private_addr());
+ dest->mutable_broadcast_addresses()->CopyFrom(source.last_known_broadcast_addr());
+ dest->mutable_cloud_info()->CopyFrom(source.cloud_info());
 }
 
 void TakeRegistration(ServerRegistrationPB* source, TSInfoPB* dest) {
@@ -108,8 +110,10 @@ void TakeRegistration(ServerRegistrationPB* source, TSInfoPB* dest) {
  dest->mutable_cloud_info()->Swap(source->mutable_cloud_info());
 }
 
-void CopyRegistration(ServerRegistrationPB source, TSInfoPB* dest) {
- TakeRegistration(&source, dest);
+void CopyRegistration(const ServerRegistrationPB& source, TSInfoPB* dest) {
+ dest->mutable_private_rpc_addresses()->CopyFrom(source.private_rpc_addresses());
+ dest->mutable_broadcast_addresses()->CopyFrom(source.broadcast_addresses());
+ dest->mutable_cloud_info()->CopyFrom(source.cloud_info());
 }
 
 bool IsSystemNamespace(const std::string& namespace_name) {

src/yb/master/master_util.h

@@ -46,10 +46,10 @@ CHECKED_STATUS GetMasterEntryForHosts(
 const HostPortPB& DesiredHostPort(const TSInfoPB& ts_info, const CloudInfoPB& from);
 
 void TakeRegistration(consensus::RaftPeerPB* source, TSInfoPB* dest);
-void CopyRegistration(consensus::RaftPeerPB source, TSInfoPB* dest);
+void CopyRegistration(const consensus::RaftPeerPB& source, TSInfoPB* dest);
 
 void TakeRegistration(ServerRegistrationPB* source, TSInfoPB* dest);
-void CopyRegistration(ServerRegistrationPB source, TSInfoPB* dest);
+void CopyRegistration(const ServerRegistrationPB& source, TSInfoPB* dest);
 
 bool IsSystemNamespace(const std::string& namespace_name);
 

src/yb/master/scoped_leader_shared_lock.cc

@@ -39,6 +39,9 @@
 
 using namespace std::literals;
 
+DEFINE_int32(master_log_lock_warning_ms, 100,
+ "Print warnings if lock is held for longer than this amount of time.");
+
 using yb::consensus::Consensus;
 using yb::consensus::ConsensusStatePB;
 
@@ -109,7 +112,8 @@ void ScopedLeaderSharedLock::Unlock() {
  lock.swap(leader_shared_lock_);
  }
  auto finish = std::chrono::steady_clock::now();
- static const auto kLongLockLimit = RegularBuildVsSanitizers(100ms, 750ms);
+ static const auto kLongLockLimit = RegularBuildVsSanitizers(
+ FLAGS_master_log_lock_warning_ms * 1ms, 750ms);
  if (finish > start_ + kLongLockLimit) {
  LOG(WARNING) << "Long lock of catalog manager: " << yb::ToString(finish - start_) << "\n"
  << GetStackTrace();

src/yb/master/yql_partitions_vtable.cc

@@ -111,7 +111,6 @@ Status YQLPartitionsVTable::GenerateAndCacheData() const {
  // Get tablets for table.
  std::vector<scoped_refptr<TabletInfo>> tablet_infos;
  table->GetAllTablets(&tablet_infos);
- tablets.reserve(tablets.size() + tablet_infos.size());
  for (const auto& info : tablet_infos) {
  tablets.emplace_back();
  auto& data = tablets.back();
@@ -138,6 +137,8 @@ Status YQLPartitionsVTable::GenerateAndCacheData() const {
  dns_results.emplace(p.first, InetAddress(VERIFY_RESULT(p.second.get())));
  }
 
+ // Reserve upfront memory, as we're likely to need to insert a row for each tablet.
+ vtable->Reserve(tablets.size());
  for (const auto& data : tablets) {
  // Skip not-found tablets: they might not be running yet or have been deleted.
  if (data.locations->table_id().empty()) {