fix: avoid snapshot query hanging during volume deletion

Try DeleteDataset first before querying snapshots — in the common case
(no dependent snapshots), this avoids the expensive zfs.snapshot.query
call entirely. When snapshot cleanup is needed, use select: ["id"] to
return only snapshot IDs instead of full objects, and add a 30-second
timeout as a safety net. This fixes a ~2 minute hang when TrueNAS
returns oversized snapshot query responses (>1MB WebSocket limit).

Author: fenio

pkg/driver/controller.go

@@ -350,31 +350,39 @@ func (s *ControllerService) lookupSnapshotByCSIName(ctx context.Context, poolDat
 //   - The snapshot moves from the source to the promoted clone.
 //   - The original source volume becomes a dependent of the promoted snapshot.
 //   - Without deleting the snapshot first, neither the clone nor the source can be deleted.
+//
+// Uses a 30-second timeout as a safety net — this is best-effort cleanup, not critical path.
+// Uses QuerySnapshotIDs with select: ["id"] to minimize response size (avoids "message too big"
+// errors when datasets have many snapshots with large property sets).
 func (s *ControllerService) deleteDatasetSnapshots(ctx context.Context, datasetID string) {
 	klog.V(4).Infof("Checking for snapshots on dataset %s before deletion", datasetID)
 
-	// Query all snapshots on this dataset
-	snapshots, err := s.apiClient.QuerySnapshots(ctx, []interface{}{
+	// Use a short timeout — this is best-effort cleanup, not critical path
+	snapCtx, cancel := context.WithTimeout(ctx, 30*time.Second)
+	defer cancel()
+
+	// Query only snapshot IDs (not full objects) to minimize response size
+	snapIDs, err := s.apiClient.QuerySnapshotIDs(snapCtx, []interface{}{
 		[]interface{}{"dataset", "=", datasetID},
 	})
 	if err != nil {
 		klog.Warningf("Failed to query snapshots for dataset %s: %v (continuing with deletion)", datasetID, err)
 		return // Don't fail deletion if we can't query snapshots
 	}
 
-	if len(snapshots) == 0 {
+	if len(snapIDs) == 0 {
 		klog.V(4).Infof("No snapshots found on dataset %s", datasetID)
 		return
 	}
 
-	klog.Infof("Found %d snapshots on dataset %s, deleting them first", len(snapshots), datasetID)
+	klog.Infof("Found %d snapshots on dataset %s, deleting them first", len(snapIDs), datasetID)
 
-	for _, snap := range snapshots {
-		klog.V(4).Infof("Deleting snapshot %s (defer=true to handle dependent clones)", snap.ID)
-		if err := s.apiClient.DeleteSnapshot(ctx, snap.ID); err != nil {
+	for _, snapID := range snapIDs {
+		klog.V(4).Infof("Deleting snapshot %s (defer=true to handle dependent clones)", snapID)
+		if err := s.apiClient.DeleteSnapshot(snapCtx, snapID); err != nil {
 			// Log warning but continue - the snapshot might already be deleted or
 			// have dependents that will be handled by defer=true
-			klog.Warningf("Failed to delete snapshot %s: %v (continuing)", snap.ID, err)
+			klog.Warningf("Failed to delete snapshot %s: %v (continuing)", snapID, err)
 		}
 	}
 }

pkg/driver/controller_iscsi.go

@@ -9,6 +9,7 @@ import (
 
 	"github.com/container-storage-interface/spec/lib/go/csi"
 	"github.com/fenio/tns-csi/pkg/metrics"
+	"github.com/fenio/tns-csi/pkg/retry"
 	"github.com/fenio/tns-csi/pkg/tnsapi"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
@@ -520,6 +521,8 @@ func (s *ControllerService) createISCSITargetExtent(ctx context.Context, targetI
 }
 
 // deleteISCSIVolume deletes an iSCSI volume and all associated resources.
+//
+//nolint:dupl // Intentionally similar ZVOL deletion pattern as NFS/NVMe-oF
 func (s *ControllerService) deleteISCSIVolume(ctx context.Context, meta *VolumeMetadata) (*csi.DeleteVolumeResponse, error) {
 	timer := metrics.NewVolumeOperationTimer(metrics.ProtocolISCSI, "delete")
 	klog.Infof("Deleting iSCSI volume: %s (Dataset: %s, Target: %d, Extent: %d)",
@@ -576,24 +579,29 @@ func (s *ControllerService) deleteISCSIVolume(ctx context.Context, meta *VolumeM
 		}
 	}
 
-	// Step 4: Delete any snapshots on the ZVOL first (handles promoted clone cleanup)
-	// This is necessary because after ZFS clone promotion, snapshots may have dependent
-	// clones that prevent deletion. Deleting with defer=true allows ZFS to clean up properly.
+	// Step 4: Delete ZVOL (try direct first, snapshot cleanup on failure)
 	if meta.DatasetID != "" {
-		s.deleteDatasetSnapshots(ctx, meta.DatasetID)
-	}
+		firstErr := s.apiClient.DeleteDataset(ctx, meta.DatasetID)
+		if firstErr != nil && !isNotFoundError(firstErr) {
+			klog.Infof("Direct deletion failed for %s: %v — cleaning up snapshots before retry",
+				meta.DatasetID, firstErr)
+			s.deleteDatasetSnapshots(ctx, meta.DatasetID)
+
+			retryConfig := retry.DeletionConfig("delete-iscsi-zvol")
+			err := retry.WithRetryNoResult(ctx, retryConfig, func() error {
+				deleteErr := s.apiClient.DeleteDataset(ctx, meta.DatasetID)
+				if deleteErr != nil && isNotFoundError(deleteErr) {
+					return nil
+				}
+				return deleteErr
+			})
 
-	// Step 5: Delete ZVOL
-	if meta.DatasetID != "" {
-		if err := s.apiClient.DeleteDataset(ctx, meta.DatasetID); err != nil {
-			if !isNotFoundError(err) {
+			if err != nil {
 				timer.ObserveError()
 				return nil, status.Errorf(codes.Internal, "Failed to delete ZVOL %s: %v", meta.DatasetID, err)
 			}
-			klog.V(4).Infof("ZVOL already deleted: %s", meta.DatasetID)
-		} else {
-			klog.V(4).Infof("Deleted ZVOL: %s", meta.DatasetID)
 		}
+		klog.V(4).Infof("Deleted ZVOL: %s", meta.DatasetID)
 	}
 
 	// Clear volume capacity metric

pkg/driver/controller_nfs.go

@@ -523,6 +523,8 @@ func (s *ControllerService) createNFSVolume(ctx context.Context, req *csi.Create
 // deleteNFSVolume deletes an NFS volume with ownership verification.
 // Dataset deletion is retried for busy resource errors.
 // If deleteStrategy is "retain", the volume is kept but CSI returns success.
+//
+//nolint:dupl // Intentionally similar dataset deletion pattern as iSCSI
 func (s *ControllerService) deleteNFSVolume(ctx context.Context, meta *VolumeMetadata) (*csi.DeleteVolumeResponse, error) {
 	timer := metrics.NewVolumeOperationTimer(metrics.ProtocolNFS, "delete")
 	klog.V(4).Infof("Deleting NFS volume: %s (dataset: %s, share ID: %d)", meta.Name, meta.DatasetName, meta.NFSShareID)
@@ -623,34 +625,31 @@ func (s *ControllerService) deleteNFSVolume(ctx context.Context, meta *VolumeMet
 		}
 	}
 
-	// Step 2: Delete any snapshots on the dataset first (handles promoted clone cleanup)
-	// This is necessary because after ZFS clone promotion, snapshots may have dependent
-	// clones that prevent deletion. Deleting with defer=true allows ZFS to clean up properly.
-	if meta.DatasetID != "" {
-		s.deleteDatasetSnapshots(ctx, meta.DatasetID)
-	}
-
-	// Step 3: Delete ZFS dataset with retry logic for busy resources
+	// Step 2: Delete dataset (try direct first, snapshot cleanup on failure)
 	if meta.DatasetID == "" {
 		klog.V(4).Infof("No dataset ID provided, skipping dataset deletion")
 	} else {
-		klog.V(4).Infof("Deleting dataset: %s (with retry for busy resources)", meta.DatasetID)
+		klog.V(4).Infof("Deleting dataset: %s", meta.DatasetID)
+
+		firstErr := s.apiClient.DeleteDataset(ctx, meta.DatasetID)
+		if firstErr != nil && !isNotFoundError(firstErr) {
+			klog.Infof("Direct deletion failed for %s: %v — cleaning up snapshots before retry",
+				meta.DatasetID, firstErr)
+			s.deleteDatasetSnapshots(ctx, meta.DatasetID)
+
+			retryConfig := retry.DeletionConfig("delete-nfs-dataset")
+			err := retry.WithRetryNoResult(ctx, retryConfig, func() error {
+				deleteErr := s.apiClient.DeleteDataset(ctx, meta.DatasetID)
+				if deleteErr != nil && isNotFoundError(deleteErr) {
+					return nil
+				}
+				return deleteErr
+			})
 
-		retryConfig := retry.DeletionConfig("delete-nfs-dataset")
-		err := retry.WithRetryNoResult(ctx, retryConfig, func() error {
-			deleteErr := s.apiClient.DeleteDataset(ctx, meta.DatasetID)
-			if deleteErr != nil && isNotFoundError(deleteErr) {
-				// Dataset already deleted - not an error (idempotency)
-				klog.V(4).Infof("Dataset %s not found, assuming already deleted (idempotency)", meta.DatasetID)
-				return nil
+			if err != nil {
+				timer.ObserveError()
+				return nil, status.Errorf(codes.Internal, "Failed to delete dataset %s: %v", meta.DatasetID, err)
 			}
-			return deleteErr
-		})
-
-		if err != nil {
-			// All retries exhausted or non-retryable error
-			timer.ObserveError()
-			return nil, status.Errorf(codes.Internal, "Failed to delete dataset %s: %v", meta.DatasetID, err)
 		}
 		klog.V(4).Infof("Successfully deleted dataset %s", meta.DatasetID)
 	}

pkg/driver/controller_nvmeof.go

@@ -928,6 +928,9 @@ func (s *ControllerService) verifyNamespaceDeletion(ctx context.Context, meta *V
 }
 
 // deleteZVOL deletes a ZVOL dataset with retry logic for busy resources.
+// Uses a try-first approach: attempts direct deletion (which handles the common case where
+// recursive=true succeeds), then falls back to snapshot cleanup + retry if the direct
+// attempt fails. This avoids the expensive snapshot query in the common case.
 func (s *ControllerService) deleteZVOL(ctx context.Context, meta *VolumeMetadata) error {
 	if meta.DatasetID == "" {
 		klog.Infof("deleteZVOL: DatasetID is empty, skipping deletion")
@@ -936,29 +939,33 @@ func (s *ControllerService) deleteZVOL(ctx context.Context, meta *VolumeMetadata
 
 	klog.Infof("deleteZVOL: Starting deletion of ZVOL %s for volume %s", meta.DatasetID, meta.Name)
 
-	// Delete any snapshots on the ZVOL first (handles promoted clone cleanup)
-	// This is necessary because after ZFS clone promotion, snapshots may have dependent
-	// clones that prevent deletion. Deleting with defer=true allows ZFS to clean up properly.
+	// Try direct deletion first (common case: no dependent snapshots)
+	firstErr := s.apiClient.DeleteDataset(ctx, meta.DatasetID)
+	if firstErr == nil || isNotFoundError(firstErr) {
+		klog.Infof("deleteZVOL: Successfully deleted ZVOL %s", meta.DatasetID)
+		return nil
+	}
+
+	// First attempt failed — clean up snapshots and retry
+	klog.Infof("deleteZVOL: Direct deletion failed for %s: %v — cleaning up snapshots before retry",
+		meta.DatasetID, firstErr)
 	s.deleteDatasetSnapshots(ctx, meta.DatasetID)
 
 	retryConfig := retry.DeletionConfig("delete-zvol")
 	err := retry.WithRetryNoResult(ctx, retryConfig, func() error {
 		deleteErr := s.apiClient.DeleteDataset(ctx, meta.DatasetID)
 		if deleteErr != nil && isNotFoundError(deleteErr) {
-			// ZVOL already deleted - not an error (idempotency)
-			klog.V(4).Infof("ZVOL %s not found, assuming already deleted (idempotency)", meta.DatasetID)
 			return nil
 		}
 		return deleteErr
 	})
 
 	if err != nil {
-		// All retries exhausted or non-retryable error
 		klog.Errorf("deleteZVOL: Failed to delete ZVOL %s: %v", meta.DatasetID, err)
 		return status.Errorf(codes.Internal, "Failed to delete ZVOL %s: %v", meta.DatasetID, err)
 	}
 
-	klog.Infof("deleteZVOL: Successfully deleted ZVOL %s for volume %s", meta.DatasetID, meta.Name)
+	klog.Infof("deleteZVOL: Successfully deleted ZVOL %s after snapshot cleanup", meta.DatasetID)
 	return nil
 }
 

pkg/driver/controller_snapshot_test.go

@@ -66,6 +66,10 @@ func (m *MockAPIClientForSnapshots) QuerySnapshots(ctx context.Context, filters
 	return nil, errors.New("QuerySnapshotsFunc not implemented")
 }
 
+func (m *MockAPIClientForSnapshots) QuerySnapshotIDs(ctx context.Context, filters []interface{}) ([]string, error) {
+	return nil, nil
+}
+
 func (m *MockAPIClientForSnapshots) CloneSnapshot(ctx context.Context, params tnsapi.CloneSnapshotParams) (*tnsapi.Dataset, error) {
 	if m.CloneSnapshotFunc != nil {
 		return m.CloneSnapshotFunc(ctx, params)

pkg/driver/controller_test.go

@@ -166,6 +166,10 @@ func (m *mockAPIClient) QuerySnapshots(ctx context.Context, filters []interface{
 	return nil, nil
 }
 
+func (m *mockAPIClient) QuerySnapshotIDs(ctx context.Context, filters []interface{}) ([]string, error) {
+	return nil, nil
+}
+
 func (m *mockAPIClient) CloneSnapshot(ctx context.Context, params tnsapi.CloneSnapshotParams) (*tnsapi.Dataset, error) {
 	return nil, errNotImplemented
 }

pkg/tnsapi/client.go

@@ -1651,6 +1651,32 @@ func (c *Client) QuerySnapshots(ctx context.Context, filters []interface{}) ([]S
 	return result, nil
 }
 
+// QuerySnapshotIDs is a lightweight version of QuerySnapshots that only returns snapshot IDs.
+// It uses select: ["id"] to minimize response size, which is critical when datasets have
+// many snapshots with large property sets (e.g., after migration from democratic-csi).
+func (c *Client) QuerySnapshotIDs(ctx context.Context, filters []interface{}) ([]string, error) {
+	klog.V(4).Infof("Querying snapshot IDs with filters: %+v", filters)
+
+	queryOpts := map[string]interface{}{
+		"select": []string{"id"},
+	}
+	var result []struct {
+		ID string `json:"id"`
+	}
+	err := c.Call(ctx, "zfs.snapshot.query", []interface{}{filters, queryOpts}, &result)
+	if err != nil {
+		return nil, fmt.Errorf("failed to query snapshot IDs: %w", err)
+	}
+
+	ids := make([]string, len(result))
+	for i, s := range result {
+		ids[i] = s.ID
+	}
+
+	klog.V(4).Infof("Found %d snapshot IDs", len(ids))
+	return ids, nil
+}
+
 // CloneSnapshotParams represents parameters for cloning a snapshot.
 type CloneSnapshotParams struct {
 	Snapshot string `json:"snapshot"`    // Source snapshot ID (dataset@snapshot)

pkg/tnsapi/interface.go

@@ -87,6 +87,7 @@ type ClientInterface interface {
 	CreateSnapshot(ctx context.Context, params SnapshotCreateParams) (*Snapshot, error)
 	DeleteSnapshot(ctx context.Context, snapshotID string) error
 	QuerySnapshots(ctx context.Context, filters []interface{}) ([]Snapshot, error)
+	QuerySnapshotIDs(ctx context.Context, filters []interface{}) ([]string, error)
 	CloneSnapshot(ctx context.Context, params CloneSnapshotParams) (*Dataset, error)
 
 	// Dataset promotion (for detached clones)

tests/sanity/mock_client.go

@@ -903,6 +903,25 @@ func (m *MockClient) QuerySnapshots(ctx context.Context, filters []any) ([]tnsap
 	return result, nil
 }
 
+// QuerySnapshotIDs mocks zfs.snapshot.query with select: ["id"].
+// Returns only snapshot IDs to minimize response size.
+func (m *MockClient) QuerySnapshotIDs(ctx context.Context, filters []any) ([]string, error) {
+	m.logCall("QuerySnapshotIDs", filters)
+
+	m.mu.Lock()
+	defer m.mu.Unlock()
+
+	var ids []string
+	for _, snap := range m.snapshots {
+		if !matchesSnapshotFilters(snap, filters) {
+			continue
+		}
+		ids = append(ids, snap.ID)
+	}
+
+	return ids, nil
+}
+
 // matchesSnapshotFilters checks if a snapshot matches the provided filters.
 //
 //nolint:goconst // Filter field names are used locally in mock filter functions.