From e48f301e95c702d31cf4cb305cfee72f882aade7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Botond=20D=C3=A9nes?= Date: Tue, 29 Oct 2019 17:17:09 +0200 Subject: [PATCH] repair: repair_cf_range(): extract result of local checksum calculation only once MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The loop that collects the result of the checksum calculations and logs any errors. The error logging includes `checksums[0]` which corresponds to the checksum calculation on the local node. This violates the assumption of the code following the loop, which assumes that the future of `checksums[0]` is intact after the loop terminates. However this is only true when the checksum calculation is successful and is false when it fails, as in this case the loop extracts the error and logs it. When the code after the loop checks again whether said calculation failed, it will get a false negative and will go ahead and attempt to extract the value, triggering an assert failure. Fix by making sure that even in the case of failed checksum calculation, the result of `checksum[0]` is extracted only once. Fixes: #5238 Signed-off-by: Botond Dénes Message-Id: <20191029151709.90986-1-bdenes@scylladb.com> --- repair/repair.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/repair/repair.cc b/repair/repair.cc index 6ba0aa2697..de909af0db 100644 --- a/repair/repair.cc +++ b/repair/repair.cc @@ -796,8 +796,10 @@ static future<> repair_cf_range(repair_info& ri, // still do our best to repair available replicas. std::vector live_neighbors; std::vector live_neighbors_checksum; + bool local_checksum_failed = false; for (unsigned i = 0; i < checksums.size(); i++) { if (checksums[i].failed()) { + local_checksum_failed |= (i == 0); rlogger.warn( "Checksum of ks={}, table={}, range={} on {} failed: {}", ri.keyspace, cf, range, @@ -813,7 +815,7 @@ static future<> repair_cf_range(repair_info& ri, live_neighbors_checksum.push_back(checksums[i].get0()); } } - if (checksums[0].failed() || live_neighbors.empty()) { + if (local_checksum_failed || live_neighbors.empty()) { return make_ready_future<>(); } // If one of the available checksums is different, repair