Fix the initial cache stats reporting (facebookincubator#10408)

Summary:
There is a bug in delta cache stats calculation which can leads to the missing initial report.
As we only set the ssd delta if both the old and new cache stats has ssd stats but for the
initial report, the old one doesn't have the ssd stats which caused the initial cache stats like
the checkpoint reads never get reported. This PR fixes this with unit test.

Pull Request resolved: facebookincubator#10408

Reviewed By: tanjialiang, zacw7

Differential Revision: D59416042

Pulled By: xiaoxmeng

fbshipit-source-id: 41cf374897e56edd0551bbb1f18174c81d14069d

velox/common/caching/AsyncDataCache.cpp

@@ -626,7 +626,7 @@ bool CacheShard::removeFileEntries(
   return true;
 }
 
-CacheStats CacheStats::operator-(CacheStats& other) const {
+CacheStats CacheStats::operator-(const CacheStats& other) const {
   CacheStats result;
   result.numHit = numHit - other.numHit;
   result.hitBytes = hitBytes - other.hitBytes;
@@ -639,9 +639,13 @@ CacheStats CacheStats::operator-(CacheStats& other) const {
   result.numStales = numStales - other.numStales;
   result.allocClocks = allocClocks - other.allocClocks;
   result.sumEvictScore = sumEvictScore - other.sumEvictScore;
-  if (ssdStats != nullptr && other.ssdStats != nullptr) {
-    result.ssdStats =
-        std::make_shared<SsdCacheStats>(*ssdStats - *other.ssdStats);
+  if (ssdStats != nullptr) {
+    if (other.ssdStats != nullptr) {
+      result.ssdStats =
+          std::make_shared<SsdCacheStats>(*ssdStats - *other.ssdStats);
+    } else {
+      result.ssdStats = std::make_shared<SsdCacheStats>(*ssdStats);
+    }
   }
   return result;
 }

velox/common/caching/AsyncDataCache.h

@@ -545,7 +545,7 @@ struct CacheStats {
   /// Ssd cache stats that include both snapshot and cumulative stats.
   std::shared_ptr<SsdCacheStats> ssdStats = nullptr;
 
-  CacheStats operator-(CacheStats& other) const;
+  CacheStats operator-(const CacheStats& other) const;
 
   std::string toString() const;
 };

velox/common/caching/tests/AsyncDataCacheTest.cpp

@@ -991,6 +991,26 @@ TEST_P(AsyncDataCacheTest, cacheStats) {
   ASSERT_EQ(cache_->toString(false), expectedShortCacheOutput);
 }
 
+TEST_P(AsyncDataCacheTest, cacheStatsWithSsd) {
+  CacheStats stats;
+  stats.numHit = 234;
+  stats.numEvict = 1024;
+  stats.ssdStats = std::make_shared<SsdCacheStats>();
+  stats.ssdStats->bytesWritten = 1;
+  stats.ssdStats->bytesRead = 1;
+
+  const CacheStats otherStats;
+  const CacheStats deltaStats = stats - otherStats;
+  ASSERT_EQ(deltaStats.numHit, 234);
+  ASSERT_EQ(deltaStats.numEvict, 1024);
+  ASSERT_TRUE(deltaStats.ssdStats != nullptr);
+  ASSERT_EQ(deltaStats.ssdStats->bytesWritten, 1);
+  ASSERT_EQ(deltaStats.ssdStats->bytesRead, 1);
+  const std::string expectedDeltaCacheStats =
+      "Cache size: 0B tinySize: 0B large size: 0B\nCache entries: 0 read pins: 0 write pins: 0 pinned shared: 0B pinned exclusive: 0B\n num write wait: 0 empty entries: 0\nCache access miss: 0 hit: 234 hit bytes: 0B eviction: 1024 savable eviction: 0 eviction checks: 0 aged out: 0 stales: 0\nPrefetch entries: 0 bytes: 0B\nAlloc Megaclocks 0";
+  ASSERT_EQ(deltaStats.toString(), expectedDeltaCacheStats);
+}
+
 TEST_P(AsyncDataCacheTest, staleEntry) {
   constexpr uint64_t kRamBytes = 1UL << 30;
   // Disable SSD cache to test in-memory cache stale entry only.