You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@asterixdb.apache.org by mb...@apache.org on 2016/05/03 01:56:52 UTC
incubator-asterixdb git commit: BufferCache Concurrency Fixes
Repository: incubator-asterixdb
Updated Branches:
refs/heads/master 8b3412cb8 -> 784417619
BufferCache Concurrency Fixes
1. Fix thread-safety issues in ClockPageReplacementStrategy.findVictimByEviction
2. Fix race-condition between page evicition & file deletion
Change-Id: I01b4ab3000ae6f481f226c0df9fe876c6b16c7aa
Reviewed-on: https://asterix-gerrit.ics.uci.edu/835
Tested-by: Jenkins <je...@fulliautomatix.ics.uci.edu>
Reviewed-by: Yingyi Bu <bu...@gmail.com>
Project: http://git-wip-us.apache.org/repos/asf/incubator-asterixdb/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-asterixdb/commit/78441761
Tree: http://git-wip-us.apache.org/repos/asf/incubator-asterixdb/tree/78441761
Diff: http://git-wip-us.apache.org/repos/asf/incubator-asterixdb/diff/78441761
Branch: refs/heads/master
Commit: 78441761917bb427564bf5eaa2265e93adc45dcd
Parents: 8b3412c
Author: Michael Blow <mb...@apache.org>
Authored: Sun May 1 17:23:57 2016 -0400
Committer: Michael Blow <mi...@couchbase.com>
Committed: Mon May 2 16:56:52 2016 -0700
----------------------------------------------------------------------
.../storage/common/buffercache/BufferCache.java | 33 ++++++++++++--
.../storage/common/buffercache/CachedPage.java | 6 +--
.../ClockPageReplacementStrategy.java | 48 ++++++++++++--------
.../common/buffercache/ICachedPageInternal.java | 2 +-
4 files changed, 61 insertions(+), 28 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-asterixdb/blob/78441761/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 27d0423..6020d7b 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
@@ -283,6 +283,15 @@ public class BufferCache implements IBufferCacheInternal, ILifeCycleComponent {
*/
bucket.bucketLock.lock();
try {
+ if (!victim.pinCount.compareAndSet(0, 1)) {
+ continue;
+ }
+ // 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();
+ continue;
+ }
if (DEBUG) {
confiscateLock.lock();
try{
@@ -324,7 +333,12 @@ public class BufferCache implements IBufferCacheInternal, ILifeCycleComponent {
*/
bucket.bucketLock.lock();
try {
- if (victim.pinCount.get() != 1) {
+ if (!victim.pinCount.compareAndSet(0, 1)) {
+ continue;
+ }
+ // 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();
continue;
}
@@ -371,7 +385,12 @@ public class BufferCache implements IBufferCacheInternal, ILifeCycleComponent {
victimBucket.bucketLock.lock();
}
try {
- if (victim.pinCount.get() != 1) {
+ if (!victim.pinCount.compareAndSet(0, 1)) {
+ continue;
+ }
+ // 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();
continue;
}
@@ -992,7 +1011,12 @@ public class BufferCache implements IBufferCacheInternal, ILifeCycleComponent {
// find a page that would possibly be evicted anyway
// Case 1 from findPage()
if (victim.dpid < 0) { // new page
- if (victim.pinCount.get() != 1) {
+ if (!victim.pinCount.compareAndSet(0, 1)) {
+ continue;
+ }
+ // 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();
continue;
}
@@ -1013,8 +1037,7 @@ public class BufferCache implements IBufferCacheInternal, ILifeCycleComponent {
while (curr != null) {
if (curr == victim) { // we found where the victim
// resides in the hash table
- if (victim.pinCount.get() != 1) {
- victim.pinCount.decrementAndGet();
+ if (!victim.pinCount.compareAndSet(0, 1)) {
break;
}
// if this is the first page in the bucket
http://git-wip-us.apache.org/repos/asf/incubator-asterixdb/blob/78441761/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/CachedPage.java
----------------------------------------------------------------------
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 305b577..cda81ad 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
@@ -91,11 +91,11 @@ public class CachedPage implements ICachedPageInternal {
}
@Override
- public boolean pinIfGoodVictim() {
+ public boolean isGoodVictim() {
if (confiscated.get())
- return false; //i am not a good victim because i cant flush!
+ return false; // i am not a good victim because i cant flush!
else {
- return pinCount.compareAndSet(0, 1);
+ return pinCount.get() == 0;
}
}
http://git-wip-us.apache.org/repos/asf/incubator-asterixdb/blob/78441761/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/ClockPageReplacementStrategy.java
----------------------------------------------------------------------
diff --git a/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/ClockPageReplacementStrategy.java b/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/ClockPageReplacementStrategy.java
index 6a82b97..5c89bb6 100644
--- a/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/ClockPageReplacementStrategy.java
+++ b/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/ClockPageReplacementStrategy.java
@@ -63,7 +63,7 @@ public class ClockPageReplacementStrategy implements IPageReplacementStrategy {
@Override
public ICachedPageInternal findVictim() {
- ICachedPageInternal cachedPage = null;
+ ICachedPageInternal cachedPage;
if (numPages.get() >= maxAllowedNumPages) {
cachedPage = findVictimByEviction();
} else {
@@ -75,31 +75,40 @@ public class ClockPageReplacementStrategy implements IPageReplacementStrategy {
private ICachedPageInternal findVictimByEviction() {
//check if we're starved from confiscation
assert (maxAllowedNumPages > 0);
- int startClockPtr = clockPtr.get();
+ int clockPtr = advanceClock();
+ int startClockPtr = clockPtr;
+ int lastClockPtr = -1;
int cycleCount = 0;
- do {
- ICachedPageInternal cPage = bufferCache.getPage(clockPtr.get());
+ boolean looped = false;
+ while (true) {
+ ICachedPageInternal cPage = bufferCache.getPage(clockPtr);
/*
* We do two things here:
* 1. If the page has been accessed, then we skip it -- The CAS would return
* false if the current value is false which makes the page a possible candidate
* for replacement.
- * 2. We check with the buffer manager if it feels its a good idea to use this
+ * 2. We check with the buffer manager if it feels it's a good idea to use this
* page as a victim.
*/
AtomicBoolean accessedFlag = getPerPageObject(cPage);
if (!accessedFlag.compareAndSet(true, false)) {
- if (cPage.pinIfGoodVictim()) {
- return cPage;
+ if (cPage.isGoodVictim()) {
+ return cPage;
}
}
- advanceClock();
- if (clockPtr.get() == startClockPtr) {
- ++cycleCount;
+ if (clockPtr < lastClockPtr) {
+ looped = true;
}
- } while (cycleCount < MAX_UNSUCCESSFUL_CYCLE_COUNT);
- return null;
+ if (looped && clockPtr >= startClockPtr) {
+ if (++cycleCount >= MAX_UNSUCCESSFUL_CYCLE_COUNT) {
+ return null;
+ }
+ looped = false;
+ }
+ lastClockPtr = clockPtr;
+ clockPtr = advanceClock();
+ }
}
@Override
@@ -113,7 +122,7 @@ public class ClockPageReplacementStrategy implements IPageReplacementStrategy {
numPages.incrementAndGet();
AtomicBoolean accessedFlag = getPerPageObject(cPage);
if (!accessedFlag.compareAndSet(true, false)) {
- if (cPage.pinIfGoodVictim()) {
+ if (cPage.isGoodVictim()) {
return cPage;
}
}
@@ -121,17 +130,18 @@ public class ClockPageReplacementStrategy implements IPageReplacementStrategy {
}
//derived from RoundRobinAllocationPolicy in Apache directmemory
- private int advanceClock(){
- boolean clockInDial = false;
- int newClockPtr = 0;
+ private int advanceClock() {
+
+ boolean clockInDial;
+ int currClockPtr;
do
{
- int currClockPtr = clockPtr.get();
- newClockPtr = ( currClockPtr + 1 ) % numPages.get();
+ currClockPtr = clockPtr.get();
+ int newClockPtr = ( currClockPtr + 1 ) % numPages.get();
clockInDial = clockPtr.compareAndSet( currClockPtr, newClockPtr );
}
while ( !clockInDial );
- return newClockPtr;
+ return currClockPtr;
}
http://git-wip-us.apache.org/repos/asf/incubator-asterixdb/blob/78441761/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/ICachedPageInternal.java
----------------------------------------------------------------------
diff --git a/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/ICachedPageInternal.java b/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/ICachedPageInternal.java
index 497192d..6a6c2f5 100644
--- a/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/ICachedPageInternal.java
+++ b/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/ICachedPageInternal.java
@@ -25,6 +25,6 @@ public interface ICachedPageInternal extends ICachedPage {
public Object getReplacementStrategyObject();
- public boolean pinIfGoodVictim();
+ public boolean isGoodVictim();
}