Fix a backup worker assertion failure

The pop version obtained from GRV proxy replies may go backwards, which can
cause assertion failure when pulling mutations, i.e., missing mutations. Fix
this bug and add an assertion that popVersion can go lower.

-f ./tests/slow/BackupAndRestore.toml -s 2467687006 -b off

100k 20250312-202457-jzhou-34e803fa47d1c946 backup tests

Author: jzhou77

design/backup_v2_partitioned_logs.md

@@ -168,7 +168,7 @@ From an end-to-end perspective, the new backup system works in the following ste
 4. Periodically, backup workers upload mutations to the requested blob storage, and save the progress into the database;
 5. The backup is restorable when backup workers have saved versions that are larger than the complete snapshot’s end version, and the backup is stopped if a stop on restorable flag is set in the request.
 
-The new backup has four major components: 1) backup workers; 2) recruitment of backup workers; 3) extension of tag partitioned log system to support pseudo tags; 4) integration with existing `TaskBucket` based backup command interface; and 5) integration with the Performant Restore System.
+The new backup has the following major components: 1) backup workers; 2) recruitment of backup workers; 3) extension of tag partitioned log system to support pseudo tags; 4) integration with existing `TaskBucket` based backup command interface; and 5) integration with the existing `TaskBucket` based restore system to read partitioned mutation logs.
 
 ### Backup workers
 

fdbserver/BackupWorker.actor.cpp

@@ -348,6 +348,10 @@ struct BackupData {
 		}
 		ASSERT_WE_THINK(backupEpoch == oldestBackupEpoch);
 		const Tag popTag = logSystem.get()->getPseudoPopTag(tag, ProcessClass::BackupClass);
+		DisabledTraceEvent("BackupWorkerPop", myId)
+		    .detail("Tag", popTag)
+		    .detail("SavedVersion", savedVersion)
+		    .detail("PopVersion", popVersion);
 		logSystem.get()->pop(std::max(popVersion, savedVersion), popTag);
 	}
 
@@ -481,6 +485,7 @@ struct BackupData {
 				// Save the noop pop version, which sets min version for
 				// the next backup job. Note this version may change after the wait.
 				state Version popVersion = self->popTrigger.get();
+				ASSERT(self->popVersion < popVersion);
 				wait(_saveNoopVersion(self, popVersion));
 				self->popVersion = popVersion;
 				TraceEvent("BackupWorkerNoopPop", self->myId)
@@ -1036,6 +1041,8 @@ ACTOR Future<Void> pullAsyncData(BackupData* self) {
 	    .detail("Tag", self->tag)
 	    .detail("Version", tagAt)
 	    .detail("PopVersion", self->popVersion)
+	    .detail("TriggerVersion", self->popTrigger.get())
+	    .detail("StartVersion", self->startVersion)
 	    .detail("SavedVersion", self->savedVersion);
 	loop {
 		while (self->paused.get()) {
@@ -1149,7 +1156,14 @@ ACTOR Future<Void> monitorBackupKeyOrPullData(BackupData* self, bool keyPresent)
 						    std::max({ self->popVersion, self->savedVersion, committedVersion.get() });
 						self->minKnownCommittedVersion =
 						    std::max(committedVersion.get(), self->minKnownCommittedVersion);
-						self->popTrigger.set(newPopVersion);
+						if (newPopVersion < self->popTrigger.get()) {
+							// this can happen if a different GRV proxy replies
+							DisabledTraceEvent("BackupWorkerNoopPop", self->myId)
+							    .detail("Version", newPopVersion)
+							    .detail("OldPop", self->popTrigger.get());
+						} else {
+							self->popTrigger.set(newPopVersion);
+						}
 						committedVersion = Never();
 					} else {
 						committedVersion = self->getMinKnownCommittedVersion();

fdbserver/TagPartitionedLogSystem.actor.cpp

@@ -253,7 +253,7 @@ Version TagPartitionedLogSystem::popPseudoLocalityTag(Tag tag, Version upTo) {
 	for (const int8_t locality : pseudoLocalities) {
 		minVersion = std::min(minVersion, pseudoLocalityPopVersion[Tag(locality, tag.id)]);
 	}
-	// TraceEvent("TLogPopPseudoTag", dbgid).detail("Tag", tag.toString()).detail("Version", upTo).detail("PopVersion", minVersion);
+	// TraceEvent("TLogPopPseudoTag", dbgid).detail("Tag", tag).detail("Version", upTo).detail("PopVersion", minVersion);
 	return minVersion;
 }