From 01ce1ab24d117898052521295ea4f43316dc7323 Mon Sep 17 00:00:00 2001 From: Josh Slocum Date: Thu, 15 Jun 2023 08:10:49 -0500 Subject: [PATCH] fix consistency scan ubsan issue and replication factor calculation (#10492) --- fdbserver/ConsistencyScan.actor.cpp | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/fdbserver/ConsistencyScan.actor.cpp b/fdbserver/ConsistencyScan.actor.cpp index b0139b5a34..2a9081a3a5 100644 --- a/fdbserver/ConsistencyScan.actor.cpp +++ b/fdbserver/ConsistencyScan.actor.cpp @@ -652,10 +652,11 @@ ACTOR Future consistencyScanCore(Database db, (statsCurrentRound.logicalBytesScanned == 0) ? 3 : ((double)statsCurrentRound.replicatedBytesRead / statsCurrentRound.logicalBytesScanned); - configuredRate = - std::min(config.maxReadByteRate, - memState->databaseSize.get() * replicationFactor / - (config.targetRoundTimeSeconds > 0 ? config.targetRoundTimeSeconds : 1)); + double bytesPerSecTarget = memState->databaseSize.get() * replicationFactor / + (config.targetRoundTimeSeconds > 0 ? config.targetRoundTimeSeconds : 1); + // avoid ubsan int conversion issue if misconfigured, also max of 100MB/s is reasonable anyway + bytesPerSecTarget = std::min(bytesPerSecTarget, 1e8); + configuredRate = std::min(config.maxReadByteRate, bytesPerSecTarget); } configuredRate = std::max(100e3, configuredRate); @@ -842,6 +843,7 @@ ACTOR Future consistencyScanCore(Database db, state std::vector>> keyValueFutures; state Optional firstValidServer; memState->stats.requests += storageServerInterfaces.size(); + state int64_t replicatedBytesReadThisLoop = 0; int newErrors = wait(consistencyCheckReadData(memState->csId, db, targetRange, @@ -849,7 +851,7 @@ ACTOR Future consistencyScanCore(Database db, &storageServerInterfaces, &keyValueFutures, &firstValidServer, - &replicatedBytesRead, + &replicatedBytesReadThisLoop, statsCurrentRound.startVersion)); errors += newErrors; memState->stats.inconsistencies += newErrors; @@ -870,6 +872,7 @@ ACTOR Future consistencyScanCore(Database db, ASSERT(firstValidServer.present()); GetKeyValuesReply rangeResult = keyValueFutures[firstValidServer.get()].get().get(); logicalBytesRead += rangeResult.data.expectedSize(); + replicatedBytesRead += replicatedBytesReadThisLoop; if (!rangeResult.more) { statsCurrentRound.lastEndKey = targetRange.end; noMoreRecords = statsCurrentRound.lastEndKey == allKeys.end; @@ -901,6 +904,7 @@ ACTOR Future consistencyScanCore(Database db, nextKey = storageNextKey; } } + replicatedBytesRead += replicatedBytesReadThisLoop; statsCurrentRound.lastEndKey = nextKey; if (nextKey == targetRange.end) { noMoreRecords = nextKey == allKeys.end; @@ -920,7 +924,9 @@ ACTOR Future consistencyScanCore(Database db, } // FIXME: increment failed request count if error present ASSERT(failedRequest.get().code() != error_code_operation_cancelled); - totalReadBytesFromStorageServers += 100000; + // don't include replicated bytes this loop in metrics to avoid getting replication + // factor wrong, but make sure we stil throttle based on it + totalReadBytesFromStorageServers += replicatedBytesReadThisLoop + 100000; break; }