scylladb / scylladb

NoSQL data store using the seastar framework, compatible with Apache Cassandra
http://scylladb.com
GNU Affero General Public License v3.0
13.57k stars 1.29k forks source link

[scylla sstable validate-checksums]: return 0 only if all sstables were scanned and no malformed sstables were found #16418

Open vladzcloudius opened 11 months ago

vladzcloudius commented 11 months ago

HEAD: 65e42e4166cef5acafa7ce7f884d6cc2b55adc35

Description Today scylla sstable validate-checksums if it succeeds to run will always return 0 regardless if malformed sstables were found or not. Instead it relies on the caller to parse the output.

This is not a standard Linux tool semantics which makes it hard(er) to automate. A standard semantics for such tools (e.g. grep) is to return an non-zero exit code if the required criteria was not met. In case of scylla sstable validate-checksums a "criteria" is "all sstables were validated and found well-formed". Hence a standard semantics would be to return 0 only when the criteria above is met and a non-zero in every other case. A tool can and should return different non-zero values for different error cases like:

For a reference, here is a corresponding section from the grep man page:

EXIT STATUS
       Normally the exit status is 0 if a line is selected, 1 if no lines were selected, and 2 if an error occurred.  However, if the -q or --quiet or --silent is used and a line is selected, the exit status is 0 even  if  an
       error occurred.

For scylla sstable validate-chechsum "a line is selected" == "all sstables were validated and found well-formed". Do s/a line is selected/all sstables were validated and found well-formed/ in the greps "EXIT STATUS" description above and you are going to get a perfect Linux semantics.

vladzcloudius commented 11 months ago

@denesb also suggested to implement a -q option which would make the tool behave as it does today however I personally don't see too much benefit of having it since a non-zero exit code can be easily "eaten up" by a construct like this one:

 scylla sstable validate-checksums ... || true