nodetool clearsnapshot can crash a node

[glommer]

This is Scylla's bug tracker, to be used for reporting bugs only.
If you have a question about Scylla, and not a bug, please ask it in
our mailing-list at scylladb-dev@googlegroups.com or in our slack channel.

• I have read the disclaimer above, and I am reporting a suspected malfunction in Scylla.

We saw a node crashing today with nodetool clearsnapshot being called. After investigation, the reason is that nodetool clearsnapshot ws called at the same time a new snapshot was created with the same tag.

nodetool clearsnapshot can't delete all files in the directory, because new files had by then been created in that directory, and crashes on I/O error.

[glommer]

There are, in my opinion, many problems with allowing those operations to proceed in parallel. Even if we fix the code not to crash and return an error on directory non-empty, the moment they do any amount of work in parallel I think the result of the operation becomes undefined. Some files in the snapshot may have been deleted by clear, for example, and a user may then not be able to properly restore from the backup if this snapshot was used to generate a backup.

Moreover, although we could lock at the granularity of a keyspace or column family, I think we should use a big hammer here and lock the entire snapshot creation/deletion to avoid surprises (for example, if a user requests creation of a snapshot for all keyspaces, and another process requests clear of a single keyspace)

[slivne]

the error we get on not being able to remove a directory is ..." seastar - Exceptional future ignored: storage_io_error (Storage I/O error: 39: filesystem error: remove failed: Directory not empty [..."

not disputing the "big hammer" approach above - lets split this out into separate issues

nodetool clearsnapshot should not crash a node on not being able to remove a directory and should return an error

[glommer]

yes, that exception should return gracefully to the caller.

[slivne]

91b71a0 does not solve the issue of the exception bringing down the node

[avikivity]

Deferring backport decision until we have more mileage with this.

[slivne]

We still need to fix

The error we get on not being able to remove a directory is ..." seastar - Exceptional future ignored: storage_io_error (Storage I/O error: 39: filesystem error: remove failed: Directory not empty [..."

[bhalevy]

Related commits that help synchronization of snapshot operations and preventing hitting ENOENT
88d2486 sstables: Synchronize deletion of SSTables in resharding with other operations
682fb3a api: storage_service: serialize true_snapshot_size

[slivne]

closing this issue - we fixed the main paths - if there is still a problem - please reopen