Add nodiscard attribute to methods of Storage (#1750)

apache · Sep 9, 2023 · c30bcec · c30bcec
1 parent 166f9a7
commit c30bcec
Show file tree

Hide file tree

Showing 5 changed files with 31 additions and 17 deletions.
diff --git a/src/commands/cmd_script.cc b/src/commands/cmd_script.cc
@@ -72,9 +72,13 @@ class CommandScript : public Commander {
     }
 
     if (args_.size() == 2 && subcommand_ == "flush") {
-      svr->ScriptFlush();
-      auto s = svr->Propagate(engine::kPropagateScriptCommand, args_);
-      if (!s.IsOK()) {
+      auto s = svr->ScriptFlush();
+      if (!s) {
+        LOG(ERROR) << "Failed to flush scripts: " << s.Msg();
+        return s;
+      }
+      s = svr->Propagate(engine::kPropagateScriptCommand, args_);
+      if (!s) {
         LOG(ERROR) << "Failed to propagate script command: " << s.Msg();
         return s;
       }

diff --git a/src/server/server.cc b/src/server/server.cc
@@ -1251,7 +1251,11 @@ Status Server::AsyncCompactDB(const std::string &begin_key, const std::string &e
     std::unique_ptr<Slice> begin = nullptr, end = nullptr;
     if (!begin_key.empty()) begin = std::make_unique<Slice>(begin_key);
     if (!end_key.empty()) end = std::make_unique<Slice>(end_key);
-    storage->Compact(begin.get(), end.get());
+
+    auto s = storage->Compact(begin.get(), end.get());
+    if (!s.ok()) {
+      LOG(ERROR) << "[task runner] Failed to do compaction: " << s.ToString();
+    }
 
     std::lock_guard<std::mutex> lg(db_job_mu_);
     db_compacting_ = false;
@@ -1538,10 +1542,12 @@ void Server::ScriptReset() {
   lua::DestroyState(lua);
 }
 
-void Server::ScriptFlush() {
+Status Server::ScriptFlush() {
   auto cf = storage->GetCFHandle(engine::kPropagateColumnFamilyName);
-  storage->FlushScripts(storage->DefaultWriteOptions(), cf);
+  auto s = storage->FlushScripts(storage->DefaultWriteOptions(), cf);
+  if (!s.ok()) return {Status::NotOK, s.ToString()};
   ScriptReset();
+  return Status::OK();
 }
 
 // Generally, we store data into RocksDB and just replicate WAL instead of propagating

diff --git a/src/server/server.h b/src/server/server.h
@@ -253,7 +253,7 @@ class Server {
   Status ScriptGet(const std::string &sha, std::string *body) const;
   Status ScriptSet(const std::string &sha, const std::string &body) const;
   void ScriptReset();
-  void ScriptFlush();
+  Status ScriptFlush();
 
   Status Propagate(const std::string &channel, const std::vector<std::string> &tokens) const;
   Status ExecPropagatedCommand(const std::vector<std::string> &tokens);

diff --git a/src/storage/compaction_checker.cc b/src/storage/compaction_checker.cc
@@ -135,6 +135,9 @@ void CompactionChecker::PickCompactionFiles(const std::string &cf_name) {
   if (best_delete_ratio > 0.1 && !best_start_key.empty() && !best_stop_key.empty()) {
     LOG(INFO) << "[compaction checker] Going to compact the key in file: " << best_filename
               << ", delete ratio: " << best_delete_ratio;
-    storage_->Compact(&best_start_key, &best_stop_key);
+    auto s = storage_->Compact(&best_start_key, &best_stop_key);
+    if (!s.ok()) {
+      LOG(ERROR) << "[compaction checker] Failed to do compaction: " << s.ToString();
+    }
   }
 }
diff --git a/src/storage/storage.h b/src/storage/storage.h
@@ -102,27 +102,28 @@ class Storage {
   Status ReplicaApplyWriteBatch(std::string &&raw_batch);
   rocksdb::SequenceNumber LatestSeqNumber();
 
-  rocksdb::Status Get(const rocksdb::ReadOptions &options, const rocksdb::Slice &key, std::string *value);
-  rocksdb::Status Get(const rocksdb::ReadOptions &options, rocksdb::ColumnFamilyHandle *column_family,
-                      const rocksdb::Slice &key, std::string *value);
+  [[nodiscard]] rocksdb::Status Get(const rocksdb::ReadOptions &options, const rocksdb::Slice &key, std::string *value);
+  [[nodiscard]] rocksdb::Status Get(const rocksdb::ReadOptions &options, rocksdb::ColumnFamilyHandle *column_family,
+                                    const rocksdb::Slice &key, std::string *value);
   void MultiGet(const rocksdb::ReadOptions &options, rocksdb::ColumnFamilyHandle *column_family, size_t num_keys,
                 const rocksdb::Slice *keys, rocksdb::PinnableSlice *values, rocksdb::Status *statuses);
   rocksdb::Iterator *NewIterator(const rocksdb::ReadOptions &options, rocksdb::ColumnFamilyHandle *column_family);
   rocksdb::Iterator *NewIterator(const rocksdb::ReadOptions &options);
 
-  rocksdb::Status Write(const rocksdb::WriteOptions &options, rocksdb::WriteBatch *updates);
+  [[nodiscard]] rocksdb::Status Write(const rocksdb::WriteOptions &options, rocksdb::WriteBatch *updates);
   const rocksdb::WriteOptions &DefaultWriteOptions() { return write_opts_; }
   rocksdb::ReadOptions DefaultScanOptions() const;
   rocksdb::ReadOptions DefaultMultiGetOptions() const;
-  rocksdb::Status Delete(const rocksdb::WriteOptions &options, rocksdb::ColumnFamilyHandle *cf_handle,
-                         const rocksdb::Slice &key);
-  rocksdb::Status DeleteRange(const std::string &first_key, const std::string &last_key);
-  rocksdb::Status FlushScripts(const rocksdb::WriteOptions &options, rocksdb::ColumnFamilyHandle *cf_handle);
+  [[nodiscard]] rocksdb::Status Delete(const rocksdb::WriteOptions &options, rocksdb::ColumnFamilyHandle *cf_handle,
+                                       const rocksdb::Slice &key);
+  [[nodiscard]] rocksdb::Status DeleteRange(const std::string &first_key, const std::string &last_key);
+  [[nodiscard]] rocksdb::Status FlushScripts(const rocksdb::WriteOptions &options,
+                                             rocksdb::ColumnFamilyHandle *cf_handle);
   bool WALHasNewData(rocksdb::SequenceNumber seq) { return seq <= LatestSeqNumber(); }
   Status InWALBoundary(rocksdb::SequenceNumber seq);
   Status WriteToPropagateCF(const std::string &key, const std::string &value);
 
-  rocksdb::Status Compact(const rocksdb::Slice *begin, const rocksdb::Slice *end);
+  [[nodiscard]] rocksdb::Status Compact(const rocksdb::Slice *begin, const rocksdb::Slice *end);
   rocksdb::DB *GetDB();
   bool IsClosing() const { return db_closing_; }
   std::string GetName() const { return config_->db_name; }