abort build upload if we fail to finalize a build part (#623)

* abort build upload if we fail to finalize a build part

2 files changed, 55 insertions, 14 deletions

diff --git a/src/zenremotestore/builds/buildstorageoperations.cpp b/src/zenremotestore/builds/buildstorageoperations.cpp
index 5c1b28695..833a25839 100644
--- a/src/zenremotestore/builds/buildstorageoperations.cpp
+++ b/src/zenremotestore/builds/buildstorageoperations.cpp
@@ -5201,7 +5201,8 @@ BuildsOperationUploadFolder::Execute()
 		IoHash PartHash = PutBuildPartResult.first;
 
 		auto UploadAttachments = [this, &LocalContent, &LocalLookup, &NewBlockChunks, &NewBlocks, &LooseChunkIndexes, &LargeAttachmentSize](
-									 std::span<IoHash> RawHashes) {
+									 std::span<IoHash>	  RawHashes,
+									 std::vector<IoHash>& OutUnknownChunks) {
 			if (!m_AbortFlag)
 			{
 				UploadStatistics	  TempUploadStats;
@@ -5239,11 +5240,14 @@ BuildsOperationUploadFolder::Execute()
 								LooseChunkIndexes,
 								LargeAttachmentSize,
 								TempUploadStats,
-								TempLooseChunksStats);
+								TempLooseChunksStats,
+								OutUnknownChunks);
 				m_UploadStats += TempUploadStats;
 				m_LooseChunksStats += TempLooseChunksStats;
 			}
 		};
+
+		std::vector<IoHash> UnknownChunks;
 		if (m_Options.IgnoreExistingBlocks)
 		{
 			if (m_Options.IsVerbose)
@@ -5269,7 +5273,7 @@ BuildsOperationUploadFolder::Execute()
 					ForceUploadChunkHashes.push_back(NewBlocks.BlockDescriptions[BlockIndex].BlockHash);
 				}
 			}
-			UploadAttachments(ForceUploadChunkHashes);
+			UploadAttachments(ForceUploadChunkHashes, UnknownChunks);
 		}
 		else if (!PutBuildPartResult.second.empty())
 		{
@@ -5277,7 +5281,20 @@ BuildsOperationUploadFolder::Execute()
 			{
 				LOG_OUTPUT(m_LogOutput, "PutBuildPart needs attachments: {}", FormatArray<IoHash>(PutBuildPartResult.second, "\n    "sv));
 			}
-			UploadAttachments(PutBuildPartResult.second);
+			UploadAttachments(PutBuildPartResult.second, UnknownChunks);
+		}
+
+		auto BuildUnkownChunksResponse = [](const std::vector<IoHash>& UnknownChunks, bool WillRetry) {
+			return fmt::format(
+				"The following build blobs was reported as needed for upload but was reported as existing at the start of the "
+				"operation.{}{}",
+				WillRetry ? " Treating this as a transient inconsistency issue and will attempt to retry finalization."sv : ""sv,
+				FormatArray<IoHash>(UnknownChunks, "\n    "sv));
+		};
+
+		if (!UnknownChunks.empty())
+		{
+			ZEN_CONSOLE_WARN("{}", BuildUnkownChunksResponse(UnknownChunks, /*WillRetry*/ true));
 		}
 
 		uint32_t FinalizeBuildPartRetryCount = 5;
@@ -5300,7 +5317,27 @@ BuildsOperationUploadFolder::Execute()
 			{
 				LOG_OUTPUT(m_LogOutput, "FinalizeBuildPart needs attachments: {}", FormatArray<IoHash>(Needs, "\n    "sv));
 			}
-			UploadAttachments(Needs);
+
+			std::vector<IoHash> RetryUnknownChunks;
+			UploadAttachments(Needs, RetryUnknownChunks);
+			if (RetryUnknownChunks == UnknownChunks)
+			{
+				if (FinalizeBuildPartRetryCount > 0)
+				{
+					// Back off a bit
+					Sleep(1000);
+				}
+			}
+			else
+			{
+				UnknownChunks = RetryUnknownChunks;
+				ZEN_CONSOLE_WARN("{}", BuildUnkownChunksResponse(UnknownChunks, /*WillRetry*/ FinalizeBuildPartRetryCount != 0));
+			}
+		}
+
+		if (!UnknownChunks.empty())
+		{
+			throw std::runtime_error(BuildUnkownChunksResponse(UnknownChunks, /*WillRetry*/ false));
 		}
 
 		m_LogOutput.SetLogOperationProgress(TaskSteps::FinalizeBuild, TaskSteps::StepCount);
@@ -6232,13 +6269,11 @@ BuildsOperationUploadFolder::UploadPartBlobs(const ChunkedFolderContent&			   Co
 											 std::span<const uint32_t>				   LooseChunkIndexes,
 											 const std::uint64_t					   LargeAttachmentSize,
 											 UploadStatistics&						   TempUploadStats,
-											 LooseChunksStatistics&					   TempLooseChunksStats)
+											 LooseChunksStatistics&					   TempLooseChunksStats,
+											 std::vector<IoHash>&					   OutUnknownChunks)
 {
 	ZEN_TRACE_CPU("UploadPartBlobs");
 	{
-		std::unique_ptr<BuildOpLogOutput::ProgressBar> ProgressBarPtr(m_LogOutput.CreateProgressBar("Upload Blobs"));
-		BuildOpLogOutput::ProgressBar&				   Progress(*ProgressBarPtr);
-
 		WorkerThreadPool& ReadChunkPool	  = m_IOWorkerPool;
 		WorkerThreadPool& UploadChunkPool = m_NetworkPool;
 
@@ -6285,12 +6320,17 @@ BuildsOperationUploadFolder::UploadPartBlobs(const ChunkedFolderContent&			   Co
 			}
 			else
 			{
-				ZEN_CONSOLE_WARN(
-					"Build blob {} was reported as needed for upload but it was reported as existing at the start of the "
-					"operation. Treating it as a transient inconsistent issue and will attempt to retry finalization",
-					RawHash);
+				OutUnknownChunks.push_back(RawHash);
 			}
 		}
+		if (BlockIndexes.empty() && LooseChunkOrderIndexes.empty())
+		{
+			return;
+		}
+
+		std::unique_ptr<BuildOpLogOutput::ProgressBar> ProgressBarPtr(m_LogOutput.CreateProgressBar("Upload Blobs"));
+		BuildOpLogOutput::ProgressBar&				   Progress(*ProgressBarPtr);
+
 		uint64_t TotalRawSize = TotalLooseChunksSize + TotalBlocksSize;
 
 		const size_t   UploadBlockCount = BlockIndexes.size();
diff --git a/src/zenremotestore/include/zenremotestore/builds/buildstorageoperations.h b/src/zenremotestore/include/zenremotestore/builds/buildstorageoperations.h
index 351f4b522..a409d08ec 100644
--- a/src/zenremotestore/include/zenremotestore/builds/buildstorageoperations.h
+++ b/src/zenremotestore/include/zenremotestore/builds/buildstorageoperations.h
@@ -677,7 +677,8 @@ private:
 						 std::span<const uint32_t>				   LooseChunkIndexes,
 						 const std::uint64_t					   LargeAttachmentSize,
 						 UploadStatistics&						   TempUploadStats,
-						 LooseChunksStatistics&					   TempLooseChunksStats);
+						 LooseChunksStatistics&					   TempLooseChunksStats,
+						 std::vector<IoHash>&					   OutUnknownChunks);
 
 	CompositeBuffer CompressChunk(const ChunkedFolderContent& Content,
 								  const ChunkedContentLookup& Lookup,