shell: restore the tier download source when its readonly mark fails

VolumeMarkReadonly blocks local writes and deletes before its second master
notification, so a failed notification returns an error with the source
already frozen. Restore it whenever the status was read.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

weed/shell/command_volume_tier_download.go

@@ -190,10 +190,12 @@ func downloadAndReplicateFromRemoteTier(grpcDialOption grpc.DialOption, writer i
 	}
 
 	restore, err := freezeTierDownloadSource(grpcDialOption, vid, source)
+	if restore != nil {
+		defer restore(writer)
+	}
 	if err != nil {
 		return fmt.Errorf("block deletes on volume %d at %s: %v", vid, source, err)
 	}
-	defer restore(writer)
 
 	// Copy over all remote-backed replicas but one, re-reading the topology
 	// so a replica that was offline or created meanwhile is replaced too. The
@@ -245,18 +247,22 @@ func downloadAndReplicateFromRemoteTier(grpcDialOption grpc.DialOption, writer i
 
 // freezeTierDownloadSource blocks deletes on the copy source: a delete applied
 // after VolumeCopy read the source index would be lost on the replaced replica.
-// restore brings back the writable mark, or reloads the persisted state.
+// restore brings back the writable mark, or reloads the persisted state. It is
+// also returned when the mark fails, since the server may have applied it.
 func freezeTierDownloadSource(grpcDialOption grpc.DialOption, vid needle.VolumeId, source pb.ServerAddress) (restore func(io.Writer), err error) {
-	var wasWritable bool
+	var wasWritable, statusRead bool
 	err = operation.WithVolumeServerClient(false, source, grpcDialOption, func(client volume_server_pb.VolumeServerClient) error {
 		resp, statusErr := client.VolumeStatus(context.Background(), &volume_server_pb.VolumeStatusRequest{VolumeId: uint32(vid)})
 		if statusErr != nil {
 			return statusErr
 		}
-		wasWritable = !resp.IsReadOnly
+		wasWritable, statusRead = !resp.IsReadOnly, true
 		_, markErr := client.VolumeMarkReadonly(context.Background(), &volume_server_pb.VolumeMarkReadonlyRequest{VolumeId: uint32(vid)})
 		return markErr
 	})
+	if !statusRead {
+		return nil, err
+	}
 	restore = func(writer io.Writer) {
 		restoreErr := operation.WithVolumeServerClient(false, source, grpcDialOption, func(client volume_server_pb.VolumeServerClient) error {
 			if wasWritable {

weed/shell/command_volume_tier_download_test.go

@@ -44,6 +44,8 @@ type tierDownloadCluster struct {
 	placement    uint32
 	// staleTwin lists a remote-backed twin of the volume on another disk
 	staleTwin map[pb.ServerAddress]bool
+	// markFails applies the readonly mark, then fails as a lost master notification does
+	markFails map[pb.ServerAddress]bool
 }
 
 type tierDownloadTestServer struct {
@@ -104,6 +106,9 @@ func (s *tierDownloadTestServer) VolumeMarkReadonly(_ context.Context, req *volu
 	c.mu.Lock()
 	defer c.mu.Unlock()
 	c.frozen[s.addr] = !req.CanDelete
+	if c.markFails[s.addr] {
+		return nil, fmt.Errorf("notify master: leader changed")
+	}
 	return &volume_server_pb.VolumeMarkReadonlyResponse{}, nil
 }
 
@@ -165,6 +170,7 @@ func newTierDownloadCluster(t *testing.T, states ...string) *tierDownloadCluster
 		writable:     make(map[pb.ServerAddress]bool),
 		remounts:     make(map[pb.ServerAddress]int),
 		staleTwin:    make(map[pb.ServerAddress]bool),
+		markFails:    make(map[pb.ServerAddress]bool),
 	}
 	for _, state := range states {
 		lis, err := net.Listen("tcp", "127.0.0.1:0")
@@ -385,3 +391,24 @@ func TestTierDownloadKeepsRemoteObjectForStaleTwinOnSource(t *testing.T) {
 		t.Errorf("remote object deleted while a twin on %s still references it", c.servers[0])
 	}
 }
+
+func TestTierDownloadRestoresSourceWhenMarkFailsAfterApplying(t *testing.T) {
+	for _, writable := range []bool{false, true} {
+		c := newTierDownloadCluster(t, tierReplicaLocal, tierReplicaRemote, tierReplicaRemote)
+		source := c.servers[0]
+		c.writable[source] = writable
+		c.markFails[source] = true
+		if err := c.run(t); err == nil {
+			t.Fatalf("writable=%v: tier download should fail when the readonly mark fails", writable)
+		}
+		if c.frozen[source] {
+			t.Errorf("writable=%v: source still blocks deletes after the failed mark", writable)
+		}
+		if c.writable[source] != writable {
+			t.Errorf("writable=%v: source writable state changed to %v", writable, c.writable[source])
+		}
+		if !c.objectExists {
+			t.Errorf("writable=%v: remote object deleted although no replica was replaced", writable)
+		}
+	}
+}