blockIDToIndex → blockIDToPartNumber (#838)

The former approach is too error-prone and without this change results in "received a block list pointing to a non-existent block" because of a missing "+ 1" when calling GetPart() in putBlockList().
cirruslabs · Feb 10, 2025 · d9c48ea · d9c48ea
1 parent ec04af7
commit d9c48ea
Show file tree

Hide file tree

Showing 3 changed files with 13 additions and 12 deletions.
diff --git a/internal/agent/http_cache/azureblob/blockid.go b/internal/agent/http_cache/azureblob/blockid.go
@@ -11,7 +11,7 @@ import (
 // [1]: https://github.com/Azure/azure-sdk-for-js/blob/fc4cbf0e10e15cbbe7cf873294db7d6e2bd02851/sdk/storage/storage-blob/src/utils/utils.common.ts#L486-L487
 const maxBlockIndexLength = 6
 
-func blockIDToIndex(blockIDRaw string) (int, error) {
+func blockIDToPartNumber(blockIDRaw string) (int, error) {
 	// Decode the Base64-encoded block ID
 	blockIDBytes, err := base64.StdEncoding.DecodeString(blockIDRaw)
 	if err != nil {
@@ -33,5 +33,6 @@ func blockIDToIndex(blockIDRaw string) (int, error) {
 		return 0, err
 	}
 
-	return blockIndex, nil
+	// Part numbers start with 1
+	return blockIndex + 1, nil
 }
diff --git a/internal/agent/http_cache/azureblob/blockid_test.go b/internal/agent/http_cache/azureblob/blockid_test.go
@@ -5,8 +5,8 @@ import (
 	"testing"
 )
 
-func TestBlockIDToIndex(t *testing.T) {
-	blockIndex, err := blockIDToIndex("Yzg4ODM0YjYtZmI3MC00OWNmLWJlYmEtNDliODFjNDE0MWM3MDAwMDAwMDAwMDAx")
+func TestBlockIDToPartNumber(t *testing.T) {
+	partNumber, err := blockIDToPartNumber("Yzg4ODM0YjYtZmI3MC00OWNmLWJlYmEtNDliODFjNDE0MWM3MDAwMDAwMDAwMDAx")
 	require.NoError(t, err)
-	require.Equal(t, 1, blockIndex)
+	require.Equal(t, 2, partNumber)
 }
diff --git a/internal/agent/http_cache/azureblob/putblob.go b/internal/agent/http_cache/azureblob/putblob.go
@@ -101,7 +101,7 @@ func (azureBlob *AzureBlob) putBlock(writer http.ResponseWriter, request *http.R
 	// Decode the block ID
 	blockID := request.URL.Query().Get("blockid")
 
-	blockIndex, err := blockIDToIndex(blockID)
+	partNumber, err := blockIDToPartNumber(blockID)
 	if err != nil {
 		fail(writer, request, http.StatusBadRequest, "failed to extract the block index "+
 			"from block ID", "key", key, "blockid", blockID, "err", err)
@@ -144,7 +144,7 @@ func (azureBlob *AzureBlob) putBlock(writer http.ResponseWriter, request *http.R
 		&api.MultipartCacheUploadPartRequest{
 			CacheKey:      cacheKey,
 			UploadId:      uploadID,
-			PartNumber:    uint32(blockIndex) + 1,
+			PartNumber:    uint32(partNumber),
 			ContentLength: contentLength,
 		},
 	)
@@ -184,7 +184,7 @@ func (azureBlob *AzureBlob) putBlock(writer http.ResponseWriter, request *http.R
 		return
 	}
 
-	if err := uploadable.AppendPart(blockIndex, resp.Header.Get("ETag")); err != nil {
+	if err := uploadable.AppendPart(partNumber, resp.Header.Get("ETag")); err != nil {
 		fail(writer, request, http.StatusBadRequest, "failed to finalize part upload", "err", err)
 
 		return
@@ -237,24 +237,24 @@ func (azureBlob *AzureBlob) putBlockList(writer http.ResponseWriter, request *ht
 
 	for _, blockID := range blockList.Latest {
 		// Decode the part number
-		blockIndex, err := blockIDToIndex(blockID)
+		partNumber, err := blockIDToPartNumber(blockID)
 		if err != nil {
 			fail(writer, request, http.StatusBadRequest, "received a block list pointing to a non-parseable block",
 				"key", key, "blockid", blockID, "err", err)
 
 			return
 		}
 
-		part := uploadable.GetPart(blockIndex + 1)
+		part := uploadable.GetPart(partNumber)
 		if part == nil {
 			fail(writer, request, http.StatusBadRequest, "received a block list pointing to a non-existent block",
-				"key", key, "blockid", blockID, "index", blockIndex)
+				"key", key, "blockid", blockID, "index", partNumber)
 
 			return
 		}
 
 		protoParts = append(protoParts, &api.MultipartCacheUploadCommitRequest_Part{
-			PartNumber: uint32(blockIndex),
+			PartNumber: uint32(partNumber),
 			Etag:       part.ETag,
 		})
 	}