From e88209ad0a00bd0d0fdccd02e6ad0162c5286be5 Mon Sep 17 00:00:00 2001 From: Andrew Kryczka Date: Thu, 26 Sep 2019 15:27:44 -0700 Subject: [PATCH] relax L0 seqno invariant for ingested files This is necessary to revert https://github.com/cockroachdb/cockroach/pull/39562 to support RocksDB built without `-DNDEBUG`. Consecutive files ingested to L0 may be assigned the same seqnum, and this PR updates the LSM validation to allow that. There is a longer-term fix in https://github.com/facebook/rocksdb/pull/5539. We should take that later, maybe after the 19.2 release. Test Plan: ran some tests that previously failed with corruption error like `ENABLE_ROCKSDB_ASSERTIONS=1 make PKG=github.com/cockroachdb/cockroach/pkg/ccl/backupccl testrace`; verified they succeed now. --- db/version_builder.cc | 39 +++++++++++++++++++++++---------------- 1 file changed, 23 insertions(+), 16 deletions(-) diff --git a/db/version_builder.cc b/db/version_builder.cc index 598469a8e24..9be59829315 100644 --- a/db/version_builder.cc +++ b/db/version_builder.cc @@ -169,22 +169,29 @@ class VersionBuilder::Rep { if (f2->fd.smallest_seqno == f2->fd.largest_seqno) { // This is an external file that we ingested - SequenceNumber external_file_seqno = f2->fd.smallest_seqno; - if (!(external_file_seqno < f1->fd.largest_seqno || - external_file_seqno == 0)) { - fprintf(stderr, - "L0 file with seqno %" PRIu64 " %" PRIu64 - " vs. file with global_seqno %" PRIu64 "\n", - f1->fd.smallest_seqno, f1->fd.largest_seqno, - external_file_seqno); - return Status::Corruption("L0 file with seqno " + - NumberToString(f1->fd.smallest_seqno) + - " " + - NumberToString(f1->fd.largest_seqno) + - " vs. file with global_seqno" + - NumberToString(external_file_seqno) + - " with fileNumber " + - NumberToString(f1->fd.GetNumber())); + SequenceNumber f2_external_file_seqno = f2->fd.smallest_seqno; + bool is_f1_ingested = f1->fd.smallest_seqno == f1->fd.largest_seqno; + if (f2_external_file_seqno != 0) { + // This file's seqnos were not zeroed so we can use its seqnos to + // determine it is ordered properly. + if (!(f1->fd.largest_seqno > f2_external_file_seqno || + (is_f1_ingested && f1->fd.largest_seqno == f2_external_file_seqno))) { + // f1 should be newer than f2. Equal seqnos are only allowed + // when both files were ingested. + fprintf(stderr, + "L0 file with seqno %" PRIu64 " %" PRIu64 + " vs. file with global_seqno %" PRIu64 "\n", + f1->fd.smallest_seqno, f1->fd.largest_seqno, + f2_external_file_seqno); + return Status::Corruption("L0 file with seqno " + + NumberToString(f1->fd.smallest_seqno) + + " " + + NumberToString(f1->fd.largest_seqno) + + " vs. file with global_seqno" + + NumberToString(f2_external_file_seqno) + + " with fileNumber " + + NumberToString(f1->fd.GetNumber())); + } } } else if (f1->fd.smallest_seqno <= f2->fd.smallest_seqno) { fprintf(stderr,