Michael Blow has uploaded a new change for review.
https://asterix-gerrit.ics.uci.edu/780
Change subject: BufferCache Concurrency Fixes
......................................................................
BufferCache Concurrency Fixes
1. Fix thread-safety issues in ClockPageReplacementStrategy.findVictimByEviction
2. Fix race-condition between page evicition & file deletion
Change-Id: Ife86104c5d1dde597e1a990b7207fda4d4bef039
---
M
hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/BufferCache.java
M
hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/ClockPageReplacementStrategy.java
M
hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/IBufferCacheInternal.java
M
hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/IPageReplacementStrategy.java
4 files changed, 70 insertions(+), 12 deletions(-)
git pull ssh://asterix-gerrit.ics.uci.edu:29418/hyracks refs/changes/80/780/1
diff --git
a/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/BufferCache.java
b/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/BufferCache.java
index 27d0423..0b80233 100644
---
a/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/BufferCache.java
+++
b/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/BufferCache.java
@@ -246,7 +246,7 @@
* If we got here, the page was not in the hash table. Now we ask
* the page replacement strategy to find us a victim.
*/
- CachedPage victim = (CachedPage)
pageReplacementStrategy.findVictim();
+ CachedPage victim = (CachedPage)
pageReplacementStrategy.findVictim(dpid);
if (victim != null) {
/*
* We have a victim with the following invariants. 1. The dpid
@@ -956,6 +956,52 @@
}
@Override
+ public boolean pinIfGoodVictim(ICachedPageInternal cPage, long newDpid) {
+ CachedPage victim = (CachedPage)cPage;
+ long dpid = victim.getDiskPageId();
+ if (dpid < 0 || BufferedFileHandle.getFileId(dpid) ==
BufferedFileHandle.getFileId(newDpid)) {
+ return victim.pinIfGoodVictim();
+ } else {
+ // prevent race condition with invalidateIfFileIdMatch, lock
bucket before pinning, invalidate dpid
+ // before unlock
+ int pageHash = hash(victim.getDiskPageId());
+ CacheBucket bucket = pageMap[pageHash];
+ bucket.bucketLock.lock();
+ try {
+ if (victim.pinIfGoodVictim()) {
+ boolean found = false;
+ if (bucket.cachedPage == victim) {
+ bucket.cachedPage = victim.next;
+ found = true;
+ } else {
+ CachedPage prev = bucket.cachedPage;
+ CachedPage curr = bucket.cachedPage.next;
+ // traverse the bucket's linked list to find the
victim.
+ while (curr != null) {
+ if (curr == victim) { // we found where the victim
resides in the hash table
+ // if this is the first page in the bucket
+ prev.next = curr.next;
+ curr.next = null;
+ found = true;
+ break;
+ }
+ // go to the next entry
+ prev = curr;
+ curr = curr.next;
+ }
+ }
+ assert found;
+ victim.dpid = -1;
+ return true;
+ }
+ } finally {
+ bucket.bucketLock.unlock();
+ }
+ }
+ return false;
+ }
+
+ @Override
public void dumpState(OutputStream os) throws IOException {
os.write(dumpState().getBytes());
}
@@ -984,7 +1030,7 @@
while (true) {
int startCleanedCount = cleanerThread.cleanedCount;
ICachedPage returnPage = null;
- CachedPage victim = (CachedPage)
pageReplacementStrategy.findVictim();
+ CachedPage victim = (CachedPage)
pageReplacementStrategy.findVictim(dpid);
if (victim != null) {
if(DEBUG) {
assert !victim.confiscated.get();
diff --git
a/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/ClockPageReplacementStrategy.java
b/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/ClockPageReplacementStrategy.java
index 6a82b97..0d205cd 100644
---
a/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/ClockPageReplacementStrategy.java
+++
b/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/ClockPageReplacementStrategy.java
@@ -62,23 +62,28 @@
}
@Override
- public ICachedPageInternal findVictim() {
+ public ICachedPageInternal findVictim(long newDpid) {
ICachedPageInternal cachedPage = null;
if (numPages.get() >= maxAllowedNumPages) {
- cachedPage = findVictimByEviction();
+ cachedPage = findVictimByEviction(newDpid);
} else {
cachedPage = allocatePage();
}
return cachedPage;
}
- private ICachedPageInternal findVictimByEviction() {
+ private ICachedPageInternal findVictimByEviction(long newDpid) {
//check if we're starved from confiscation
assert (maxAllowedNumPages > 0);
- int startClockPtr = clockPtr.get();
+ int startClockPtr = -1;
+ int clockPtr = -1;
+ int lastClockPtr;
int cycleCount = 0;
+ boolean looped = false;
do {
- ICachedPageInternal cPage = bufferCache.getPage(clockPtr.get());
+ lastClockPtr = clockPtr;
+ clockPtr = advanceClock();
+ ICachedPageInternal cPage = bufferCache.getPage(clockPtr);
/*
* We do two things here:
@@ -90,13 +95,18 @@
*/
AtomicBoolean accessedFlag = getPerPageObject(cPage);
if (!accessedFlag.compareAndSet(true, false)) {
- if (cPage.pinIfGoodVictim()) {
- return cPage;
+ if (bufferCache.pinIfGoodVictim(cPage, newDpid)) {
+ return cPage;
}
}
- advanceClock();
- if (clockPtr.get() == startClockPtr) {
+
+ if (startClockPtr == -1) {
+ startClockPtr = clockPtr;
+ } else if (clockPtr < lastClockPtr) {
+ looped = true;
+ } else if (looped && clockPtr >= startClockPtr) {
++cycleCount;
+ looped = false;
}
} while (cycleCount < MAX_UNSUCCESSFUL_CYCLE_COUNT);
return null;
diff --git
a/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/IBufferCacheInternal.java
b/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/IBufferCacheInternal.java
index ec05542..f3dc8a4 100644
---
a/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/IBufferCacheInternal.java
+++
b/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/IBufferCacheInternal.java
@@ -24,4 +24,6 @@
public ICachedPageInternal getPage(int cpid);
public void addPage(ICachedPageInternal page);
+
+ boolean pinIfGoodVictim(ICachedPageInternal cPage, long newDpid);
}
diff --git
a/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/IPageReplacementStrategy.java
b/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/IPageReplacementStrategy.java
index 661d363..be1ae37 100644
---
a/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/IPageReplacementStrategy.java
+++
b/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/IPageReplacementStrategy.java
@@ -29,7 +29,7 @@
public void adviseWontNeed(ICachedPageInternal cPage);
- public ICachedPageInternal findVictim();
+ public ICachedPageInternal findVictim(long newDpid);
public int getNumPages();
--
To view, visit https://asterix-gerrit.ics.uci.edu/780
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ife86104c5d1dde597e1a990b7207fda4d4bef039
Gerrit-PatchSet: 1
Gerrit-Project: hyracks
Gerrit-Branch: master
Gerrit-Owner: Michael Blow <[email protected]>
