PLAT-3457: clean up options file code processing of file_preload (#37)

* erase temp file option creation fails. unknown if temp file exists. * create option file output and input mapping for file_preload * add file_preload to column family options test (and clean up two lines of db options test * make clang-format happy for CircleCI test
stardog-union · Jan 4, 2022 · 550cd54 · 550cd54
1 parent 30ad741
commit 550cd54
Show file tree

Hide file tree

Showing 4 changed files with 53 additions and 33 deletions.
diff --git a/db/db_impl/db_impl.cc b/db/db_impl/db_impl.cc
@@ -4089,6 +4089,10 @@ Status DBImpl::WriteOptionsFile(bool need_mutex_lock,
 
   if (s.ok()) {
     s = RenameTempFileToOptionsFile(file_name);
+  } else {
+    // do not accumulate failed files
+    //  (could be hundreds with bad options code)
+    (void)env_->DeleteFile(file_name).ok();
   }
   // restore lock
   if (!need_mutex_lock) {

diff --git a/options/options_helper.cc b/options/options_helper.cc
@@ -149,10 +149,8 @@ DBOptions BuildDBOptions(const ImmutableDBOptions& immutable_db_options,
       immutable_db_options.avoid_flush_during_recovery;
   options.avoid_flush_during_shutdown =
       mutable_db_options.avoid_flush_during_shutdown;
-  options.allow_ingest_behind =
-      immutable_db_options.allow_ingest_behind;
-  options.preserve_deletes =
-      immutable_db_options.preserve_deletes;
+  options.allow_ingest_behind = immutable_db_options.allow_ingest_behind;
+  options.preserve_deletes = immutable_db_options.preserve_deletes;
   options.two_write_queues = immutable_db_options.two_write_queues;
   options.manual_wal_flush = immutable_db_options.manual_wal_flush;
   options.atomic_flush = immutable_db_options.atomic_flush;
@@ -276,6 +274,11 @@ std::map<CompactionStopStyle, std::string>
         {kCompactionStopStyleSimilarSize, "kCompactionStopStyleSimilarSize"},
         {kCompactionStopStyleTotalSize, "kCompactionStopStyleTotalSize"}};
 
+std::map<FilePreload, std::string> OptionsHelper::file_preload_to_string = {
+    {FilePreload::kFilePreloadWithPinning, "kFilePreloadWithPinning"},
+    {FilePreload::kFilePreloadWithoutPinning, "kFilePreloadWithoutPinning"},
+    {FilePreload::kFilePreloadDisabled, "kFilePreloadDisabled"}};
+
 std::unordered_map<std::string, ChecksumType>
     OptionsHelper::checksum_type_string_map = {{"kNoChecksum", kNoChecksum},
                                                {"kCRC32c", kCRC32c},
@@ -395,7 +398,8 @@ static bool ParseOptionHelper(char* opt_address, const OptionType& opt_type,
       *reinterpret_cast<uint32_t*>(opt_address) = ParseUint32(value);
       break;
     case OptionType::kUInt64T:
-      PutUnaligned(reinterpret_cast<uint64_t*>(opt_address), ParseUint64(value));
+      PutUnaligned(reinterpret_cast<uint64_t*>(opt_address),
+                   ParseUint64(value));
       break;
     case OptionType::kSizeT:
       PutUnaligned(reinterpret_cast<size_t*>(opt_address), ParseSizeT(value));
@@ -418,6 +422,10 @@ static bool ParseOptionHelper(char* opt_address, const OptionType& opt_type,
       return ParseEnum<CompressionType>(
           compression_type_string_map, value,
           reinterpret_cast<CompressionType*>(opt_address));
+    case OptionType::kFilePreload:
+      return ParseEnum<FilePreload>(
+          file_preload_string_map, value,
+          reinterpret_cast<FilePreload*>(opt_address));
     case OptionType::kSliceTransform:
       return ParseSliceTransform(
           value, reinterpret_cast<std::shared_ptr<const SliceTransform>*>(
@@ -443,7 +451,6 @@ static bool ParseOptionHelper(char* opt_address, const OptionType& opt_type,
 bool SerializeSingleOptionHelper(const char* opt_address,
                                  const OptionType opt_type,
                                  std::string* value) {
-
   assert(value);
   switch (opt_type) {
     case OptionType::kBoolean:
@@ -455,33 +462,27 @@ bool SerializeSingleOptionHelper(const char* opt_address,
     case OptionType::kInt32T:
       *value = ToString(*(reinterpret_cast<const int32_t*>(opt_address)));
       break;
-    case OptionType::kInt64T:
-      {
-        int64_t v;
-        GetUnaligned(reinterpret_cast<const int64_t*>(opt_address), &v);
-        *value = ToString(v);
-      }
-      break;
+    case OptionType::kInt64T: {
+      int64_t v;
+      GetUnaligned(reinterpret_cast<const int64_t*>(opt_address), &v);
+      *value = ToString(v);
+    } break;
     case OptionType::kUInt:
       *value = ToString(*(reinterpret_cast<const unsigned int*>(opt_address)));
       break;
     case OptionType::kUInt32T:
       *value = ToString(*(reinterpret_cast<const uint32_t*>(opt_address)));
       break;
-    case OptionType::kUInt64T:
-      {
-        uint64_t v;
-        GetUnaligned(reinterpret_cast<const uint64_t*>(opt_address), &v);
-        *value = ToString(v);
-      }
-      break;
-    case OptionType::kSizeT:
-      {
-        size_t v;
-        GetUnaligned(reinterpret_cast<const size_t*>(opt_address), &v);
-        *value = ToString(v);
-      }
-      break;
+    case OptionType::kUInt64T: {
+      uint64_t v;
+      GetUnaligned(reinterpret_cast<const uint64_t*>(opt_address), &v);
+      *value = ToString(v);
+    } break;
+    case OptionType::kSizeT: {
+      size_t v;
+      GetUnaligned(reinterpret_cast<const size_t*>(opt_address), &v);
+      *value = ToString(v);
+    } break;
     case OptionType::kDouble:
       *value = ToString(*(reinterpret_cast<const double*>(opt_address)));
       break;
@@ -501,6 +502,10 @@ bool SerializeSingleOptionHelper(const char* opt_address,
       return SerializeEnum<CompressionType>(
           compression_type_string_map,
           *(reinterpret_cast<const CompressionType*>(opt_address)), value);
+    case OptionType::kFilePreload:
+      return SerializeEnum<FilePreload>(
+          file_preload_string_map,
+          *(reinterpret_cast<const FilePreload*>(opt_address)), value);
     case OptionType::kSliceTransform: {
       const auto* slice_transform_ptr =
           reinterpret_cast<const std::shared_ptr<const SliceTransform>*>(
@@ -756,10 +761,9 @@ Status GetColumnFamilyOptionsFromMap(
   }
 }
 
-Status GetColumnFamilyOptionsFromString(
-    const ColumnFamilyOptions& base_options,
-    const std::string& opts_str,
-    ColumnFamilyOptions* new_options) {
+Status GetColumnFamilyOptionsFromString(const ColumnFamilyOptions& base_options,
+                                        const std::string& opts_str,
+                                        ColumnFamilyOptions* new_options) {
   ConfigOptions config_options;
   config_options.input_strings_escaped = false;
   config_options.ignore_unknown_options = false;
@@ -905,6 +909,12 @@ std::unordered_map<std::string, CompactionStopStyle>
         {"kCompactionStopStyleSimilarSize", kCompactionStopStyleSimilarSize},
         {"kCompactionStopStyleTotalSize", kCompactionStopStyleTotalSize}};
 
+std::unordered_map<std::string, FilePreload>
+    OptionsHelper::file_preload_string_map = {
+        {"kFilePreloadWithPinning", FilePreload::kFilePreloadWithPinning},
+        {"kFilePreloadWithoutPinning", FilePreload::kFilePreloadWithoutPinning},
+        {"kFilePreloadDisabled", FilePreload::kFilePreloadDisabled}};
+
 Status OptionTypeInfo::NextToken(const std::string& opts, char delimiter,
                                  size_t pos, size_t* end, std::string* token) {
   while (pos < opts.size() && isspace(opts[pos])) {
@@ -1200,6 +1210,8 @@ static bool AreOptionsEqual(OptionType type, const char* this_offset,
       return IsOptionEqual<CompactionPri>(this_offset, that_offset);
     case OptionType::kCompressionType:
       return IsOptionEqual<CompressionType>(this_offset, that_offset);
+    case OptionType::kFilePreload:
+      return IsOptionEqual<FilePreload>(this_offset, that_offset);
     case OptionType::kChecksumType:
       return IsOptionEqual<ChecksumType>(this_offset, that_offset);
     case OptionType::kEncodingType:

diff --git a/options/options_helper.h b/options/options_helper.h
@@ -94,6 +94,8 @@ struct OptionsHelper {
   static std::unordered_map<std::string, CompactionPri>
       compaction_pri_string_map;
 #endif  // !ROCKSDB_LITE
+  static std::map<FilePreload, std::string> file_preload_to_string;
+  static std::unordered_map<std::string, FilePreload> file_preload_string_map;
 };
 
 // Some aliasing
@@ -114,5 +116,6 @@ static auto& compaction_style_string_map =
 static auto& compaction_pri_string_map =
     OptionsHelper::compaction_pri_string_map;
 #endif  // !ROCKSDB_LITE
+static auto& file_preload_string_map = OptionsHelper::file_preload_string_map;
 
 }  // namespace ROCKSDB_NAMESPACE
diff --git a/options/options_settable_test.cc b/options/options_settable_test.cc
@@ -336,9 +336,9 @@ TEST_F(OptionsSettableTest, DBOptionsAllFieldsSettable) {
                              "write_dbid_to_manifest=false;"
                              "best_efforts_recovery=false;"
                              "max_bgerror_resume_count=2;"
-                             "bgerror_resume_retry_interval=1000000"
+                             "bgerror_resume_retry_interval=1000000;"
                              "db_host_id=hostname;"
-                             "allow_data_in_errors=false",
+                             "allow_data_in_errors=false;",
                              new_options));
 
   ASSERT_EQ(unset_bytes_base, NumUnsetBytes(new_options_ptr, sizeof(DBOptions),
@@ -510,6 +510,7 @@ TEST_F(OptionsSettableTest, ColumnFamilyOptionsAllFieldsSettable) {
       "blob_compression_type=kBZip2Compression;"
       "enable_blob_garbage_collection=true;"
       "blob_garbage_collection_age_cutoff=0.5;"
+      "file_preload=kFilePreloadWithPinning;"
       "compaction_options_fifo={max_table_files_size=3;allow_"
       "compaction=false;};",
       new_options));