Browse Source

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.
pull/7310/head
Chris Lu 2 days ago
parent
commit
b3e2a03cbe
  1. 14
      weed/remote_storage/azure/azure_storage_client.go
  2. 4
      weed/replication/sink/azuresink/azure_sink.go

14
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)
}

4
weed/replication/sink/azuresink/azure_sink.go

@ -135,6 +135,9 @@ func (g *AzureSink) CreateEntry(key string, entry *filer_pb.Entry, signatures []
})
if err != 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 {
@ -143,6 +146,7 @@ func (g *AzureSink) CreateEntry(key string, entry *filer_pb.Entry, signatures []
}
return fmt.Errorf("azure create append blob %s/%s: %v", g.container, key, err)
}
}
writeFunc := func(data []byte) error {
_, writeErr := appendBlobClient.AppendBlock(context.Background(), streaming.NopCloser(bytes.NewReader(data)), &appendblob.AppendBlockOptions{})

Loading…
Cancel
Save