From cd1335696440b752b4efcca66da0f35b8f3f0e81 Mon Sep 17 00:00:00 2001
From: Xiaoxi Wang <xiaoxi.wang@snowflake.com>
Date: Wed, 20 Jul 2022 16:09:38 -0700
Subject: [PATCH] solve review comments

---
 fdbserver/DataDistributionQueue.actor.cpp            | 9 ++-------
 fdbserver/DataDistributionTracker.actor.cpp          | 8 +++++---
 fdbserver/include/fdbserver/DataDistribution.actor.h | 4 +++-
 3 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/fdbserver/DataDistributionQueue.actor.cpp b/fdbserver/DataDistributionQueue.actor.cpp
index a135b83dca..8830523e12 100644
--- a/fdbserver/DataDistributionQueue.actor.cpp
+++ b/fdbserver/DataDistributionQueue.actor.cpp
@@ -1795,17 +1795,12 @@ ACTOR Future<bool> rebalanceReadLoad(DDQueueData* self,
 	deterministicRandom()->randomShuffle(metricsList);
 	traceEvent->detail("MinReadLoad", reply.minReadLoad).detail("MaxReadLoad", reply.maxReadLoad);
 
-	int chosenIdx = -1;
-	for (int i = 0; i < metricsList.size(); ++i) {
-		chosenIdx = i;
-		break;
-	}
-	if (chosenIdx == -1) {
+	if (metricsList.empty()) {
 		traceEvent->detail("SkipReason", "NoEligibleShards");
 		return false;
 	}
 
-	auto& [shard, metrics] = metricsList[chosenIdx];
+	auto& [shard, metrics] = metricsList[0];
 	traceEvent->detail("ShardReadBandwidth", metrics.bytesReadPerKSecond);
 	//  Verify the shard is still in ShardsAffectedByTeamFailure
 	shards = self->shardsAffectedByTeamFailure->getShardsFor(
diff --git a/fdbserver/DataDistributionTracker.actor.cpp b/fdbserver/DataDistributionTracker.actor.cpp
index fd12198c57..3c235cc438 100644
--- a/fdbserver/DataDistributionTracker.actor.cpp
+++ b/fdbserver/DataDistributionTracker.actor.cpp
@@ -866,7 +866,7 @@ ACTOR Future<Void> fetchTopKShardMetrics_impl(DataDistributionTracker* self, Get
 					maxReadLoad = std::max(metrics.bytesReadPerKSecond, maxReadLoad);
 					if (req.minBytesReadPerKSecond <= metrics.bytesReadPerKSecond &&
 					    metrics.bytesReadPerKSecond <= req.maxBytesReadPerKSecond) {
-						returnMetrics.push_back({ range, metrics });
+						returnMetrics.emplace_back(range, metrics);
 					}
 				}
 
@@ -877,8 +877,10 @@ ACTOR Future<Void> fetchTopKShardMetrics_impl(DataDistributionTracker* self, Get
 				if (req.topK >= returnMetrics.size())
 					req.reply.send(GetTopKMetricsReply(returnMetrics, minReadLoad, maxReadLoad));
 				else {
-					std::nth_element(
-					    returnMetrics.begin(), returnMetrics.begin() + req.topK - 1, returnMetrics.end(), req.compare);
+					std::nth_element(returnMetrics.begin(),
+					                 returnMetrics.begin() + req.topK - 1,
+					                 returnMetrics.end(),
+					                 GetTopKMetricsRequest::compare);
 					req.reply.send(GetTopKMetricsReply(std::vector<GetTopKMetricsReply::KeyRangeStorageMetrics>(
 					                                       returnMetrics.begin(), returnMetrics.begin() + req.topK),
 					                                   minReadLoad,
diff --git a/fdbserver/include/fdbserver/DataDistribution.actor.h b/fdbserver/include/fdbserver/DataDistribution.actor.h
index ad625692d1..8b323c6205 100644
--- a/fdbserver/include/fdbserver/DataDistribution.actor.h
+++ b/fdbserver/include/fdbserver/DataDistribution.actor.h
@@ -231,6 +231,8 @@ struct GetTopKMetricsReply {
 	struct KeyRangeStorageMetrics {
 		KeyRange range;
 		StorageMetrics metrics;
+		KeyRangeStorageMetrics() = default;
+		KeyRangeStorageMetrics(const KeyRange& range, const StorageMetrics& s) : range(range), metrics(s) {}
 	};
 	std::vector<KeyRangeStorageMetrics> shardMetrics;
 	double minReadLoad = -1, maxReadLoad = -1;
@@ -239,7 +241,7 @@ struct GetTopKMetricsReply {
 	  : shardMetrics(m), minReadLoad(minReadLoad), maxReadLoad(maxReadLoad) {}
 };
 struct GetTopKMetricsRequest {
-	int topK = 1; // default only return the top 1 shard based on the comparator
+	int topK = 1; // default only return the top 1 shard based on the GetTopKMetricsRequest::compare function
 	std::vector<KeyRange> keys;
 	Promise<GetTopKMetricsReply> reply; // topK storage metrics
 	double maxBytesReadPerKSecond = 0, minBytesReadPerKSecond = 0; // all returned shards won't exceed this read load