Skip to content

Commit

Permalink
Fix runtime stats when cache hit
Browse files Browse the repository at this point in the history
### What changes are proposed in this pull request?
Fix runtime stats when cache hit

### Why are the changes needed?

We already count the runtime stats in local cache manager, but we didn't pass in a proper cache context

### Does this PR introduce any user facing changes?

Please list the user-facing changes introduced by your change, including
  1. change in user-facing APIs
  2. addition or removal of property keys
  3. webui

			pr-link: #18503
			change-id: cid-233916f6b410432f8300f10ac8a2385afe969f2c
  • Loading branch information
beinan authored Feb 23, 2024
1 parent a3ecbc7 commit 7429c25
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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.
Expand All @@ -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
Expand All @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down

0 comments on commit 7429c25

Please sign in to comment.