Change Configurable::SerializeOptions method to return a map (#619)

Changes the Configurable::SerializeOptions method to return a map of options. This map is equivalent to the input map for the ConfigureOptions method. By returning a map, it will be possible in the future to add different output representations (such as DUMP, JSON, Properties) formats to the string representations of the configurable objects. This PR is a first-step to a few additional changes. For example, is a gateway to: Only serializing changed options; Allowing different string formats for options (e.g. Default, Properties, Log/Dump, JSON, etc)
speedb-io · Oct 19, 2023 · dc39061 · dc39061
1 parent 9e2b4d3
commit dc39061
Show file tree

Hide file tree

Showing 25 changed files with 513 additions and 223 deletions.
diff --git a/HISTORY.md b/HISTORY.md
@@ -6,6 +6,7 @@
 
 ### New Features
 * Non-Blocking Manual Compaction (CompactRange()) - Support non-blocking manual compactions by setting a new CompactRangeOptions option (async_completion_cb). When set, the CompactRange() call will return control to the caller immediately. The manual compaction iteslf will be performed in an internally created thread. The manual compaction will ALWAYS call the specified callback upon completion and provide the completion status (#597).
+* Change the internal Configurable API SerializeOptions to return UserProperties (instead of the final string representation).  Added ToString methods to the ConfigurableOptions class to complete the serialization of Options properties.
 
 
 ### Enhancements

diff --git a/db/compaction/compaction_service_job.cc b/db/compaction/compaction_service_job.cc
@@ -1,3 +1,17 @@
+// Copyright (C) 2022 Speedb Ltd. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
 //  Copyright (c) Meta Platforms, Inc. and affiliates.
 //
 //  This source code is licensed under both the GPLv2 (found in the
@@ -693,8 +707,8 @@ static std::unordered_map<std::string, OptionTypeInfo> cs_result_type_info = {
         const auto status_obj = static_cast<const Status*>(addr);
         StatusSerializationAdapter adapter(*status_obj);
         std::string result;
-        Status s = OptionTypeInfo::SerializeType(opts, status_adapter_type_info,
-                                                 &adapter, &result);
+        Status s = OptionTypeInfo::TypeToString(
+            opts, "", status_adapter_type_info, &adapter, &result);
         *value = "{" + result + "}";
         return s;
       },
@@ -770,7 +784,13 @@ Status CompactionServiceInput::Write(std::string* output) {
   output->append(buf, sizeof(BinaryFormatVersion));
   ConfigOptions cf;
   cf.invoke_prepare_options = false;
-  return OptionTypeInfo::SerializeType(cf, cs_input_type_info, this, output);
+  OptionProperties props;
+  Status s =
+      OptionTypeInfo::SerializeType(cf, "", cs_input_type_info, this, &props);
+  if (s.ok()) {
+    output->append(cf.ToString("", props) + cf.delimiter);
+  }
+  return s;
 }
 
 Status CompactionServiceResult::Read(const std::string& data_str,
@@ -799,7 +819,13 @@ Status CompactionServiceResult::Write(std::string* output) {
   output->append(buf, sizeof(BinaryFormatVersion));
   ConfigOptions cf;
   cf.invoke_prepare_options = false;
-  return OptionTypeInfo::SerializeType(cf, cs_result_type_info, this, output);
+  OptionProperties props;
+  Status s =
+      OptionTypeInfo::SerializeType(cf, "", cs_result_type_info, this, &props);
+  if (s.ok()) {
+    output->append(cf.ToString("", props) + cf.delimiter);
+  }
+  return s;
 }
 
 #ifndef NDEBUG

diff --git a/db/db_basic_test.cc b/db/db_basic_test.cc
@@ -2197,6 +2197,8 @@ TEST_F(DBBasicTest, DBSetThreadAffinity) {
     CPU_ZERO(&cpuset);
     CPU_SET(0, &cpuset);
     pthread_setaffinity_np(thr, sizeof(cpu_set_t), &cpuset);
+#else
+    (void)thr;
 #endif
   };
   options.on_thread_start_callback =

diff --git a/env/composite_env.cc b/env/composite_env.cc
@@ -1,3 +1,17 @@
+// Copyright (C) 2022 Speedb Ltd. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
 // Copyright (c) 2019-present, Facebook, Inc.  All rights reserved.
 //  This source code is licensed under both the GPLv2 (found in the
 //  COPYING file in the root directory) and Apache 2.0 License
@@ -391,7 +405,7 @@ Status CompositeEnv::NewDirectory(const std::string& name,
 
 namespace {
 static std::unordered_map<std::string, OptionTypeInfo> env_wrapper_type_info = {
-    {"target",
+    {Customizable::kTargetPropName(),
      OptionTypeInfo(0, OptionType::kUnknown, OptionVerificationType::kByName,
                     OptionTypeFlags::kDontSerialize)
          .SetParseFunc([](const ConfigOptions& opts,
@@ -482,14 +496,16 @@ Status CompositeEnvWrapper::PrepareOptions(const ConfigOptions& options) {
   return Env::PrepareOptions(options);
 }
 
-std::string CompositeEnvWrapper::SerializeOptions(
-    const ConfigOptions& config_options, const std::string& header) const {
-  auto options = CompositeEnv::SerializeOptions(config_options, header);
+Status CompositeEnvWrapper::SerializeOptions(
+    const ConfigOptions& config_options, const std::string& prefix,
+    OptionProperties* props) const {
   if (target_.env != nullptr && target_.env != Env::Default()) {
-    options.append("target=");
-    options.append(target_.env->ToString(config_options));
+    props->insert({kTargetPropName(),
+                   target_.env->ToString(
+                       config_options,
+                       OptionTypeInfo::MakePrefix(prefix, kTargetPropName()))});
   }
-  return options;
+  return CompositeEnv::SerializeOptions(config_options, prefix, props);
 }
 
 EnvWrapper::EnvWrapper(Env* t) : target_(t) {
@@ -511,24 +527,17 @@ Status EnvWrapper::PrepareOptions(const ConfigOptions& options) {
   return Env::PrepareOptions(options);
 }
 
-std::string EnvWrapper::SerializeOptions(const ConfigOptions& config_options,
-                                         const std::string& header) const {
-  auto parent = Env::SerializeOptions(config_options, "");
-  if (config_options.IsShallow() || target_.env == nullptr ||
-      target_.env == Env::Default()) {
-    return parent;
-  } else {
-    std::string result = header;
-    if (!StartsWith(parent, OptionTypeInfo::kIdPropName())) {
-      result.append(OptionTypeInfo::kIdPropName()).append("=");
-    }
-    result.append(parent);
-    if (!EndsWith(result, config_options.delimiter)) {
-      result.append(config_options.delimiter);
-    }
-    result.append("target=").append(target_.env->ToString(config_options));
-    return result;
-  }
+Status EnvWrapper::SerializeOptions(const ConfigOptions& config_options,
+                                    const std::string& prefix,
+                                    OptionProperties* props) const {
+  if (!config_options.IsShallow() && target_.env != nullptr &&
+      target_.env != Env::Default()) {
+    props->insert({kTargetPropName(),
+                   target_.env->ToString(
+                       config_options,
+                       OptionTypeInfo::MakePrefix(prefix, kTargetPropName()))});
+  }
+  return Env::SerializeOptions(config_options, prefix, props);
 }
 
 }  // namespace ROCKSDB_NAMESPACE
diff --git a/env/composite_env_wrapper.h b/env/composite_env_wrapper.h
@@ -1,3 +1,17 @@
+// Copyright (C) 2022 Speedb Ltd. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
 // Copyright (c) 2019-present, Facebook, Inc.  All rights reserved.
 //  This source code is licensed under both the GPLv2 (found in the
 //  COPYING file in the root directory) and Apache 2.0 License
@@ -289,8 +303,9 @@ class CompositeEnvWrapper : public CompositeEnv {
   const Customizable* Inner() const override { return target_.env; }
 
   Status PrepareOptions(const ConfigOptions& options) override;
-  std::string SerializeOptions(const ConfigOptions& config_options,
-                               const std::string& header) const override;
+  Status SerializeOptions(const ConfigOptions& config_options,
+                          const std::string& prefix,
+                          OptionProperties* props) const override;
 
   // Return the target to which this Env forwards all calls
   Env* env_target() const { return target_.env; }

diff --git a/env/env.cc b/env/env.cc
@@ -1,3 +1,17 @@
+// Copyright (C) 2022 Speedb Ltd. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
 //  Copyright (c) 2011-present, Facebook, Inc.  All rights reserved.
 //  This source code is licensed under both the GPLv2 (found in the
 //  COPYING file in the root directory) and Apache 2.0 License
@@ -93,12 +107,14 @@ class LegacySystemClock : public SystemClock {
     return env_->TimeToString(time);
   }
 
-  std::string SerializeOptions(const ConfigOptions& /*config_options*/,
-                               const std::string& /*prefix*/) const override {
+  Status SerializeOptions(const ConfigOptions& /*config_options*/,
+                          const std::string& /*prefix*/,
+                          OptionProperties* /*options*/) const override {
     // We do not want the LegacySystemClock to appear in the serialized output.
     // This clock is an internal class for those who do not implement one and
     // would be part of the Env.  As such, do not serialize it here.
-    return "";
+    return Status::OK();
+    ;
   }
 };
 
@@ -599,13 +615,15 @@ class LegacyFileSystemWrapper : public FileSystem {
     return status_to_io_status(target_->IsDirectory(path, is_dir));
   }
 
-  std::string SerializeOptions(const ConfigOptions& /*config_options*/,
-                               const std::string& /*prefix*/) const override {
+  Status SerializeOptions(const ConfigOptions& /*config_options*/,
+                          const std::string& /*prefix*/,
+                          OptionProperties* /*options*/) const override {
     // We do not want the LegacyFileSystem to appear in the serialized output.
     // This clock is an internal class for those who do not implement one and
     // would be part of the Env.  As such, do not serialize it here.
-    return "";
+    return Status::OK();
   }
+
  private:
   Env* target_;
 };
@@ -1164,7 +1182,7 @@ const std::shared_ptr<SystemClock>& Env::GetSystemClock() const {
 }
 namespace {
 static std::unordered_map<std::string, OptionTypeInfo> sc_wrapper_type_info = {
-    {"target",
+    {Customizable::kTargetPropName(),
      OptionTypeInfo::AsCustomSharedPtr<SystemClock>(
          0, OptionVerificationType::kByName, OptionTypeFlags::kDontSerialize)},
 };
@@ -1182,24 +1200,17 @@ Status SystemClockWrapper::PrepareOptions(const ConfigOptions& options) {
   return SystemClock::PrepareOptions(options);
 }
 
-std::string SystemClockWrapper::SerializeOptions(
-    const ConfigOptions& config_options, const std::string& header) const {
-  auto parent = SystemClock::SerializeOptions(config_options, "");
-  if (config_options.IsShallow() || target_ == nullptr ||
-      target_->IsInstanceOf(SystemClock::kDefaultName())) {
-    return parent;
-  } else {
-    std::string result = header;
-    if (!StartsWith(parent, OptionTypeInfo::kIdPropName())) {
-      result.append(OptionTypeInfo::kIdPropName()).append("=");
-    }
-    result.append(parent);
-    if (!EndsWith(result, config_options.delimiter)) {
-      result.append(config_options.delimiter);
-    }
-    result.append("target=").append(target_->ToString(config_options));
-    return result;
-  }
+Status SystemClockWrapper::SerializeOptions(const ConfigOptions& config_options,
+                                            const std::string& prefix,
+                                            OptionProperties* props) const {
+  if (!config_options.IsShallow() && target_ != nullptr &&
+      !target_->IsInstanceOf(SystemClock::kDefaultName())) {
+    props->insert(
+        {kTargetPropName(),
+         target_->ToString(config_options, OptionTypeInfo::MakePrefix(
+                                               prefix, kTargetPropName()))});
+  }
+  return SystemClock::SerializeOptions(config_options, prefix, props);
 }
 
 static int RegisterBuiltinSystemClocks(ObjectLibrary& library,

diff --git a/env/file_system.cc b/env/file_system.cc
@@ -226,7 +226,7 @@ IOStatus ReadFileToString(FileSystem* fs, const std::string& fname,
 
 namespace {
 static std::unordered_map<std::string, OptionTypeInfo> fs_wrapper_type_info = {
-    {"target",
+    {Customizable::kTargetPropName(),
      OptionTypeInfo::AsCustomSharedPtr<FileSystem>(
          0, OptionVerificationType::kByName, OptionTypeFlags::kDontSerialize)},
 };
@@ -243,24 +243,17 @@ Status FileSystemWrapper::PrepareOptions(const ConfigOptions& options) {
   return FileSystem::PrepareOptions(options);
 }
 
-std::string FileSystemWrapper::SerializeOptions(
-    const ConfigOptions& config_options, const std::string& header) const {
-  auto parent = FileSystem::SerializeOptions(config_options, "");
-  if (config_options.IsShallow() || target_ == nullptr ||
-      target_->IsInstanceOf(FileSystem::kDefaultName())) {
-    return parent;
-  } else {
-    std::string result = header;
-    if (!StartsWith(parent, OptionTypeInfo::kIdPropName())) {
-      result.append(OptionTypeInfo::kIdPropName()).append("=");
-    }
-    result.append(parent);
-    if (!EndsWith(result, config_options.delimiter)) {
-      result.append(config_options.delimiter);
-    }
-    result.append("target=").append(target_->ToString(config_options));
-    return result;
+Status FileSystemWrapper::SerializeOptions(const ConfigOptions& config_options,
+                                           const std::string& prefix,
+                                           OptionProperties* props) const {
+  if (!config_options.IsShallow() && target_ != nullptr &&
+      !target_->IsInstanceOf(FileSystem::kDefaultName())) {
+    props->insert(
+        {kTargetPropName(),
+         target_->ToString(config_options, OptionTypeInfo::MakePrefix(
+                                               prefix, kTargetPropName()))});
   }
+  return FileSystem::SerializeOptions(config_options, prefix, props);
 }
 
 DirFsyncOptions::DirFsyncOptions() { reason = kDefault; }

diff --git a/include/rocksdb/configurable.h b/include/rocksdb/configurable.h
@@ -1,3 +1,17 @@
+// Copyright (C) 2022 Speedb Ltd. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
 // Copyright (c) 2011-present, Facebook, Inc.  All rights reserved.
 //  This source code is licensed under both the GPLv2 (found in the
 //  COPYING file in the root directory) and Apache 2.0 License
@@ -20,7 +34,9 @@
 namespace ROCKSDB_NAMESPACE {
 class Logger;
 class ObjectRegistry;
+class OptionProperties;
 class OptionTypeInfo;
+
 struct ColumnFamilyOptions;
 struct ConfigOptions;
 struct DBOptions;
@@ -343,8 +359,14 @@ class Configurable {
                                std::string* bad_name) const;
   // Internal method to serialize options (ToString)
   // Classes may override this value to change its behavior.
-  virtual std::string SerializeOptions(const ConfigOptions& config_options,
-                                       const std::string& header) const;
+  // @param config_options Controls how the options are being matched
+  // @param prefix A string that may be prepended to every option.
+  // @param props Filled with the serialized name-value pairs of the options
+  //
+  // Returns OK on success or an error status if serialized failed.
+  virtual Status SerializeOptions(const ConfigOptions& config_options,
+                                  const std::string& prefix,
+                                  OptionProperties* props) const;
 
   //  Given a name (e.g. rocksdb.my.type.opt), returns the short name (opt)
   virtual std::string GetOptionName(const std::string& long_name) const;