Improve comments for StatsHistoryIterator and InMemoryStatsHistoryIterator

Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/5346

Differential Revision: D15497679

Pulled By: miasantreble

fbshipit-source-id: c10caf10293c3d9663bfb398a0d331326d1e9e67

5 files changed, 45 insertions, 4 deletions

diff --git a/db/db_impl.h b/db/db_impl.h
index 08cb19491..f574a8f44 100644
--- a/db/db_impl.h
+++ b/db/db_impl.h
@@ -761,7 +761,6 @@ class DBImpl : public DB {
   static Status CreateAndNewDirectory(Env* env, const std::string& dirname,
                                       std::unique_ptr<Directory>* directory);
 
-  // Given a time window, return an iterator for accessing stats history
   Status GetStatsHistory(
       uint64_t start_time, uint64_t end_time,
       std::unique_ptr<StatsHistoryIterator>* stats_iterator) override;
diff --git a/db/in_memory_stats_history.cc b/db/in_memory_stats_history.cc
index 39355cfbe..e9e0cc749 100644
--- a/db/in_memory_stats_history.cc
+++ b/db/in_memory_stats_history.cc
@@ -17,6 +17,10 @@ bool InMemoryStatsHistoryIterator::Valid() const { return valid_; }
 
 Status InMemoryStatsHistoryIterator::status() const { return status_; }
 
+// Because of garbage collection, the next stats snapshot may or may not be
+// right after the current one. When reading from DBImpl::stats_history_, this
+// call will be protected by DB Mutex so it will not return partial or
+// corrupted results.
 void InMemoryStatsHistoryIterator::Next() {
   // increment start_time by 1 to avoid infinite loop
   AdvanceIteratorByTime(GetStatsTime() + 1, end_time_);
diff --git a/db/in_memory_stats_history.h b/db/in_memory_stats_history.h
index 4b52e23ff..eeb679cc0 100644
--- a/db/in_memory_stats_history.h
+++ b/db/in_memory_stats_history.h
@@ -12,8 +12,20 @@
 
 namespace rocksdb {
 
+// InMemoryStatsHistoryIterator can be used to access stats history that was
+// stored by an in-memory two level std::map(DBImpl::stats_history_). It keeps
+// a copy of the stats snapshot (in stats_map_) that is currently being pointed
+// to, which allows the iterator to access the stats snapshot even when
+// the background garbage collecting thread purges it from the source of truth
+// (`DBImpl::stats_history_`). In that case, the iterator will continue to be
+// valid until a call to `Next()` returns no result and invalidates it. In
+// some extreme cases, the iterator may also return fragmented segments of
+// stats snapshots due to long gaps between `Next()` calls and interleaved
+// garbage collection.
 class InMemoryStatsHistoryIterator final : public StatsHistoryIterator {
  public:
+  // Setup InMemoryStatsHistoryIterator to return stats snapshots between
+  // microsecond timestamps [start_time, end_time)
   InMemoryStatsHistoryIterator(uint64_t start_time, uint64_t end_time,
                                DBImpl* db_impl)
       : start_time_(start_time),
@@ -26,9 +38,16 @@ class InMemoryStatsHistoryIterator final : public StatsHistoryIterator {
   bool Valid() const override;
   Status status() const override;
 
+  // Move to the next stats snapshot currently available
+  // This function may invalidate the iterator
+  // REQUIRES: Valid()
   void Next() override;
+
+  // REQUIRES: Valid()
   uint64_t GetStatsTime() const override;
 
+  // This function is idempotent
+  // REQUIRES: Valid()
   const std::map<std::string, uint64_t>& GetStatsMap() const override;
 
  private:
diff --git a/include/rocksdb/db.h b/include/rocksdb/db.h
index 7b49b92c2..b0538433b 100644
--- a/include/rocksdb/db.h
+++ b/include/rocksdb/db.h
@@ -1322,8 +1322,9 @@ class DB {
   // Needed for StackableDB
   virtual DB* GetRootDB() { return this; }
 
-  // Given a time window, return an iterator for accessing stats history
-  // User is responsible for deleting StatsHistoryIterator after use
+  // Given a window [start_time, end_time), setup a StatsHistoryIterator
+  // to access stats history. Note the start_time and end_time are epoch
+  // time measured in microsecond, and end_time is an exclusive bound.
   virtual Status GetStatsHistory(
       uint64_t /*start_time*/, uint64_t /*end_time*/,
       std::unique_ptr<StatsHistoryIterator>* /*stats_iterator*/) {
diff --git a/include/rocksdb/stats_history.h b/include/rocksdb/stats_history.h
index 40ea51d1f..1a8419081 100644
--- a/include/rocksdb/stats_history.h
+++ b/include/rocksdb/stats_history.h
@@ -11,7 +11,6 @@
 #include <map>
 #include <string>
 
-// #include "db/db_impl.h"
 #include "rocksdb/statistics.h"
 #include "rocksdb/status.h"
 
@@ -19,6 +18,25 @@ namespace rocksdb {
 
 class DBImpl;
 
+// StatsHistoryIterator is the main interface for users to programmatically
+// access statistics snapshots that was automatically stored by RocksDB.
+// Depending on options, the stats can be in memory or on disk.
+// The stats snapshots are indexed by time that they were recorded, and each
+// stats snapshot contains individual stat name and value at the time of
+// recording.
+// Example:
+//   std::unique_ptr<StatsHistoryIterator> stats_iter;
+//   Status s = db->GetStatsHistory(0 /* start_time */,
+//                                  env->NowMicros() /* end_time*/,
+//                                  &stats_iter);
+//   if (s.ok) {
+//     for (; stats_iter->Valid(); stats_iter->Next()) {
+//       uint64_t stats_time = stats_iter->GetStatsTime();
+//       const std::map<std::string, uint64_t>& stats_map =
+//           stats_iter->GetStatsMap();
+//       process(stats_time, stats_map);
+//     }
+//   }
 class StatsHistoryIterator {
  public:
   StatsHistoryIterator() {}