commit be01a7358caef59535d2a1d929638e4a375f2bf5
author Wail Alkowaileet <wael.y.k@gmail.com> Tue May 14 16:03:28 2024 -0700
committer Wail Alkowaileet <wael.y.k@gmail.com> Wed May 15 02:59:14 2024 +0000
tree 693feaf08fec3b92524b9d30671518698dd2a446
parent 97da44e9d1ee49dbc3db753279e40e73d102c3cc

[NO ISSUE] Ensure confiscated pages

- user model changes: no
- storage format changes: no
- interface changes: no

Details:
Simple patch to check if we suffered from confiscating
a page that is already confiscated. There's a potetntial
of a race condititon here.

Change-Id: I771f29d7709d94f0c6683886edcd3a8ab0e9eec2
Reviewed-on: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/18289
Integration-Tests: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Tested-by: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Reviewed-by: Ian Maxon <imaxon@uci.edu>

hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/BufferCache.java

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 d47b8e3..53e27a9 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
@@ -1176,7 +1176,7 @@
     }
 
     private ICachedPage confiscateInner(long dpid, int multiplier) {
-        ICachedPage returnPage = null;
+        CachedPage returnPage = null;
         CachedPage victim = (CachedPage) pageReplacementStrategy.findVictim(multiplier);
         if (victim == null) {
             return victim;
@@ -1197,7 +1197,7 @@
                 return null;
             }
             returnPage = victim;
-            ((CachedPage) returnPage).dpid = dpid;
+            returnPage.dpid = dpid;
         } else {
             // Case 2a/b
             int pageHash = hash(victim.getDiskPageId());
@@ -1240,7 +1240,7 @@
                 }
                 if (found) {
                     returnPage = victim;
-                    ((CachedPage) returnPage).dpid = dpid;
+                    returnPage.dpid = dpid;
                 } //otherwise, someone took the same victim before we acquired the lock. try again!
             } finally {
                 bucket.bucketLock.unlock();
@@ -1248,12 +1248,17 @@
         }
         // if we found a page after all that, go ahead and finish
         if (returnPage != null) {
-            ((CachedPage) returnPage).confiscated.set(true);
+            int pinCount = returnPage.pinCount.get();
+            if (!returnPage.confiscated.compareAndSet(false, true)) {
+                throw new IllegalStateException("Returned page is already confiscated");
+            } else if (pinCount != 1) {
+                throw new IllegalStateException("Returned page's pinCount is not 1. It is " + pinCount);
+            }
             if (DEBUG) {
                 confiscateLock.lock();
                 try {
-                    confiscatedPages.add((CachedPage) returnPage);
-                    confiscatedPagesOwner.put((CachedPage) returnPage, Thread.currentThread().getStackTrace());
+                    confiscatedPages.add(returnPage);
+                    confiscatedPagesOwner.put(returnPage, Thread.currentThread().getStackTrace());
                 } finally {
                     confiscateLock.unlock();
                 }