>From Ian Maxon <[email protected]>: Ian Maxon has submitted this change. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17969 )
Change subject: [ASTERIXDB-3315][RT] Exclude compression from hit ratio ...................................................................... [ASTERIXDB-3315][RT] Exclude compression from hit ratio - user model changes: no - storage format changes: no - interface changes: no Details: Add a new signature of pin() to allow a caller to skip counts used in metrics from being collected. This is to stop compression from inflating the bufferCacheHitRatio metric. Change-Id: I568a2df4594bdde19932ba72362c9c61291b9183 Reviewed-on: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17969 Integration-Tests: Jenkins <[email protected]> Reviewed-by: Wail Alkowaileet <[email protected]> Tested-by: Jenkins <[email protected]> --- M hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/MultitenantVirtualBufferCache.java M hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/IBufferCache.java M asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/GlobalVirtualBufferCache.java M hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/VirtualBufferCache.java M hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/DebugBufferCache.java M hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/compression/file/CompressedFileManager.java M hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/BufferCache.java M hyracks-fullstack/hyracks/hyracks-tests/hyracks-storage-am-lsm-btree-test/src/test/java/org/apache/hyracks/storage/am/lsm/btree/impl/TestVirtualBufferCache.java 8 files changed, 90 insertions(+), 10 deletions(-) Approvals: Wail Alkowaileet: Looks good to me, approved Jenkins: Verified; Verified diff --git a/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/GlobalVirtualBufferCache.java b/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/GlobalVirtualBufferCache.java index 0bb9bd5..a7210bc 100644 --- a/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/GlobalVirtualBufferCache.java +++ b/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/GlobalVirtualBufferCache.java @@ -273,6 +273,11 @@ return page; } + @Override + public ICachedPage pin(long dpid, boolean newPage, boolean incrementStats) throws HyracksDataException { + return pin(dpid, newPage); + } + private void incrementFilteredMemoryComponentUsage(long dpid, int pages) { if (filteredMemoryComponentMaxNumPages > 0) { // update memory usage of filtered index diff --git a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/MultitenantVirtualBufferCache.java b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/MultitenantVirtualBufferCache.java index e77acea..959bb50 100644 --- a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/MultitenantVirtualBufferCache.java +++ b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/MultitenantVirtualBufferCache.java @@ -70,6 +70,11 @@ } @Override + public ICachedPage pin(long dpid, boolean newPage, boolean incrementStats) throws HyracksDataException { + return vbc.pin(dpid, newPage); + } + + @Override public void unpin(ICachedPage page) throws HyracksDataException { vbc.unpin(page); } diff --git a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/VirtualBufferCache.java b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/VirtualBufferCache.java index 5871b31..e222461 100644 --- a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/VirtualBufferCache.java +++ b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/VirtualBufferCache.java @@ -260,6 +260,11 @@ return page; } + @Override + public ICachedPage pin(long dpid, boolean newPage, boolean incrementStats) throws HyracksDataException { + return pin(dpid, newPage); + } + private int hash(long dpid) { int hashValue = (int) dpid ^ (Integer.reverse((int) (dpid >>> 32)) >>> 1); return hashValue % buckets.length; diff --git a/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/BufferCache.java b/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/BufferCache.java index 70500e5..e0999d4 100644 --- a/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/BufferCache.java +++ b/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/BufferCache.java @@ -166,13 +166,18 @@ @Override public ICachedPage pin(long dpid, boolean newPage) throws HyracksDataException { + return pin(dpid, newPage, true); + } + + @Override + public ICachedPage pin(long dpid, boolean newPage, boolean incrementStats) throws HyracksDataException { // Calling the pinSanityCheck should be used only for debugging, since // the synchronized block over the fileInfoMap is a hot spot. if (DEBUG) { pinSanityCheck(dpid); } final IThreadStats threadStats = statsSubscribers.get(Thread.currentThread()); - if (threadStats != null) { + if (threadStats != null && incrementStats) { threadStats.pagePinned(); } CachedPage cPage = findPage(dpid); @@ -194,7 +199,7 @@ synchronized (cPage) { if (!cPage.valid) { try { - tryRead(cPage); + tryRead(cPage, incrementStats); cPage.valid = true; } catch (Exception e) { LOGGER.log(ExceptionUtils.causedByInterrupt(e) ? Level.DEBUG : Level.WARN, @@ -521,10 +526,10 @@ return false; } - private void tryRead(CachedPage cPage) throws HyracksDataException { + private void tryRead(CachedPage cPage, boolean incrementStats) throws HyracksDataException { for (int i = 1; i <= MAX_PAGE_READ_ATTEMPTS; i++) { try { - read(cPage); + read(cPage, incrementStats); return; } catch (HyracksDataException readException) { if (readException.matches(ErrorCode.CANNOT_READ_CLOSED_FILE) && i != MAX_PAGE_READ_ATTEMPTS) { @@ -548,12 +553,12 @@ } } - private void read(CachedPage cPage) throws HyracksDataException { + private void read(CachedPage cPage, boolean incrementStats) throws HyracksDataException { BufferedFileHandle fInfo = getFileHandle(cPage); cPage.buffer.clear(); fInfo.read(cPage); final IThreadStats threadStats = statsSubscribers.get(Thread.currentThread()); - if (threadStats != null) { + if (threadStats != null && incrementStats) { threadStats.coldRead(); } } diff --git a/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/DebugBufferCache.java b/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/DebugBufferCache.java index 8c3d492..c76c781 100644 --- a/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/DebugBufferCache.java +++ b/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/DebugBufferCache.java @@ -77,13 +77,18 @@ } @Override - public ICachedPage pin(long dpid, boolean newPage) throws HyracksDataException { - ICachedPage page = bufferCache.pin(dpid, newPage); + public ICachedPage pin(long dpid, boolean newPage, boolean incrementStats) throws HyracksDataException { + ICachedPage page = bufferCache.pin(dpid, newPage, incrementStats); pinCount.addAndGet(1); return page; } @Override + public ICachedPage pin(long dpid, boolean newPage) throws HyracksDataException { + return pin(dpid, newPage, true); + } + + @Override public void unpin(ICachedPage page) throws HyracksDataException { bufferCache.unpin(page); unpinCount.addAndGet(1); diff --git a/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/IBufferCache.java b/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/IBufferCache.java index df0bea8..3e977bd 100644 --- a/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/IBufferCache.java +++ b/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/IBufferCache.java @@ -105,6 +105,28 @@ ICachedPage pin(long dpid, boolean newPage) throws HyracksDataException; /** + * Pin the page so it can't be evicted from the buffer cache... + * + * @param dpid + * page id is a unique id that is a combination of file id and page id + * @param newPage + * whether this page is expected to be new. + * NOTE: undefined: + * -- what if the flag is true but the page exists? + * -- what if the flag is false but the page doesn't exist + * + * @param incrementStats + * whether to increment the coldRead and pinCount counters when + * the page is pinned. this is to not bias the count when using + * accessors that cause nested pins due to wrapping file handles, + * like compression + * + * @return the pinned page + * @throws HyracksDataException + */ + ICachedPage pin(long dpid, boolean newPage, boolean incrementStats) throws HyracksDataException; + + /** * Unpin a pinned page so its buffer can be recycled * * @param page diff --git a/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/compression/file/CompressedFileManager.java b/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/compression/file/CompressedFileManager.java index 7d0cc62..b334b1f 100644 --- a/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/compression/file/CompressedFileManager.java +++ b/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/compression/file/CompressedFileManager.java @@ -311,7 +311,9 @@ final int numOfEntriesPerPage = bufferCache.getPageSize() / ENTRY_LENGTH; //get the last page which may contain less entries than maxNumOfEntries final long dpid = getDiskPageId(numOfPages - 1); - final ICachedPage page = bufferCache.pin(dpid, false); + //exclude the LAF from the buffer cache pin/read stats, because it is hot in any + //sane scenario. otherwise it inflates the query's cache ratio greatly. + final ICachedPage page = bufferCache.pin(dpid, false, false); try { final ByteBuffer buf = page.getBuffer(); @@ -330,7 +332,10 @@ private ICachedPage pinAndGetPage(int compressedPageId) throws HyracksDataException { final int pageId = compressedPageId * ENTRY_LENGTH / bufferCache.getPageSize(); - return bufferCache.pin(getDiskPageId(pageId), false); + //don't increment the stats with this pin. this is because we are pinning on behalf + //of a caller, so our pins will be an interfering access pattern that conflates itself + //with the caller's cache pattern. + return bufferCache.pin(getDiskPageId(pageId), false, false); } private long getDiskPageId(int pageId) { diff --git a/hyracks-fullstack/hyracks/hyracks-tests/hyracks-storage-am-lsm-btree-test/src/test/java/org/apache/hyracks/storage/am/lsm/btree/impl/TestVirtualBufferCache.java b/hyracks-fullstack/hyracks/hyracks-tests/hyracks-storage-am-lsm-btree-test/src/test/java/org/apache/hyracks/storage/am/lsm/btree/impl/TestVirtualBufferCache.java index 8cad497..e697b54 100644 --- a/hyracks-fullstack/hyracks/hyracks-tests/hyracks-storage-am-lsm-btree-test/src/test/java/org/apache/hyracks/storage/am/lsm/btree/impl/TestVirtualBufferCache.java +++ b/hyracks-fullstack/hyracks/hyracks-tests/hyracks-storage-am-lsm-btree-test/src/test/java/org/apache/hyracks/storage/am/lsm/btree/impl/TestVirtualBufferCache.java @@ -101,6 +101,11 @@ } @Override + public ICachedPage pin(long dpid, boolean newPage, boolean incrementStats) throws HyracksDataException { + return pin(dpid, newPage); + } + + @Override public void unpin(ICachedPage page) throws HyracksDataException { vbc.unpin(page); } -- To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17969 To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-Project: asterixdb Gerrit-Branch: trinity Gerrit-Change-Id: I568a2df4594bdde19932ba72362c9c61291b9183 Gerrit-Change-Number: 17969 Gerrit-PatchSet: 4 Gerrit-Owner: Ian Maxon <[email protected]> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Ian Maxon <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Peeyush Gupta <[email protected]> Gerrit-Reviewer: Wail Alkowaileet <[email protected]> Gerrit-MessageType: merged
