Fix the runtime stats from presto when cache hit

Alluxio · Jan 31, 2024 · c69d301 · c69d301
1 parent c65c32d
commit c69d301
Show file tree

Hide file tree

Showing 2 changed files with 6 additions and 18 deletions.
diff --git a/dora/core/client/fs/src/main/java/alluxio/client/file/cache/LocalCacheFileInStream.java b/dora/core/client/fs/src/main/java/alluxio/client/file/cache/LocalCacheFileInStream.java
@@ -59,8 +59,6 @@ public class LocalCacheFileInStream extends FileInStream {
   private final CacheManager mCacheManager;
   private final boolean mIsDora;
   private final boolean mQuotaEnabled;
-  /** Cache related context of this file. */
-  private final CacheContext mCacheContext;
   /** File info, fetched from external FS. */
   private final URIStatus mStatus;
   private final FileInStreamOpener mExternalFileInStreamOpener;
@@ -118,11 +116,6 @@ public LocalCacheFileInStream(URIStatus status, FileInStreamOpener fileOpener,
     mIsDora = conf.getBoolean(PropertyKey.DORA_ENABLED);
     // Currently quota is only supported when it is set by external systems in status context
     mQuotaEnabled = conf.getBoolean(PropertyKey.USER_CLIENT_CACHE_QUOTA_ENABLED);
-    if (mQuotaEnabled && status.getCacheContext() != null) {
-      mCacheContext = status.getCacheContext();
-    } else {
-      mCacheContext = CacheContext.defaults();
-    }
     Metrics.registerGauges();
 
     mBufferSize = (int) conf.getBytes(PropertyKey.USER_CLIENT_CACHE_IN_STREAM_BUFFER_SIZE);
@@ -184,7 +177,10 @@ private int localCachedRead(ReadTargetBuffer bytesBuffer, int length,
     long currentPage = position / mPageSize;
     PageId pageId;
     CacheContext cacheContext = mStatus.getCacheContext();
-    if (cacheContext != null && cacheContext.getCacheIdentifier() != null) {
+    if (cacheContext == null) {
+      cacheContext = CacheContext.defaults();
+    }
+    if (cacheContext.getCacheIdentifier() != null) {
       pageId = new PageId(cacheContext.getCacheIdentifier(), currentPage);
     } else {
       // In Dora, the fileId is generated by Worker or by local client, which maybe is not unique.
@@ -198,15 +194,10 @@ private int localCachedRead(ReadTargetBuffer bytesBuffer, int length,
     int bytesToReadInPage = Math.min(bytesLeftInPage, length);
     stopwatch.reset().start();
     int bytesRead =
-        mCacheManager.get(pageId, currentPageOffset, bytesToReadInPage, bytesBuffer, mCacheContext);
+        mCacheManager.get(pageId, currentPageOffset, bytesToReadInPage, bytesBuffer, cacheContext);
     stopwatch.stop();
     if (bytesRead > 0) {
       MetricsSystem.counter(MetricKey.CLIENT_CACHE_HIT_REQUESTS.getName()).inc();
-      if (cacheContext != null) {
-        cacheContext.incrementCounter(
-            MetricKey.CLIENT_CACHE_PAGE_READ_CACHE_TIME_NS.getMetricName(), NANO,
-            stopwatch.elapsed(TimeUnit.NANOSECONDS));
-      }
       return bytesRead;
     }
     // on local cache miss, read a complete page from external storage. This will always make
@@ -229,7 +220,7 @@ private int localCachedRead(ReadTargetBuffer bytesBuffer, int length,
             stopwatch.elapsed(TimeUnit.NANOSECONDS)
         );
       }
-      mCacheManager.put(pageId, page, mCacheContext);
+      mCacheManager.put(pageId, page, cacheContext);
     }
     return bytesToReadInPage;
   }

diff --git a/dora/core/client/fs/src/test/java/alluxio/client/file/cache/LocalCacheFileInStreamTest.java b/dora/core/client/fs/src/test/java/alluxio/client/file/cache/LocalCacheFileInStreamTest.java
@@ -543,11 +543,8 @@ protected Stopwatch createUnstartedStopwatch() {
         };
 
     Assert.assertArrayEquals(testData, ByteStreams.toByteArray(stream));
-    long timeReadCache = recordedMetrics.get(
-        MetricKey.CLIENT_CACHE_PAGE_READ_CACHE_TIME_NS.getMetricName());
     long timeReadExternal = recordedMetrics.get(
         MetricKey.CLIENT_CACHE_PAGE_READ_EXTERNAL_TIME_NS.getMetricName());
-    Assert.assertEquals(timeSource.get(StepTicker.Type.CACHE_HIT), timeReadCache);
     Assert.assertEquals(timeSource.get(StepTicker.Type.CACHE_MISS), timeReadExternal);
   }