volume: say "volume N is read only" like Go, so filer retries deletes (#11544)

VolumeError::ReadOnly displayed "volume is read-only". Go's store and
volume say "volume %d is read only", and the filer's deletion classifier
requeues a failed delete only when the error contains "is read only".
Against a Rust volume server a BatchDelete on a read-only volume (tier
move, maintenance) was booked as a permanent failure and the chunk was
never deleted.

ReadOnly now carries the volume id and displays Go's text. The text
reaches clients through BatchDelete results, the HTTP write and delete
error bodies, and gRPC statuses; the gRPC code (FailedPrecondition) and
the HTTP/BatchDelete status codes are unchanged.

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-authored-by: Chris Lu <chris.lu@gmail.com>

seaweed-volume/src/server/grpc_server.rs

@@ -11808,4 +11808,71 @@ mod tests {
         assert!(err.message().contains("has no live entries"), "{err:?}");
         assert!(!std::path::Path::new(&format!("{data}/1.dat")).exists());
     }
+
+    // Among storage errors, the filer requeues a delete only on "is read only"
+    // (Go's DeleteVolumeNeedle text).
+    #[tokio::test]
+    async fn test_batch_delete_on_read_only_volume_says_is_read_only() {
+        let request = || {
+            Request::new(volume_server_pb::BatchDeleteRequest {
+                file_ids: vec!["1,b00003344".to_string()],
+                skip_cookie_check: false,
+            })
+        };
+
+        let (service, _tmp) = make_local_service_with_volume("", None);
+        service
+            .state
+            .store
+            .write()
+            .unwrap()
+            .find_volume_mut(VolumeId(1))
+            .unwrap()
+            .1
+            .set_no_write_or_delete(true);
+        let resp = service.batch_delete(request()).await.unwrap().into_inner();
+        assert_eq!(resp.results.len(), 1);
+        assert_eq!(resp.results[0].status, 500);
+        assert_eq!(resp.results[0].error, "volume 1 is read only");
+
+        // Controls: a writable volume, and one read-only but deletable.
+        let (service, _tmp) = make_local_service_with_volume("", None);
+        let resp = service.batch_delete(request()).await.unwrap().into_inner();
+        assert_eq!(resp.results[0].status, 202, "{:?}", resp.results[0].error);
+
+        let (service, _tmp) = make_local_service_with_volume("", None);
+        service
+            .state
+            .store
+            .write()
+            .unwrap()
+            .find_volume_mut(VolumeId(1))
+            .unwrap()
+            .1
+            .set_read_only_persist(true, false)
+            .unwrap();
+        let resp = service.batch_delete(request()).await.unwrap().into_inner();
+        assert_eq!(resp.results[0].status, 202, "{:?}", resp.results[0].error);
+    }
+
+    // Go says "volume N not found[ on host:port]" with the same statuses; the
+    // bare "not found" is kept on purpose: the filer skips it, whereas Go's
+    // text is booked permanent and outranks a retryable sibling replica.
+    #[tokio::test]
+    async fn test_batch_delete_on_missing_volume_says_not_found() {
+        let (service, _tmp) = make_local_service_with_volume("", None);
+        for (skip_cookie_check, status) in [(false, 404), (true, 500)] {
+            let resp = service
+                .batch_delete(Request::new(volume_server_pb::BatchDeleteRequest {
+                    file_ids: vec!["2,b00003344".to_string()],
+                    skip_cookie_check,
+                }))
+                .await
+                .unwrap()
+                .into_inner();
+            assert_eq!(resp.results[0].status, status, "skip={skip_cookie_check}");
+            assert_eq!(resp.results[0].error, "not found");
+        }
+    }
+
 }

seaweed-volume/src/server/mod.rs

@@ -27,7 +27,9 @@ impl From<VolumeError> for Status {
             VolumeError::NotFound
             | VolumeError::VolumeNotFound(_)
             | VolumeError::Tier(TierError::NotFound(_)) => Status::not_found(message),
-            VolumeError::ReadOnly | VolumeError::NotEmpty => Status::failed_precondition(message),
+            VolumeError::ReadOnly(_) | VolumeError::NotEmpty => {
+                Status::failed_precondition(message)
+            }
             VolumeError::InsufficientSpace { .. } => Status::resource_exhausted(message),
             VolumeError::AlreadyExists => Status::already_exists(message),
             _ => Status::internal(message),
@@ -69,7 +71,14 @@ mod tests {
             Code::NotFound
         );
         assert_eq!(code(VolumeError::NotFound), Code::NotFound);
-        assert_eq!(code(VolumeError::ReadOnly), Code::FailedPrecondition);
+        assert_eq!(
+            code(VolumeError::ReadOnly(VolumeId(7))),
+            Code::FailedPrecondition
+        );
+        assert_eq!(
+            VolumeError::ReadOnly(VolumeId(7)).to_string(),
+            "volume 7 is read only"
+        );
         assert_eq!(
             code(VolumeError::InsufficientSpace {
                 vid: VolumeId(7),

seaweed-volume/src/storage/store.rs

@@ -779,7 +779,7 @@ impl Store {
             .is_disk_space_low
             .load(Ordering::Relaxed)
         {
-            return Err(VolumeError::ReadOnly);
+            return Err(VolumeError::ReadOnly(vid));
         }
 
         let (_, vol) = self.find_volume_mut(vid).ok_or(VolumeError::NotFound)?;
@@ -795,7 +795,7 @@ impl Store {
         // Match Go's DeleteVolumeNeedle: check noWriteOrDelete before proceeding.
         let (_, vol) = self.find_volume(vid).ok_or(VolumeError::NotFound)?;
         if vol.is_no_write_or_delete() {
-            return Err(VolumeError::ReadOnly);
+            return Err(VolumeError::ReadOnly(vid));
         }
 
         let (_, vol) = self.find_volume_mut(vid).ok_or(VolumeError::NotFound)?;

seaweed-volume/src/storage/volume.rs

@@ -61,8 +61,8 @@ pub enum VolumeError {
     #[error("volume already exists")]
     AlreadyExists,
 
-    #[error("volume is read-only")]
-    ReadOnly,
+    #[error("volume {0} is read only")]
+    ReadOnly(VolumeId),
 
     #[error("volume is unavailable: {0}")]
     Unavailable(String),
@@ -2422,7 +2422,7 @@ impl Volume {
             return Err(e);
         }
         if self.is_read_only() {
-            return Err(VolumeError::ReadOnly);
+            return Err(VolumeError::ReadOnly(self.id));
         }
         Ok(())
     }
@@ -2803,7 +2803,7 @@ impl Volume {
             return Err(e);
         }
         if self.no_write_or_delete {
-            return Err(VolumeError::ReadOnly);
+            return Err(VolumeError::ReadOnly(self.id));
         }
         self.do_delete_request(n)
     }
@@ -4155,7 +4155,7 @@ impl Volume {
         needle_blob: &[u8],
     ) -> Result<(), VolumeError> {
         if self.is_read_only() {
-            return Err(VolumeError::ReadOnly);
+            return Err(VolumeError::ReadOnly(self.id));
         }
         let dat_file = self
             .dat_file
@@ -4176,7 +4176,7 @@ impl Volume {
     ) -> Result<(), VolumeError> {
         // nm.put on a read-only volume fails only after the blob is appended to .dat.
         if self.is_read_only() {
-            return Err(VolumeError::ReadOnly);
+            return Err(VolumeError::ReadOnly(self.id));
         }
         // Storage guard: negativity-only (Go parity). See parse_needle_at.
         if size.0 < 0 {
@@ -6187,7 +6187,7 @@ mod tests {
         assert!(
             matches!(
                 v.write_needle(&mut later, true, false),
-                Err(VolumeError::ReadOnly)
+                Err(VolumeError::ReadOnly(_))
             ),
             "later writes must not append past the record whose index is in doubt"
         );
@@ -8394,7 +8394,7 @@ mod tests {
                 false,
             )
             .unwrap_err();
-        assert!(matches!(err, VolumeError::ReadOnly));
+        assert!(matches!(err, VolumeError::ReadOnly(_)));
 
         let deleted_size = v
             .delete_needle(&mut Needle {
@@ -9318,7 +9318,7 @@ mod tests {
                     false,
                 )
                 .unwrap_err();
-            assert!(matches!(err, VolumeError::ReadOnly));
+            assert!(matches!(err, VolumeError::ReadOnly(_)));
 
             let deleted = v
                 .delete_needle(&mut Needle {
@@ -9359,7 +9359,7 @@ mod tests {
                 false,
             )
             .unwrap_err();
-        assert!(matches!(err, VolumeError::ReadOnly));
+        assert!(matches!(err, VolumeError::ReadOnly(_)));
 
         let deleted = v
             .delete_needle(&mut Needle {
@@ -9426,7 +9426,7 @@ mod tests {
             })
             .unwrap_err();
         assert!(
-            matches!(err, VolumeError::ReadOnly),
+            matches!(err, VolumeError::ReadOnly(_)),
             "plain readonly must reject deletes"
         );
 

seaweed-volume/tests/http_integration.rs

@@ -477,6 +477,74 @@ async fn delete_then_get_returns_404() {
     );
 }
 
+// Go answers both with 500 and an error containing "volume N is read only"
+// (the write behind "failed to write to local disk: ").
+#[tokio::test]
+async fn write_and_delete_on_read_only_volume_say_is_read_only() {
+    let (state, _tmp) = test_state();
+    let uri = "/1,01637037d6";
+
+    let request = |method: &str, body: &[u8]| {
+        Request::builder()
+            .method(method)
+            .uri(uri)
+            .body(Body::from(body.to_vec()))
+            .unwrap()
+    };
+    let error_of = |body: Vec<u8>| -> String {
+        let json: serde_json::Value = serde_json::from_slice(&body).unwrap();
+        json["error"].as_str().unwrap_or_default().to_string()
+    };
+
+    let response = build_admin_router(state.clone())
+        .oneshot(request("POST", b"written before read-only"))
+        .await
+        .unwrap();
+    assert_eq!(response.status(), StatusCode::CREATED);
+
+    state
+        .store
+        .write()
+        .unwrap()
+        .find_volume_mut(VolumeId(1))
+        .unwrap()
+        .1
+        .set_no_write_or_delete(true);
+
+    let response = build_admin_router(state.clone())
+        .oneshot(request("POST", b"refused"))
+        .await
+        .unwrap();
+    assert_eq!(response.status(), StatusCode::INTERNAL_SERVER_ERROR);
+    let error = error_of(body_bytes(response).await);
+    assert!(error.contains("volume 1 is read only"), "{error}");
+
+    let response = build_admin_router(state.clone())
+        .oneshot(request("DELETE", b""))
+        .await
+        .unwrap();
+    assert_eq!(response.status(), StatusCode::INTERNAL_SERVER_ERROR);
+    assert_eq!(
+        error_of(body_bytes(response).await),
+        "Deletion Failed: volume 1 is read only"
+    );
+
+    // The needle is still there to delete once the volume is writable again.
+    state
+        .store
+        .write()
+        .unwrap()
+        .find_volume_mut(VolumeId(1))
+        .unwrap()
+        .1
+        .set_no_write_or_delete(false);
+    let response = build_admin_router(state.clone())
+        .oneshot(request("DELETE", b""))
+        .await
+        .unwrap();
+    assert_eq!(response.status(), StatusCode::ACCEPTED);
+}
+
 // ============================================================================
 // 6. HEAD returns headers without body
 // ============================================================================