solve review comments: mark const; add comments; template abbreviation

fdbclient/include/fdbclient/NativeAPI.actor.h

@@ -602,7 +602,8 @@ ACTOR Future<Optional<StorageMetrics>> waitStorageMetricsWithLocation(TenantInfo
                                                                       StorageMetrics permittedError);
 
 // Return the suggested split points from storage server.The locations tell which interface should
-// serve the request. The
+// serve the request. `limit` is the current estimated storage metrics of `keys`.The returned points, if present,
+// guarantee the metrics of split result is within limit.
 ACTOR Future<Optional<Standalone<VectorRef<KeyRef>>>> splitStorageMetricsWithLocations(
     std::vector<KeyRangeLocationInfo> locations,
     KeyRange keys,

fdbserver/DataDistribution.actor.cpp

@@ -53,6 +53,20 @@
 #include "fdbserver/DDSharedContext.h"
 #include "flow/actorcompiler.h" // This must be the last #include.
 
+ShardSizeBounds ShardSizeBounds::shardSizeBoundsBeforeTrack() {
+	return ShardSizeBounds{
+		.max = StorageMetrics{ .bytes = -1,
+		                       .bytesPerKSecond = StorageMetrics::infinity,
+		                       .iosPerKSecond = StorageMetrics::infinity,
+		                       .bytesReadPerKSecond = StorageMetrics::infinity },
+		.min = StorageMetrics{ .bytes = -1, .bytesPerKSecond = 0, .iosPerKSecond = 0, .bytesReadPerKSecond = 0 },
+		.permittedError = StorageMetrics{ .bytes = -1,
+		                                  .bytesPerKSecond = StorageMetrics::infinity,
+		                                  .iosPerKSecond = StorageMetrics::infinity,
+		                                  .bytesReadPerKSecond = StorageMetrics::infinity }
+	};
+}
+
 struct DDAudit {
 	DDAudit(UID id, KeyRange range, AuditType type)
 	  : id(id), range(range), type(type), auditMap(AuditPhase::Invalid, allKeys.end), actors(true) {}

fdbserver/include/fdbserver/DataDistribution.actor.h

@@ -477,19 +477,7 @@ struct ShardSizeBounds {
 		return max == rhs.max && min == rhs.min && permittedError == rhs.permittedError;
 	}
 
-	static ShardSizeBounds shardSizeBoundsBeforeTrack() {
+	static ShardSizeBounds shardSizeBoundsBeforeTrack();
-		return ShardSizeBounds{
-			.max = StorageMetrics{ .bytes = -1,
-			                       .bytesPerKSecond = StorageMetrics::infinity,
-			                       .iosPerKSecond = StorageMetrics::infinity,
-			                       .bytesReadPerKSecond = StorageMetrics::infinity },
-			.min = StorageMetrics{ .bytes = -1, .bytesPerKSecond = 0, .iosPerKSecond = 0, .bytesReadPerKSecond = 0 },
-			.permittedError = StorageMetrics{ .bytes = -1,
-			                                  .bytesPerKSecond = StorageMetrics::infinity,
-			                                  .iosPerKSecond = StorageMetrics::infinity,
-			                                  .bytesReadPerKSecond = StorageMetrics::infinity }
-		};
-	}
 };
 
 // Gets the permitted size and IO bounds for a shard

fdbserver/include/fdbserver/MockGlobalState.h

@@ -113,11 +113,12 @@ public:
 	void getStorageMetrics(const GetStorageMetricsRequest& req) override;
 
 	template <class Reply>
-	using isLoadBalancedReply = std::is_base_of<LoadBalancedReply, Reply>;
+	static constexpr bool isLoadBalancedReply = std::is_base_of_v<LoadBalancedReply, Reply>;
 
 	template <class Reply>
-	typename std::enable_if<isLoadBalancedReply<Reply>::value, void>::type
+	typename std::enable_if_t<isLoadBalancedReply<Reply>, void> sendErrorWithPenalty(const ReplyPromise<Reply>& promise,
-	sendErrorWithPenalty(const ReplyPromise<Reply>& promise, const Error& err, double penalty) {
+	                                                                                 const Error& err,
+	                                                                                 double penalty) {
 		Reply reply;
 		reply.error = err;
 		reply.penalty = penalty;
@@ -125,7 +126,7 @@ public:
 	}
 
 	template <class Reply>
-	typename std::enable_if<!isLoadBalancedReply<Reply>::value, void>::type
+	typename std::enable_if_t<!isLoadBalancedReply<Reply>, void>
 	sendErrorWithPenalty(const ReplyPromise<Reply>& promise, const Error& err, double) {
 		promise.sendError(err);
 	}

fdbserver/include/fdbserver/ShardsAffectedByTeamFailure.h

@@ -80,8 +80,8 @@ public:
 	bool hasShards(Team team) const;
 
 	// The first element of the pair is either the source for non-moving shards or the destination team for in-flight
-	// shards The second element of the pair is all previous sources for in-flight shards. This function only return the
+	// shards. The second element of the pair is all previous sources for in-flight shards. This function only returns
-	// teams for the first shard in [keys.begin, keys.end)
+	// the teams for the first shard in [keys.begin, keys.end)
 	std::pair<std::vector<Team>, std::vector<Team>> getTeamsForFirstShard(KeyRangeRef keys);
 
 	std::pair<std::vector<Team>, std::vector<Team>> getTeamsFor(KeyRef key);

fdbserver/include/fdbserver/StorageMetrics.actor.h

@@ -163,9 +163,9 @@ public:
 	StorageServerMetrics metrics;
 
 	// penalty used by loadBalance() to balance requests among service instances
-	virtual double getPenalty() { return 1; }
+	virtual double getPenalty() const { return 1; }
 
-	virtual bool isReadable(KeyRangeRef const& keys) { return true; }
+	virtual bool isReadable(KeyRangeRef const& keys) const { return true; }
 
 	virtual void addActor(Future<Void> future) = 0;
 

fdbserver/storageserver.actor.cpp

@@ -807,8 +807,8 @@ public:
 	VersionedData const& data() const { return versionedData; }
 	VersionedData& mutableData() { return versionedData; }
 
-	double old_rate = 1.0;
+	mutable double old_rate = 1.0;
-	double currentRate() {
+	double currentRate() const {
 		auto versionLag = version.get() - durableVersion.get();
 		double res;
 		if (versionLag >= SERVER_KNOBS->STORAGE_DURABILITY_LAG_HARD_MAX) {
@@ -1379,7 +1379,7 @@ public:
 	// This is the maximum version that might be read from storage (the minimum version is durableVersion)
 	Version storageVersion() const { return oldestVersion.get(); }
 
-	bool isReadable(KeyRangeRef const& keys) override {
+	bool isReadable(KeyRangeRef const& keys) const override {
 		auto sh = shards.intersectingRanges(keys);
 		for (auto i = sh.begin(); i != sh.end(); ++i)
 			if (!i->value()->isReadable())
@@ -1405,10 +1405,10 @@ public:
 		}
 	}
 
-	Counter::Value queueSize() { return counters.bytesInput.getValue() - counters.bytesDurable.getValue(); }
+	Counter::Value queueSize() const { return counters.bytesInput.getValue() - counters.bytesDurable.getValue(); }
 
 	// penalty used by loadBalance() to balance requests among SSes. We prefer SS with less write queue size.
-	double getPenalty() override {
+	double getPenalty() const override {
 		return std::max(std::max(1.0,
 		                         (queueSize() - (SERVER_KNOBS->TARGET_BYTES_PER_STORAGE_SERVER -
 		                                         2.0 * SERVER_KNOBS->SPRING_BYTES_STORAGE_SERVER)) /