From 8e8159ab660f6bca5b0e8c21182b23ba55bac8fd Mon Sep 17 00:00:00 2001 From: Zhe Wang Date: Wed, 15 Jan 2025 13:38:29 -0800 Subject: [PATCH 1/3] improve-auditLocationMetadataPostCheck-coverage --- fdbserver/MoveKeys.actor.cpp | 46 +++++++++++++++++++++--------------- 1 file changed, 27 insertions(+), 19 deletions(-) diff --git a/fdbserver/MoveKeys.actor.cpp b/fdbserver/MoveKeys.actor.cpp index 2d778e4be01..f07f1052121 100644 --- a/fdbserver/MoveKeys.actor.cpp +++ b/fdbserver/MoveKeys.actor.cpp @@ -743,7 +743,10 @@ ACTOR Future cleanUpSingleShardDataMove(Database occ, // Post validate consistency of update of keyServers and serverKeys if (SERVER_KNOBS->AUDIT_DATAMOVE_POST_CHECK) { - wait(auditLocationMetadataPostCheck(occ, keys, "cleanUpSingleShardDataMove_postcheck", dataMoveId)); + ASSERT_WE_THINK(!keys.empty()); + if (!keys.empty()) { + wait(auditLocationMetadataPostCheck(occ, keys, "cleanUpSingleShardDataMove_postcheck", dataMoveId)); + } } break; } catch (Error& e) { @@ -1927,15 +1930,15 @@ ACTOR static Future startMoveShards(Database occ, .detail("DataMove", dataMove.toString()) .detail("BulkLoadTaskState", bulkLoadTaskState.present() ? bulkLoadTaskState.get().toString() : ""); - // Post validate consistency of update of keyServers and serverKeys - if (SERVER_KNOBS->AUDIT_DATAMOVE_POST_CHECK) { - if (!currentKeys.empty()) { - wait(auditLocationMetadataPostCheck(occ, currentKeys, "startMoveShards_postcheck", dataMoveId)); - } - } - dataMove = DataMoveMetaData(); if (currentKeys.end == keys.end) { + // Post validate consistency of update of keyServers and serverKeys + if (SERVER_KNOBS->AUDIT_DATAMOVE_POST_CHECK) { + ASSERT_WE_THINK(!keys.empty()); + if (!keys.empty()) { + wait(auditLocationMetadataPostCheck(occ, keys, "startMoveShards_postcheck", dataMoveId)); + } + } break; } } catch (Error& e) { @@ -2355,13 +2358,15 @@ ACTOR static Future finishMoveShards(Database occ, wait(tr.commit()); - // Post validate consistency of update of keyServers and serverKeys - if (SERVER_KNOBS->AUDIT_DATAMOVE_POST_CHECK) { - wait(auditLocationMetadataPostCheck( - occ, range, "finishMoveShards_postcheck", relocationIntervalId)); - } - if (range.end == dataMove.ranges.front().end) { + // Post validate consistency of update of keyServers and serverKeys + if (SERVER_KNOBS->AUDIT_DATAMOVE_POST_CHECK) { + ASSERT_WE_THINK(!dataMove.ranges.front().empty()); + if (!dataMove.ranges.front().empty()) { + wait(auditLocationMetadataPostCheck( + occ, dataMove.ranges.front(), "finishMoveShards_postcheck", relocationIntervalId)); + } + } break; } } else { @@ -3151,12 +3156,15 @@ ACTOR Future cleanUpDataMoveCore(Database occ, .detail("DataMoveID", dataMoveId) .detail("Range", range); - // Post validate consistency of update of keyServers and serverKeys - if (SERVER_KNOBS->AUDIT_DATAMOVE_POST_CHECK) { - wait(auditLocationMetadataPostCheck(occ, range, "cleanUpDataMoveCore_postcheck", dataMoveId)); - } - if (range.end == dataMove.ranges.front().end) { + // Post validate consistency of update of keyServers and serverKeys + if (SERVER_KNOBS->AUDIT_DATAMOVE_POST_CHECK) { + ASSERT_WE_THINK(!dataMove.ranges.front().empty()); + if (!dataMove.ranges.front().empty()) { + wait(auditLocationMetadataPostCheck( + occ, dataMove.ranges.front(), "cleanUpDataMoveCore_postcheck", dataMoveId)); + } + } break; } } catch (Error& e) { From d52f8a0e7a432aafef60a395ab819c107c8d6ae8 Mon Sep 17 00:00:00 2001 From: Zhe Wang Date: Fri, 17 Jan 2025 13:18:16 -0800 Subject: [PATCH 2/3] address comments --- fdbserver/MoveKeys.actor.cpp | 31 ++++++++++++------------------- 1 file changed, 12 insertions(+), 19 deletions(-) diff --git a/fdbserver/MoveKeys.actor.cpp b/fdbserver/MoveKeys.actor.cpp index f07f1052121..2fe17ce640b 100644 --- a/fdbserver/MoveKeys.actor.cpp +++ b/fdbserver/MoveKeys.actor.cpp @@ -23,6 +23,7 @@ #include "fdbclient/BlobRestoreCommon.h" #include "fdbclient/FDBOptions.g.h" +#include "flow/Buggify.h" #include "flow/Error.h" #include "flow/Trace.h" #include "flow/Util.h" @@ -549,7 +550,11 @@ ACTOR Future auditLocationMetadataPreCheck(Database occ, ACTOR Future auditLocationMetadataPostCheck(Database occ, KeyRange range, std::string context, UID dataMoveId) { if (range.empty()) { - TraceEvent(SevWarn, "CheckLocationMetadataEmptyInputRange").detail("By", "PostCheck").detail("Range", range); + TraceEvent(g_network->isSimulated() ? SevError : SevWarnAlways, "CheckLocationMetadataEmptyInputRange") + .detail("By", "PostCheck") + .detail("Range", range) + .detail("Context", context) + .detail("DataMoveId", dataMoveId.toString()); return Void(); } state std::vector> actors; @@ -743,10 +748,7 @@ ACTOR Future cleanUpSingleShardDataMove(Database occ, // Post validate consistency of update of keyServers and serverKeys if (SERVER_KNOBS->AUDIT_DATAMOVE_POST_CHECK) { - ASSERT_WE_THINK(!keys.empty()); - if (!keys.empty()) { - wait(auditLocationMetadataPostCheck(occ, keys, "cleanUpSingleShardDataMove_postcheck", dataMoveId)); - } + wait(auditLocationMetadataPostCheck(occ, keys, "cleanUpSingleShardDataMove_postcheck", dataMoveId)); } break; } catch (Error& e) { @@ -1934,10 +1936,7 @@ ACTOR static Future startMoveShards(Database occ, if (currentKeys.end == keys.end) { // Post validate consistency of update of keyServers and serverKeys if (SERVER_KNOBS->AUDIT_DATAMOVE_POST_CHECK) { - ASSERT_WE_THINK(!keys.empty()); - if (!keys.empty()) { - wait(auditLocationMetadataPostCheck(occ, keys, "startMoveShards_postcheck", dataMoveId)); - } + wait(auditLocationMetadataPostCheck(occ, keys, "startMoveShards_postcheck", dataMoveId)); } break; } @@ -2361,11 +2360,8 @@ ACTOR static Future finishMoveShards(Database occ, if (range.end == dataMove.ranges.front().end) { // Post validate consistency of update of keyServers and serverKeys if (SERVER_KNOBS->AUDIT_DATAMOVE_POST_CHECK) { - ASSERT_WE_THINK(!dataMove.ranges.front().empty()); - if (!dataMove.ranges.front().empty()) { - wait(auditLocationMetadataPostCheck( - occ, dataMove.ranges.front(), "finishMoveShards_postcheck", relocationIntervalId)); - } + wait(auditLocationMetadataPostCheck( + occ, dataMove.ranges.front(), "finishMoveShards_postcheck", relocationIntervalId)); } break; } @@ -3159,11 +3155,8 @@ ACTOR Future cleanUpDataMoveCore(Database occ, if (range.end == dataMove.ranges.front().end) { // Post validate consistency of update of keyServers and serverKeys if (SERVER_KNOBS->AUDIT_DATAMOVE_POST_CHECK) { - ASSERT_WE_THINK(!dataMove.ranges.front().empty()); - if (!dataMove.ranges.front().empty()) { - wait(auditLocationMetadataPostCheck( - occ, dataMove.ranges.front(), "cleanUpDataMoveCore_postcheck", dataMoveId)); - } + wait(auditLocationMetadataPostCheck( + occ, dataMove.ranges.front(), "cleanUpDataMoveCore_postcheck", dataMoveId)); } break; } From 6b7ac54cfb50497d12ab92ff37af508dfa053130 Mon Sep 17 00:00:00 2001 From: Zhe Wang Date: Fri, 17 Jan 2025 13:20:51 -0800 Subject: [PATCH 3/3] nit --- fdbserver/MoveKeys.actor.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/fdbserver/MoveKeys.actor.cpp b/fdbserver/MoveKeys.actor.cpp index 2fe17ce640b..f92756ba450 100644 --- a/fdbserver/MoveKeys.actor.cpp +++ b/fdbserver/MoveKeys.actor.cpp @@ -23,7 +23,6 @@ #include "fdbclient/BlobRestoreCommon.h" #include "fdbclient/FDBOptions.g.h" -#include "flow/Buggify.h" #include "flow/Error.h" #include "flow/Trace.h" #include "flow/Util.h"