DONOTMERGE: merge leveldb upstream

[laanwj]

This merges the current upstream commit 0c40829 of leveldb into our tree. Closes #25.

Significant changes / points for review:

• Hardware-accelerated crc32c handling is no longer built-in. It uses the crc32c library. See 5c39524 .

  • I reverted our changes in the affected code.

  • This is build system work for bitcoin. Are we going to subtree crc32c? I think we need to, to be self-contained, and also because there are hardly distribution packages for it.

  • cool: google's crc32 library implements CRC32C on some more platforms than just x86 as before, for ex. ARM

• port_posix[_sse].(h|cc) is no longer used, it has been replaced by header-only port_stdcxx.h (based on C++11 threading primitives). The posix file, as well as build_detect_platform still lingers along but are not used by the CMake build system (note this is about port_ which implements threading functions, not env_ that implements file functions).

  • Need to investigate whether port_stdcxx.h is sufficient on windows. Possibly, we want to keep using port_win.h there. Selection happens in port.h. I have not tried to build for windows yet.

• I had to re-implement two changes (see separate commits):

  • Do not crash if filesystem can't fsync
  • Increase maximum read-only mmap()s used from 1000 to 4096 on 64-bit systems
    • We might be able to drop this patch completely, if we're willing to use a test interface SetReadOnlyMMapLimit:

// A helper for the POSIX Env to facilitate testing.
class EnvPosixTestHelper {
 private:
  friend class EnvPosixTest;

  // Set the maximum number of read-only files that will be opened.
  // Must be called before creating an Env.
  static void SetReadOnlyFDLimit(int limit);

  // Set the maximum number of read-only files that will be mapped via mmap.
  // Must be called before creating an Env.
  static void SetReadOnlyMMapLimit(int limit);
};

• There's a native env port for windows included now! util/env_windows.cc, similar to our own env_win.cc.

  • TODO: investigate the differences

• The configuration system changed. There's now port_config.h generated by CMake that defines a few environment specific parameters:

  • HAVE_FDATASYNC Define to 1 if you have a definition for fdatasync() in <unistd.h>.
  • HAVE_FULLFSYNC Define to 1 if you have a definition for F_FULLFSYNC in <fcntl.h>.
  • HAVE_O_CLOEXEC Define to 1 if you have a definition for O_CLOEXEC in <fcntl.h>.
  • HAVE_CRC32C Define to 1 if you have Google CRC32C.
  • HAVE_SNAPPY Define to 1 if you have Google Snappy. (should be hardwired to 0, obviously)
  • LEVELDB_IS_BIG_ENDIAN Define to 1 if your processor stores words with the most significant byte first (like Motorola and SPARC, unlike Intel and VAX).

[sipa]

Thanks for working on this. I suspect that port_stdcxx.h is actually sufficient on Windows.

[maflcko]

Thanks! Concept ACK

[laanwj]

After some discussion on IRC I'm thinking of re-starting as a patch stack on upstream. It looks like pretty much all our changes have been subsumed, apart from the things I've already had to locate and reimplement 😄

[laanwj]

I pushed a series of reverts. The diff with upstream is only 336 lines now: I'll just post it here:

Diff

diff --git a/db/db_impl.cc b/db/db_impl.cc
index 95e2bb4107d03a77c58c72af3659aafabeb3b4db..65e31724bcec1b6e80480d928f129b21be59fba1 100644
--- a/db/db_impl.cc
+++ b/db/db_impl.cc
@@ -428,7 +428,7 @@ Status DBImpl::RecoverLogFile(uint64_t log_number, bool last_log,
   while (reader.ReadRecord(&record, &scratch) && status.ok()) {
     if (record.size() < 12) {
       reporter.Corruption(record.size(),
-                          Status::Corruption("log record too small"));
+                          Status::Corruption("log record too small", fname));
       continue;
     }
     WriteBatchInternal::SetContents(&batch, record);
diff --git a/db/db_test.cc b/db/db_test.cc
index 9a8faf100672edb816f6d94251ee7c8039c7f559..beb1d3bdef61d6a885061c8eb95f1c2d13dd0d2a 100644
--- a/db/db_test.cc
+++ b/db/db_test.cc
@@ -158,6 +158,7 @@ class SpecialEnv : public EnvWrapper {
         }
         return base_->Sync();
       }
+      std::string GetName() const override { return ""; }
     };
     class ManifestFile : public WritableFile {
      private:
@@ -183,6 +184,7 @@ class SpecialEnv : public EnvWrapper {
           return base_->Sync();
         }
       }
+      std::string GetName() const override { return ""; }
     };
 
     if (non_writable_.load(std::memory_order_acquire)) {
@@ -216,6 +218,7 @@ class SpecialEnv : public EnvWrapper {
         counter_->Increment();
         return target_->Read(offset, n, result, scratch);
       }
+      std::string GetName() const override { return ""; }
     };
 
     Status s = target()->NewRandomAccessFile(f, r);
diff --git a/db/fault_injection_test.cc b/db/fault_injection_test.cc
index 5b31bb8053f2650c213b2152d0d78551dc86e613..bf705cb60f2431f8b6d163a5fd94656b47eda431 100644
--- a/db/fault_injection_test.cc
+++ b/db/fault_injection_test.cc
@@ -114,6 +114,7 @@ class TestWritableFile : public WritableFile {
   Status Close() override;
   Status Flush() override;
   Status Sync() override;
+  std::string GetName() const override { return ""; }
 
  private:
   FileState state_;
diff --git a/db/leveldbutil.cc b/db/leveldbutil.cc
index 55cdcc5772236a042a63a25fd260fcc1e412ec69..f4ecbc7ab4cec715b5d391994492b17452c4386c 100644
--- a/db/leveldbutil.cc
+++ b/db/leveldbutil.cc
@@ -17,9 +17,10 @@ class StdoutPrinter : public WritableFile {
     fwrite(data.data(), 1, data.size(), stdout);
     return Status::OK();
   }
-  Status Close() override { return Status::OK(); }
-  Status Flush() override { return Status::OK(); }
-  Status Sync() override { return Status::OK(); }
+  Status Close() { return Status::OK(); }
+  Status Flush() { return Status::OK(); }
+  Status Sync() { return Status::OK(); }
+  std::string GetName() const override { return "[stdout]"; }
 };
 
 bool HandleDumpCommand(Env* env, char** files, int num) {
diff --git a/db/log_reader.cc b/db/log_reader.cc
index b770fee1a0de25d64e180620bf7d15fc2d770394..1ccfb7b34aa85a6f37b5e515ff10e09b786c642a 100644
--- a/db/log_reader.cc
+++ b/db/log_reader.cc
@@ -176,7 +176,7 @@ bool Reader::ReadRecord(Slice* record, std::string* scratch) {
 uint64_t Reader::LastRecordOffset() { return last_record_offset_; }
 
 void Reader::ReportCorruption(uint64_t bytes, const char* reason) {
-  ReportDrop(bytes, Status::Corruption(reason));
+  ReportDrop(bytes, Status::Corruption(reason, file_->GetName()));
 }
 
 void Reader::ReportDrop(uint64_t bytes, const Status& reason) {
diff --git a/db/log_test.cc b/db/log_test.cc
index 0e31648cc8ffcd2e3d8ff4e3abd4917bfe401c32..41fc043068dcec4d587f36d80bff5013761e1149 100644
--- a/db/log_test.cc
+++ b/db/log_test.cc
@@ -168,6 +168,7 @@ class LogTest {
       contents_.append(slice.data(), slice.size());
       return Status::OK();
     }
+    std::string GetName() const override { return ""; }
 
     std::string contents_;
   };
@@ -204,6 +205,7 @@ class LogTest {
 
       return Status::OK();
     }
+    std::string GetName() const { return ""; }
 
     Slice contents_;
     bool force_error_;
diff --git a/db/repair.cc b/db/repair.cc
index d9d12ba556f0e78bd50218c4f9b06c9283fdd0a8..04847c3bbfb73d40f3c2ebb90ef701fd957ea3b1 100644
--- a/db/repair.cc
+++ b/db/repair.cc
@@ -183,7 +183,7 @@ class Repairer {
     while (reader.ReadRecord(&record, &scratch)) {
       if (record.size() < 12) {
         reporter.Corruption(record.size(),
-                            Status::Corruption("log record too small"));
+                            Status::Corruption("log record too small", logname));
         continue;
       }
       WriteBatchInternal::SetContents(&batch, record);
diff --git a/helpers/memenv/memenv.cc b/helpers/memenv/memenv.cc
index 31d2bc0f6f63a159ad30b129603716f35336cd79..95fa4ae8beacee380c2a851942029bf10b057b2a 100644
--- a/helpers/memenv/memenv.cc
+++ b/helpers/memenv/memenv.cc
@@ -178,6 +178,7 @@ class SequentialFileImpl : public SequentialFile {
     return Status::OK();
   }
 
+  virtual std::string GetName() const { return "[memenv]"; }
  private:
   FileState* file_;
   uint64_t pos_;
@@ -194,6 +195,7 @@ class RandomAccessFileImpl : public RandomAccessFile {
     return file_->Read(offset, n, result, scratch);
   }
 
+  virtual std::string GetName() const { return "[memenv]"; }
  private:
   FileState* file_;
 };
@@ -210,6 +212,7 @@ class WritableFileImpl : public WritableFile {
   Status Flush() override { return Status::OK(); }
   Status Sync() override { return Status::OK(); }
 
+  virtual std::string GetName() const { return "[memenv]"; }
  private:
   FileState* file_;
 };
diff --git a/include/leveldb/env.h b/include/leveldb/env.h
index 112fe964fb1cb2ce3e208ba64b92a82147279266..96c21b3966c4c35a6012450474f12868dc4e006b 100644
--- a/include/leveldb/env.h
+++ b/include/leveldb/env.h
@@ -217,6 +217,9 @@ class LEVELDB_EXPORT SequentialFile {
   //
   // REQUIRES: External synchronization
   virtual Status Skip(uint64_t n) = 0;
+
+  // Get a name for the file, only for error reporting
+  virtual std::string GetName() const = 0;
 };
 
 // A file abstraction for randomly reading the contents of a file.
@@ -240,6 +243,9 @@ class LEVELDB_EXPORT RandomAccessFile {
   // Safe for concurrent use by multiple threads.
   virtual Status Read(uint64_t offset, size_t n, Slice* result,
                       char* scratch) const = 0;
+
+  // Get a name for the file, only for error reporting
+  virtual std::string GetName() const = 0;
 };
 
 // A file abstraction for sequential writing.  The implementation
@@ -258,6 +264,9 @@ class LEVELDB_EXPORT WritableFile {
   virtual Status Close() = 0;
   virtual Status Flush() = 0;
   virtual Status Sync() = 0;
+
+  // Get a name for the file, only for error reporting
+  virtual std::string GetName() const = 0;
 };
 
 // An interface for writing log messages.
diff --git a/table/format.cc b/table/format.cc
index e1839779e4bab2d595fd996241da4a4184db2f2b..a3d67de2e41d4ffa16e927432ee389568cfda32f 100644
--- a/table/format.cc
+++ b/table/format.cc
@@ -79,7 +79,7 @@ Status ReadBlock(RandomAccessFile* file, const ReadOptions& options,
   }
   if (contents.size() != n + kBlockTrailerSize) {
     delete[] buf;
-    return Status::Corruption("truncated block read");
+    return Status::Corruption("truncated block read", file->GetName());
   }
 
   // Check the crc of the type and the block contents
@@ -89,7 +89,7 @@ Status ReadBlock(RandomAccessFile* file, const ReadOptions& options,
     const uint32_t actual = crc32c::Value(data, n + 1);
     if (actual != crc) {
       delete[] buf;
-      s = Status::Corruption("block checksum mismatch");
+      s = Status::Corruption("block checksum mismatch", file->GetName());
       return s;
     }
   }
@@ -116,13 +116,13 @@ Status ReadBlock(RandomAccessFile* file, const ReadOptions& options,
       size_t ulength = 0;
       if (!port::Snappy_GetUncompressedLength(data, n, &ulength)) {
         delete[] buf;
-        return Status::Corruption("corrupted compressed block contents");
+        return Status::Corruption("corrupted compressed block contents", file->GetName());
       }
       char* ubuf = new char[ulength];
       if (!port::Snappy_Uncompress(data, n, ubuf)) {
         delete[] buf;
         delete[] ubuf;
-        return Status::Corruption("corrupted compressed block contents");
+        return Status::Corruption("corrupted compressed block contents", file->GetName());
       }
       delete[] buf;
       result->data = Slice(ubuf, ulength);
@@ -132,7 +132,7 @@ Status ReadBlock(RandomAccessFile* file, const ReadOptions& options,
     }
     default:
       delete[] buf;
-      return Status::Corruption("bad block type");
+      return Status::Corruption("bad block type", file->GetName());
   }
 
   return Status::OK();
diff --git a/table/table_test.cc b/table/table_test.cc
index f689a276577e02c86c1579dd772ecbe4aa932e9a..17aaea2f9e8b48484c4ba129a656854e4681596d 100644
--- a/table/table_test.cc
+++ b/table/table_test.cc
@@ -102,6 +102,7 @@ class StringSink : public WritableFile {
     return Status::OK();
   }
 
+  std::string GetName() const override { return ""; }
  private:
   std::string contents_;
 };
@@ -128,6 +129,7 @@ class StringSource : public RandomAccessFile {
     return Status::OK();
   }
 
+  std::string GetName() const { return ""; }
  private:
   std::string contents_;
 };
diff --git a/util/env_posix.cc b/util/env_posix.cc
index 00ca9aedef58b2bd6e2439cc52dee37fffb67c97..9f5863a0f35abdfe5c702fd0df2100dcab0ce70a 100644
--- a/util/env_posix.cc
+++ b/util/env_posix.cc
@@ -42,8 +42,8 @@ namespace {
 // Set by EnvPosixTestHelper::SetReadOnlyMMapLimit() and MaxOpenFiles().
 int g_open_read_only_file_limit = -1;
 
-// Up to 1000 mmap regions for 64-bit binaries; none for 32-bit.
-constexpr const int kDefaultMmapLimit = (sizeof(void*) >= 8) ? 1000 : 0;
+// Up to 4096 mmap regions for 64-bit binaries; none for 32-bit.
+constexpr const int kDefaultMmapLimit = (sizeof(void*) >= 8) ? 4096 : 0;
 
 // Can be set using EnvPosixTestHelper::SetReadOnlyMMapLimit().
 int g_mmap_limit = kDefaultMmapLimit;
@@ -135,6 +135,8 @@ class PosixSequentialFile final : public SequentialFile {
     return Status::OK();
   }
 
+  virtual std::string GetName() const override { return filename_; }
+
  private:
   const int fd_;
   const std::string filename_;
@@ -195,6 +197,8 @@ class PosixRandomAccessFile final : public RandomAccessFile {
     return status;
   }
 
+  virtual std::string GetName() const override { return filename_; }
+
  private:
   const bool has_permanent_fd_;  // If false, the file is opened on every read.
   const int fd_;                 // -1 if has_permanent_fd_ is false.
@@ -239,6 +243,8 @@ class PosixMmapReadableFile final : public RandomAccessFile {
     return Status::OK();
   }
 
+  virtual std::string GetName() const override { return filename_; }
+
  private:
   char* const mmap_base_;
   const size_t length_;
@@ -319,7 +325,7 @@ class PosixWritableFile final : public WritableFile {
       return status;
     }
 
-    return SyncFd(fd_, filename_);
+    return SyncFd(fd_, filename_, false);
   }
 
  private:
@@ -354,7 +360,7 @@ class PosixWritableFile final : public WritableFile {
     if (fd < 0) {
       status = PosixError(dirname_, errno);
     } else {
-      status = SyncFd(fd, dirname_);
+      status = SyncFd(fd, dirname_, true);
       ::close(fd);
     }
     return status;
@@ -366,7 +372,7 @@ class PosixWritableFile final : public WritableFile {
   //
   // The path argument is only used to populate the description string in the
   // returned Status if an error occurs.
-  static Status SyncFd(int fd, const std::string& fd_path) {
+  static Status SyncFd(int fd, const std::string& fd_path, bool syncing_dir) {
 #if HAVE_FULLFSYNC
     // On macOS and iOS, fsync() doesn't guarantee durability past power
     // failures. fcntl(F_FULLFSYNC) is required for that purpose. Some
@@ -386,6 +392,11 @@ class PosixWritableFile final : public WritableFile {
     if (sync_success) {
       return Status::OK();
     }
+    // Do not crash if filesystem can't fsync directories
+    // (see https://github.com/bitcoin/bitcoin/pull/10000)
+    if (syncing_dir && errno == EINVAL) {
+      return Status::OK();
+    }
     return PosixError(fd_path, errno);
   }
 
@@ -426,6 +437,8 @@ class PosixWritableFile final : public WritableFile {
     return Basename(filename).starts_with("MANIFEST");
   }
 
+  virtual std::string GetName() const override { return filename_; }
+
   // buf_[0, pos_ - 1] contains data to be written to fd_.
   char buf_[kWritableFileBufferSize];
   size_t pos_;

This is only the following patches:

• re-apply: Add filename to corruption errors
• re-apply: Do not crash if filesystem can't fsync
• re-apply: Increase maximum read-only mmap()s used from 1000 to 4096 on 64-bit systems

I'll need to patch env_windows.cpp for the GetName so it's expected to grow by another three lines or so.

[laanwj]

This PR is no longer relevant, except for discussion, I started over at: https://github.com/bitcoin-core/leveldb/tree/bitcoin-fork-new