From b3e2a03cbefb22f87cb6896252dda4bbeb490431 Mon Sep 17 00:00:00 2001
From: Chris Lu <chris.lu@gmail.com>
Date: Wed, 8 Oct 2025 22:09:30 -0700
Subject: [PATCH] Address fourth round of Gemini Code Assist review comments

Fixed two critical issues identified in the fourth review:

1. HIGH: Handle BlobAlreadyExists in append blob creation
   - Problem: If append blob already exists, Create() fails causing replication failure
   - Fix: Added bloberror.HasCode(err, bloberror.BlobAlreadyExists) check
   - Behavior: Existing append blobs are now acceptable, appends can proceed
   - Impact: Makes replication sink more robust, prevents unnecessary failures
   - Location: azure_sink.go CreateEntry function

2. MEDIUM: Configure custom retry policy for download resiliency
   - Problem: Old SDK had MaxRetryRequests: 20, new SDK defaults to 3 retries
   - Fix: Configured policy.RetryOptions with MaxRetries: 10
   - Settings: TryTimeout=1min, RetryDelay=2s, MaxRetryDelay=1min
   - Impact: Maintains similar resiliency in unreliable network conditions
   - Location: azure_storage_client.go client initialization

Changes:
- Added import: github.com/Azure/azure-sdk-for-go/sdk/azcore/policy
- Updated NewClientWithSharedKeyCredential to include ClientOptions with retry policy
- Updated CreateEntry error handling to allow BlobAlreadyExists

Technical details:
- Retry policy uses exponential backoff (default SDK behavior)
- MaxRetries=10 provides good balance (was 20 in old SDK, default is 3)
- TryTimeout prevents individual requests from hanging indefinitely
- BlobAlreadyExists handling allows idempotent append operations

All tests pass. Build succeeds. Code is more resilient and robust.
---
 .../remote_storage/azure/azure_storage_client.go | 14 +++++++++++++-
 weed/replication/sink/azuresink/azure_sink.go    | 16 ++++++++++------
 2 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/weed/remote_storage/azure/azure_storage_client.go b/weed/remote_storage/azure/azure_storage_client.go
index b657b5777..6034add09 100644
--- a/weed/remote_storage/azure/azure_storage_client.go
+++ b/weed/remote_storage/azure/azure_storage_client.go
@@ -7,7 +7,10 @@ import (
 	"os"
 	"reflect"
 	"strings"
+	"time"
 
+	"github.com/Azure/azure-sdk-for-go/sdk/azcore"
+	"github.com/Azure/azure-sdk-for-go/sdk/azcore/policy"
 	"github.com/Azure/azure-sdk-for-go/sdk/azcore/to"
 	"github.com/Azure/azure-sdk-for-go/sdk/storage/azblob"
 	"github.com/Azure/azure-sdk-for-go/sdk/storage/azblob/blob"
@@ -57,7 +60,16 @@ func (s azureRemoteStorageMaker) Make(conf *remote_pb.RemoteConf) (remote_storag
 	}
 
 	serviceURL := fmt.Sprintf("https://%s.blob.core.windows.net/", accountName)
-	azClient, err := azblob.NewClientWithSharedKeyCredential(serviceURL, credential, nil)
+	azClient, err := azblob.NewClientWithSharedKeyCredential(serviceURL, credential, &azblob.ClientOptions{
+		ClientOptions: azcore.ClientOptions{
+			Retry: policy.RetryOptions{
+				MaxRetries:    10, // Increased from default 3 to maintain resiliency similar to old SDK's 20
+				TryTimeout:    time.Minute,
+				RetryDelay:    2 * time.Second,
+				MaxRetryDelay: time.Minute,
+			},
+		},
+	})
 	if err != nil {
 		return nil, fmt.Errorf("failed to create Azure client: %v", err)
 	}
diff --git a/weed/replication/sink/azuresink/azure_sink.go b/weed/replication/sink/azuresink/azure_sink.go
index 13e4c9f30..90d901c76 100644
--- a/weed/replication/sink/azuresink/azure_sink.go
+++ b/weed/replication/sink/azuresink/azure_sink.go
@@ -135,13 +135,17 @@ func (g *AzureSink) CreateEntry(key string, entry *filer_pb.Entry, signatures []
 	})
 	
 	if err != nil {
-		// Check if this is a precondition failed error (HTTP 412)
-		var respErr *azcore.ResponseError
-		if ok := errors.As(err, &respErr); ok && respErr.StatusCode == http.StatusPreconditionFailed {
-			glog.V(0).Infof("skip overwriting %s/%s: precondition failed", g.container, key)
-			return nil
+		if bloberror.HasCode(err, bloberror.BlobAlreadyExists) {
+			// Blob already exists, which is fine for an append blob - we can append to it
+		} else {
+			// Check if this is a precondition failed error (HTTP 412)
+			var respErr *azcore.ResponseError
+			if ok := errors.As(err, &respErr); ok && respErr.StatusCode == http.StatusPreconditionFailed {
+				glog.V(0).Infof("skip overwriting %s/%s: precondition failed", g.container, key)
+				return nil
+			}
+			return fmt.Errorf("azure create append blob %s/%s: %v", g.container, key, err)
 		}
-		return fmt.Errorf("azure create append blob %s/%s: %v", g.container, key, err)
 	}
 
 	writeFunc := func(data []byte) error {