This is an automated email from the ASF dual-hosted git repository.
wyk pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/asterixdb.git
The following commit(s) were added to refs/heads/master by this push:
new d9fdcabd32 [ASTERIXDB-3461][STO] Guard CachePage pinCount
d9fdcabd32 is described below
commit d9fdcabd325756bd9af2ab91e1df7e3e2bb8860d
Author: Wail Alkowaileet <[email protected]>
AuthorDate: Tue Jul 16 15:23:28 2024 -0700
[ASTERIXDB-3461][STO] Guard CachePage pinCount
- user model changes: no
- storage format changes: no
- interface changes: no
Details:
Guard against decrementing pinCount to
a negative number
Ext-ref: MB-62736
Change-Id: I85196ec1d99259b43c94d6dc9916a2ae042a9a10
Reviewed-on: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/18490
Integration-Tests: Jenkins <[email protected]>
Reviewed-by: Michael Blow <[email protected]>
Tested-by: Jenkins <[email protected]>
---
.../storage/common/buffercache/BufferCache.java | 30 +++++++++++-----------
.../storage/common/buffercache/CachedPage.java | 14 +++++++++-
2 files changed, 28 insertions(+), 16 deletions(-)
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 15a019c595..4c2395fd12 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
@@ -254,7 +254,7 @@ public class BufferCache implements IBufferCacheInternal,
ILifeCycleComponent, I
if (DEBUG) {
assert !cPage.confiscated.get();
}
- cPage.pinCount.incrementAndGet();
+ cPage.incrementAndGetPinCount();
return cPage;
}
cPage = cPage.next;
@@ -311,7 +311,7 @@ public class BufferCache implements IBufferCacheInternal,
ILifeCycleComponent, I
// now that we have the pin, ensure the victim's dpid still is
< 0, if it's not, decrement
// pin count and try again
if (victim.dpid >= 0) {
- victim.pinCount.decrementAndGet();
+ victim.decrementAndGetPinCount();
return null;
}
if (DEBUG) {
@@ -353,7 +353,7 @@ public class BufferCache implements IBufferCacheInternal,
ILifeCycleComponent, I
// now that we have the pin, ensure the victim's bucket hasn't
changed, if it has, decrement
// pin count and try again
if (victimHash != hash(victim.dpid)) {
- victim.pinCount.decrementAndGet();
+ victim.decrementAndGetPinCount();
return null;
}
if (DEBUG) {
@@ -397,7 +397,7 @@ public class BufferCache implements IBufferCacheInternal,
ILifeCycleComponent, I
// now that we have the pin, ensure the victim's bucket hasn't
changed, if it has, decrement
// pin count and try again
if (victimHash != hash(victim.dpid)) {
- victim.pinCount.decrementAndGet();
+ victim.decrementAndGetPinCount();
return null;
}
if (DEBUG && confiscatedPages.contains(victim)) {
@@ -436,8 +436,8 @@ public class BufferCache implements IBufferCacheInternal,
ILifeCycleComponent, I
private CachedPage findTargetInBucket(long dpid, CachedPage cPage,
CachedPage victim) {
while (cPage != null) {
if (cPage.dpid == dpid) {
- cPage.pinCount.incrementAndGet();
- victim.pinCount.decrementAndGet();
+ cPage.incrementAndGetPinCount();
+ victim.decrementAndGetPinCount();
if (DEBUG) {
assert !cPage.confiscated.get();
}
@@ -610,7 +610,7 @@ public class BufferCache implements IBufferCacheInternal,
ILifeCycleComponent, I
}
context.onUnpin(page);
- int pinCount = ((CachedPage) page).pinCount.decrementAndGet();
+ int pinCount = ((CachedPage) page).decrementAndGetPinCount();
if (DEBUG && pinCount == 0) {
pinnedPageOwner.remove(page);
}
@@ -700,7 +700,7 @@ public class BufferCache implements IBufferCacheInternal,
ILifeCycleComponent, I
}
if (cleaned) {
cPage.dirty.set(false);
- cPage.pinCount.decrementAndGet();
+ cPage.decrementAndGetPinCount();
// this increment of a volatile is OK as there is only one
writer
cleanedCount++;
synchronized (cleanNotification) {
@@ -925,11 +925,11 @@ public class BufferCache implements IBufferCacheInternal,
ILifeCycleComponent, I
write(cPage, DefaultBufferCacheWriteContext.INSTANCE);
}
cPage.dirty.set(false);
- pinCount = cPage.pinCount.decrementAndGet();
+ pinCount = cPage.decrementAndGetPinCount();
} else {
pinCount = cPage.pinCount.get();
}
- if (pinCount > 0) {
+ if (pinCount != 0) {
throw new IllegalStateException("Page " +
BufferedFileHandle.getFileId(cPage.dpid) + ":"
+ BufferedFileHandle.getPageId(cPage.dpid)
+ " is pinned and file is being closed. Pincount is: "
+ pinCount + " Page is confiscated: "
@@ -1081,7 +1081,7 @@ public class BufferCache implements IBufferCacheInternal,
ILifeCycleComponent, I
// now that we have the pin, ensure the victim's dpid still is <
0, if it's not, decrement
// pin count and try again
if (victim.dpid >= 0) {
- victim.pinCount.decrementAndGet();
+ victim.decrementAndGetPinCount();
return false;
}
} else {
@@ -1096,7 +1096,7 @@ public class BufferCache implements IBufferCacheInternal,
ILifeCycleComponent, I
// now that we have the pin, ensure the victim's bucket hasn't
changed, if it has, decrement
// pin count and try again
if (pageHash != hash(victim.dpid)) {
- victim.pinCount.decrementAndGet();
+ victim.decrementAndGetPinCount();
return false;
}
// readjust the next pointers to remove this page from
@@ -1198,7 +1198,7 @@ public class BufferCache implements IBufferCacheInternal,
ILifeCycleComponent, I
// now that we have the pin, ensure the victim's dpid still is <
0, if it's not, decrement
// pin count and try again
if (victim.dpid >= 0) {
- victim.pinCount.decrementAndGet();
+ victim.decrementAndGetPinCount();
return null;
}
returnPage = victim;
@@ -1383,7 +1383,7 @@ public class BufferCache implements IBufferCacheInternal,
ILifeCycleComponent, I
cPage.valid = true;
cPage.next = bucket.cachedPage;
bucket.cachedPage = cPage;
- cPage.pinCount.decrementAndGet();
+ cPage.decrementAndGetPinCount();
if (DEBUG) {
assert cPage.pinCount.get() == 0;
assert cPage.latch.getReadLockCount() == 0;
@@ -1399,7 +1399,7 @@ public class BufferCache implements IBufferCacheInternal,
ILifeCycleComponent, I
}
} else {
cPage.invalidate();
- cPage.pinCount.decrementAndGet();
+ cPage.decrementAndGetPinCount();
if (DEBUG) {
assert cPage.pinCount.get() == 0;
assert cPage.latch.getReadLockCount() == 0;
diff --git
a/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/CachedPage.java
b/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/CachedPage.java
index b9d4e5a681..88dfaef643 100644
---
a/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/CachedPage.java
+++
b/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/CachedPage.java
@@ -62,7 +62,19 @@ public class CachedPage implements ICachedPageInternal {
}
public int incrementAndGetPinCount() {
- return pinCount.incrementAndGet();
+ int count = pinCount.incrementAndGet();
+ if (count <= 0) {
+ throw new IllegalStateException("incrementAndGet: Invalid
pinCount: " + count + " in page: " + this);
+ }
+ return count;
+ }
+
+ public int decrementAndGetPinCount() {
+ int count = pinCount.decrementAndGet();
+ if (count < 0) {
+ throw new IllegalStateException("decrementAndGet: Invalid
pinCount: " + count + " in page: " + this);
+ }
+ return count;
}
public CachedPage(int cpid, ByteBuffer buffer, IPageReplacementStrategy
pageReplacementStrategy) {
