You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by ji...@apache.org on 2008/02/07 07:35:10 UTC
svn commit: r619288 - in /hadoop/hbase/trunk: CHANGES.txt
src/java/org/apache/hadoop/hbase/HMaster.java
src/java/org/apache/hadoop/hbase/HRegionServer.java
src/java/org/apache/hadoop/hbase/Leases.java
Author: jimk
Date: Wed Feb 6 22:35:09 2008
New Revision: 619288
URL: http://svn.apache.org/viewvc?rev=619288&view=rev
Log:
HBASE-415 Rewrite leases to use DelayedBlockingQueue instead of polling
Modified:
hadoop/hbase/trunk/CHANGES.txt
hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/HMaster.java
hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/HRegionServer.java
hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/Leases.java
Modified: hadoop/hbase/trunk/CHANGES.txt
URL: http://svn.apache.org/viewvc/hadoop/hbase/trunk/CHANGES.txt?rev=619288&r1=619287&r2=619288&view=diff
==============================================================================
--- hadoop/hbase/trunk/CHANGES.txt (original)
+++ hadoop/hbase/trunk/CHANGES.txt Wed Feb 6 22:35:09 2008
@@ -27,6 +27,7 @@
HADOOP-2555 Refactor the HTable#get and HTable#getRow methods to avoid
repetition of retry-on-failure logic (thanks to Peter Dolan and
Bryan Duxbury)
+ HBASE-415 Rewrite leases to use DelayedBlockingQueue instead of polling
Release 0.16.0
Modified: hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/HMaster.java
URL: http://svn.apache.org/viewvc/hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/HMaster.java?rev=619288&r1=619287&r2=619288&view=diff
==============================================================================
--- hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/HMaster.java (original)
+++ hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/HMaster.java Wed Feb 6 22:35:09 2008
@@ -46,6 +46,7 @@
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
+import org.apache.hadoop.fs.FileStatus;
import org.apache.hadoop.fs.FileSystem;
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.fs.PathFilter;
@@ -372,10 +373,10 @@
Path p = HStoreFile.getMapDir(tabledir, split.getEncodedName(),
family.getFamilyName());
- // Look for reference files. Call listPaths with an anonymous
+ // Look for reference files. Call listStatus with an anonymous
// instance of PathFilter.
- Path [] ps = fs.listPaths(p,
+ FileStatus [] ps = fs.listStatus(p,
new PathFilter () {
public boolean accept(Path path) {
return HStore.isReference(path);
@@ -1306,8 +1307,7 @@
loadToServers.put(load, servers);
if (!closed.get()) {
- long serverLabel = getServerLabel(s);
- serverLeases.createLease(serverLabel, serverLabel, new ServerExpirer(s));
+ serverLeases.createLease(s, new ServerExpirer(s));
}
return createConfigurationSubset();
@@ -1327,15 +1327,10 @@
return mw;
}
- private long getServerLabel(final String s) {
- return s.hashCode();
- }
-
/** {@inheritDoc} */
public HMsg[] regionServerReport(HServerInfo serverInfo, HMsg msgs[])
throws IOException {
String serverName = serverInfo.getServerAddress().toString().trim();
- long serverLabel = getServerLabel(serverName);
if (msgs.length > 0) {
if (msgs[0].getMsg() == HMsg.MSG_REPORT_EXITING) {
synchronized (serversToServerInfo) {
@@ -1348,7 +1343,7 @@
": MSG_REPORT_EXITING -- cancelling lease");
}
- if (cancelLease(serverName, serverLabel)) {
+ if (cancelLease(serverName)) {
// Only process the exit message if the server still has a lease.
// Otherwise we could end up processing the server exit twice.
LOG.info("Region server " + serverName +
@@ -1428,7 +1423,7 @@
}
synchronized (serversToServerInfo) {
- cancelLease(serverName, serverLabel);
+ cancelLease(serverName);
serversToServerInfo.notifyAll();
}
return new HMsg[]{new HMsg(HMsg.MSG_REGIONSERVER_STOP)};
@@ -1439,7 +1434,7 @@
// This will always succeed; otherwise, the fetch of serversToServerInfo
// would have failed above.
- serverLeases.renewLease(serverLabel, serverLabel);
+ serverLeases.renewLease(serverName);
// Refresh the info object and the load information
@@ -1476,7 +1471,7 @@
}
/** Cancel a server's lease and update its load information */
- private boolean cancelLease(final String serverName, final long serverLabel) {
+ private boolean cancelLease(final String serverName) {
boolean leaseCancelled = false;
HServerInfo info = serversToServerInfo.remove(serverName);
if (info != null) {
@@ -1487,7 +1482,7 @@
unassignRootRegion();
}
LOG.info("Cancelling lease for " + serverName);
- serverLeases.cancelLease(serverLabel, serverLabel);
+ serverLeases.cancelLease(serverName);
leaseCancelled = true;
// update load information
@@ -3120,20 +3115,20 @@
/*
* Data structure used to return results out of the toRowMap method.
*/
- private class RowMap {
+ class RowMap {
final Text row;
final SortedMap<Text, byte[]> map;
- private RowMap(final Text r, final SortedMap<Text, byte[]> m) {
+ RowMap(final Text r, final SortedMap<Text, byte[]> m) {
this.row = r;
this.map = m;
}
- private Text getRow() {
+ Text getRow() {
return this.row;
}
- private SortedMap<Text, byte[]> getMap() {
+ SortedMap<Text, byte[]> getMap() {
return this.map;
}
}
Modified: hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/HRegionServer.java
URL: http://svn.apache.org/viewvc/hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/HRegionServer.java?rev=619288&r1=619287&r2=619288&view=diff
==============================================================================
--- hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/HRegionServer.java (original)
+++ hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/HRegionServer.java Wed Feb 6 22:35:09 2008
@@ -1395,7 +1395,7 @@
if (s == null) {
throw new UnknownScannerException("Name: " + scannerName);
}
- this.leases.renewLease(scannerId, scannerId);
+ this.leases.renewLease(scannerName);
// Collect values to be returned here
HbaseMapWritable values = new HbaseMapWritable();
@@ -1458,7 +1458,7 @@
scanners.put(scannerName, s);
}
this.leases.
- createLease(scannerId, scannerId, new ScannerListener(scannerName));
+ createLease(scannerName, new ScannerListener(scannerName));
return scannerId;
} catch (IOException e) {
LOG.error("Error opening scanner (fsOk: " + this.fsOk + ")",
@@ -1482,7 +1482,7 @@
throw new UnknownScannerException(scannerName);
}
s.close();
- this.leases.cancelLease(scannerId, scannerId);
+ this.leases.cancelLease(scannerName);
} catch (IOException e) {
checkFileSystem();
throw e;
Modified: hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/Leases.java
URL: http://svn.apache.org/viewvc/hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/Leases.java?rev=619288&r1=619287&r2=619288&view=diff
==============================================================================
--- hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/Leases.java (original)
+++ hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/Leases.java Wed Feb 6 22:35:09 2008
@@ -21,9 +21,13 @@
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
-import java.io.*;
-import java.util.*;
-import java.util.concurrent.atomic.AtomicBoolean;
+
+import java.util.ConcurrentModificationException;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.Delayed;
+import java.util.concurrent.DelayQueue;
+import java.util.concurrent.TimeUnit;
/**
* Leases
@@ -39,19 +43,19 @@
* An instance of the Leases class will create a thread to do its dirty work.
* You should close() the instance if you want to clean up the thread properly.
*/
-public class Leases {
- protected static final Log LOG = LogFactory.getLog(Leases.class.getName());
-
- protected final int leasePeriod;
- protected final int leaseCheckFrequency;
- private final Thread leaseMonitorThread;
- protected final Map<LeaseName, Lease> leases =
- new HashMap<LeaseName, Lease>();
- protected final TreeSet<Lease> sortedLeases = new TreeSet<Lease>();
- protected AtomicBoolean stop = new AtomicBoolean(false);
+public class Leases extends Thread {
+ private static final Log LOG = LogFactory.getLog(Leases.class.getName());
+ private final int leasePeriod;
+ private final int leaseCheckFrequency;
+ private volatile DelayQueue<Lease> leaseQueue = new DelayQueue<Lease>();
+
+ protected final Map<String, Lease> leases = new HashMap<String, Lease>();
+ protected final Map<String, LeaseListener> listeners =
+ new HashMap<String, LeaseListener>();
+ private volatile boolean stopRequested = false;
/**
- * Creates a lease
+ * Creates a lease monitor
*
* @param leasePeriod - length of time (milliseconds) that the lease is valid
* @param leaseCheckFrequency - how often the lease should be checked
@@ -60,21 +64,39 @@
public Leases(final int leasePeriod, final int leaseCheckFrequency) {
this.leasePeriod = leasePeriod;
this.leaseCheckFrequency = leaseCheckFrequency;
- this.leaseMonitorThread =
- new LeaseMonitor(this.leaseCheckFrequency, this.stop);
- this.leaseMonitorThread.setDaemon(true);
}
- /** Starts the lease monitor */
- public void start() {
- leaseMonitorThread.start();
- }
-
- /**
- * @param name Set name on the lease checking daemon thread.
- */
- public void setName(final String name) {
- this.leaseMonitorThread.setName(name);
+ /** {@inheritDoc} */
+ @Override
+ public void run() {
+ while (!stopRequested || (stopRequested && leaseQueue.size() > 0) ) {
+ Lease lease = null;
+ try {
+ lease = leaseQueue.poll(leaseCheckFrequency, TimeUnit.MILLISECONDS);
+
+ } catch (InterruptedException e) {
+ continue;
+
+ } catch (ConcurrentModificationException e) {
+ continue;
+ }
+ if (lease == null) {
+ continue;
+ }
+ // A lease expired
+ LeaseListener listener = null;
+ synchronized (leaseQueue) {
+ String leaseName = lease.getLeaseName();
+ leases.remove(leaseName);
+ listener = listeners.remove(leaseName);
+ if (listener == null) {
+ LOG.error("lease listener is null for lease " + leaseName);
+ continue;
+ }
+ }
+ listener.leaseExpired();
+ }
+ close();
}
/**
@@ -85,20 +107,7 @@
* allocation of new leases.
*/
public void closeAfterLeasesExpire() {
- synchronized(this.leases) {
- while (this.leases.size() > 0) {
- LOG.info(Thread.currentThread().getName() + " " +
- Integer.toString(leases.size()) + " lease(s) " +
- "outstanding. Waiting for them to expire.");
- try {
- this.leases.wait(this.leaseCheckFrequency);
- } catch (InterruptedException e) {
- // continue
- }
- }
- }
- // Now call close since no leases outstanding.
- close();
+ this.stopRequested = true;
}
/**
@@ -107,271 +116,124 @@
*/
public void close() {
LOG.info(Thread.currentThread().getName() + " closing leases");
- this.stop.set(true);
- while (this.leaseMonitorThread.isAlive()) {
- try {
- this.leaseMonitorThread.interrupt();
- this.leaseMonitorThread.join();
- } catch (InterruptedException iex) {
- // Ignore
- }
- }
- synchronized(leases) {
- synchronized(sortedLeases) {
- leases.clear();
- sortedLeases.clear();
- }
+ this.stopRequested = true;
+ synchronized (leaseQueue) {
+ leaseQueue.clear();
+ leases.clear();
+ listeners.clear();
+ leaseQueue.notifyAll();
}
LOG.info(Thread.currentThread().getName() + " closed leases");
}
- /* A client obtains a lease... */
-
/**
* Obtain a lease
*
- * @param holderId id of lease holder
- * @param resourceId id of resource being leased
+ * @param leaseName name of the lease
* @param listener listener that will process lease expirations
*/
- public void createLease(final long holderId, final long resourceId,
- final LeaseListener listener) {
- LeaseName name = null;
- synchronized(leases) {
- synchronized(sortedLeases) {
- Lease lease = new Lease(holderId, resourceId, listener);
- name = lease.getLeaseName();
- if(leases.get(name) != null) {
- throw new AssertionError("Impossible state for createLease(): " +
- "Lease " + name + " is still held.");
- }
- leases.put(name, lease);
- sortedLeases.add(lease);
- }
+ public void createLease(String leaseName, final LeaseListener listener) {
+ if (stopRequested) {
+ return;
+ }
+ Lease lease = new Lease(leaseName, System.currentTimeMillis() + leasePeriod);
+ synchronized (leaseQueue) {
+ if (leases.containsKey(leaseName)) {
+ throw new IllegalStateException("lease '" + leaseName +
+ "' already exists");
+ }
+ leases.put(leaseName, lease);
+ listeners.put(leaseName, listener);
+ leaseQueue.add(lease);
}
-// if (LOG.isDebugEnabled()) {
-// LOG.debug("Created lease " + name);
-// }
}
- /* A client renews a lease... */
/**
* Renew a lease
*
- * @param holderId id of lease holder
- * @param resourceId id of resource being leased
- * @throws IOException
+ * @param leaseName name of lease
*/
- public void renewLease(final long holderId, final long resourceId)
- throws IOException {
- LeaseName name = null;
- synchronized(leases) {
- synchronized(sortedLeases) {
- name = createLeaseName(holderId, resourceId);
- Lease lease = leases.get(name);
- if (lease == null) {
- // It's possible that someone tries to renew the lease, but
- // it just expired a moment ago. So fail.
- throw new IOException("Cannot renew lease that is not held: " +
- name);
- }
- sortedLeases.remove(lease);
- lease.renew();
- sortedLeases.add(lease);
- }
+ public void renewLease(final String leaseName) {
+ synchronized (leaseQueue) {
+ Lease lease = leases.get(leaseName);
+ if (lease == null) {
+ throw new IllegalArgumentException("lease '" + leaseName +
+ "' does not exist");
+ }
+ leaseQueue.remove(lease);
+ lease.setExpirationTime(System.currentTimeMillis() + leasePeriod);
+ leaseQueue.add(lease);
}
-// if (LOG.isDebugEnabled()) {
-// LOG.debug("Renewed lease " + name);
-// }
}
/**
* Client explicitly cancels a lease.
*
- * @param holderId id of lease holder
- * @param resourceId id of resource being leased
+ * @param leaseName name of lease
*/
- public void cancelLease(final long holderId, final long resourceId) {
- LeaseName name = null;
- synchronized(leases) {
- synchronized(sortedLeases) {
- name = createLeaseName(holderId, resourceId);
- Lease lease = leases.get(name);
- if (lease == null) {
- // It's possible that someone tries to renew the lease, but
- // it just expired a moment ago. So just skip it.
- return;
- }
- sortedLeases.remove(lease);
- leases.remove(name);
+ public void cancelLease(final String leaseName) {
+ synchronized (leaseQueue) {
+ Lease lease = leases.remove(leaseName);
+ if (lease == null) {
+ throw new IllegalArgumentException("lease '" + leaseName +
+ "' does not exist");
}
+ leaseQueue.remove(lease);
+ listeners.remove(leaseName);
}
}
- /**
- * LeaseMonitor is a thread that expires Leases that go on too long.
- * Its a daemon thread.
- */
- class LeaseMonitor extends Chore {
- /**
- * @param p
- * @param s
- */
- public LeaseMonitor(int p, AtomicBoolean s) {
- super(p, s);
+ /** This class tracks a single Lease. */
+ private static class Lease implements Delayed {
+ private final String leaseName;
+ private long expirationTime;
+
+ Lease(final String leaseName, long expirationTime) {
+ this.leaseName = leaseName;
+ this.expirationTime = expirationTime;
}
- /** {@inheritDoc} */
- @Override
- protected void chore() {
- synchronized(leases) {
- synchronized(sortedLeases) {
- Lease top;
- while((sortedLeases.size() > 0)
- && ((top = sortedLeases.first()) != null)) {
- if(top.shouldExpire()) {
- leases.remove(top.getLeaseName());
- sortedLeases.remove(top);
- top.expired();
- } else {
- break;
- }
- }
- }
- }
- }
- }
-
- /*
- * A Lease name.
- * More lightweight than String or Text.
- */
- @SuppressWarnings("unchecked")
- class LeaseName implements Comparable {
- private final long holderId;
- private final long resourceId;
-
- LeaseName(final long hid, final long rid) {
- this.holderId = hid;
- this.resourceId = rid;
+ /** @return the lease name */
+ public String getLeaseName() {
+ return leaseName;
}
-
+
/** {@inheritDoc} */
@Override
public boolean equals(Object obj) {
- LeaseName other = (LeaseName)obj;
- return this.holderId == other.holderId &&
- this.resourceId == other.resourceId;
+ return this.hashCode() == ((Lease) obj).hashCode();
}
/** {@inheritDoc} */
@Override
public int hashCode() {
- // Copy OR'ing from javadoc for Long#hashCode.
- int result = (int)(this.holderId ^ (this.holderId >>> 32));
- result ^= (int)(this.resourceId ^ (this.resourceId >>> 32));
- return result;
+ return this.leaseName.hashCode();
}
-
+
/** {@inheritDoc} */
- @Override
- public String toString() {
- return Long.toString(this.holderId) + "/" +
- Long.toString(this.resourceId);
+ public long getDelay(TimeUnit unit) {
+ return unit.convert(this.expirationTime - System.currentTimeMillis(),
+ TimeUnit.MILLISECONDS);
}
/** {@inheritDoc} */
- public int compareTo(Object obj) {
- LeaseName other = (LeaseName)obj;
- if (this.holderId < other.holderId) {
- return -1;
- }
- if (this.holderId > other.holderId) {
- return 1;
- }
- // holderIds are equal
- if (this.resourceId < other.resourceId) {
- return -1;
- }
- if (this.resourceId > other.resourceId) {
- return 1;
- }
- // Objects are equal
- return 0;
- }
- }
-
- /** Create a lease id out of the holder and resource ids. */
- protected LeaseName createLeaseName(final long hid, final long rid) {
- return new LeaseName(hid, rid);
- }
+ public int compareTo(Delayed o) {
+ long delta = this.getDelay(TimeUnit.MILLISECONDS) -
+ o.getDelay(TimeUnit.MILLISECONDS);
- /** This class tracks a single Lease. */
- @SuppressWarnings("unchecked")
- private class Lease implements Comparable {
- final long holderId;
- final long resourceId;
- final LeaseListener listener;
- long lastUpdate;
- private LeaseName leaseId;
-
- Lease(final long holderId, final long resourceId,
- final LeaseListener listener) {
- this.holderId = holderId;
- this.resourceId = resourceId;
- this.listener = listener;
- renew();
- }
-
- synchronized LeaseName getLeaseName() {
- if (this.leaseId == null) {
- this.leaseId = createLeaseName(holderId, resourceId);
+ int value = 0;
+ if (delta > 0) {
+ value = 1;
+
+ } else if (delta < 0) {
+ value = -1;
}
- return this.leaseId;
- }
-
- boolean shouldExpire() {
- return (System.currentTimeMillis() - lastUpdate > leasePeriod);
+ return value;
}
-
- void renew() {
- this.lastUpdate = System.currentTimeMillis();
- }
-
- void expired() {
- LOG.info(Thread.currentThread().getName() + " lease expired " +
- getLeaseName());
- listener.leaseExpired();
- }
-
- /** {@inheritDoc} */
- @Override
- public boolean equals(Object obj) {
- return compareTo(obj) == 0;
- }
-
- /** {@inheritDoc} */
- @Override
- public int hashCode() {
- int result = this.getLeaseName().hashCode();
- result ^= this.lastUpdate;
- return result;
- }
-
- //////////////////////////////////////////////////////////////////////////////
- // Comparable
- //////////////////////////////////////////////////////////////////////////////
- /** {@inheritDoc} */
- public int compareTo(Object o) {
- Lease other = (Lease) o;
- if(this.lastUpdate < other.lastUpdate) {
- return -1;
- } else if(this.lastUpdate > other.lastUpdate) {
- return 1;
- } else {
- return this.getLeaseName().compareTo(other.getLeaseName());
- }
+ /** @param expirationTime the expirationTime to set */
+ public void setExpirationTime(long expirationTime) {
+ this.expirationTime = expirationTime;
}
}
}