search

scylladb / scylla-jmx

Scylla JMX proxy
GNU Affero General Public License v3.0
28 stars 52 forks source link

Testing scrubMode != "" compares references, not values #180

Open bhalevy opened 2 years ago

bhalevy commented 2 years ago

Change 70b19e6270df1ebe0494af39a2634da0513b38a6 introduce a scrubMode parameter to scrub. But, https://github.com/scylladb/scylla-jmx/blob/70b19e6270df1ebe0494af39a2634da0513b38a6/src/main/java/org/apache/cassandra/service/StorageService.java#L1805

misuses Java by comparing scrubMode to an empty string using the != operator. As @haaawk noted, this is wrong as in Java != compares references, not values.

avikivity commented 2 years ago

Please in the future file issues in a way that indicates user impact.

bhalevy commented 2 years ago

Please in the future file issues in a way that indicates user impact.

Agreed. I'm not exactly sure what the user impact is in this case. I think that the user might pass an empty scrub mode and we won't detect it here.

But storage_service scrub api handles an empty scrub_mode as the legacy api and it looked at the skip_corrupted option in this case and in any case it defaults to the ABORT scrub_mode which is exactly what will happen if the test condition detected the empty string correctly.

So bottom line, modulo testing I think there should be no user impact.