You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by st...@apache.org on 2011/06/21 22:58:51 UTC
svn commit: r1138182 - in /hbase/branches/0.90: CHANGES.txt
src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
src/main/java/org/apache/hadoop/hbase/regionserver/Leases.java
Author: stack
Date: Tue Jun 21 20:58:51 2011
New Revision: 1138182
URL: http://svn.apache.org/viewvc?rev=1138182&view=rev
Log:
HBASE-2077 NullPointerException with an open scanner that expired causing an immediate region server shutdown -- part 2
Modified:
hbase/branches/0.90/CHANGES.txt
hbase/branches/0.90/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
hbase/branches/0.90/src/main/java/org/apache/hadoop/hbase/regionserver/Leases.java
Modified: hbase/branches/0.90/CHANGES.txt
URL: http://svn.apache.org/viewvc/hbase/branches/0.90/CHANGES.txt?rev=1138182&r1=1138181&r2=1138182&view=diff
==============================================================================
--- hbase/branches/0.90/CHANGES.txt (original)
+++ hbase/branches/0.90/CHANGES.txt Tue Jun 21 20:58:51 2011
@@ -41,6 +41,8 @@ Release 0.90.4 - Unreleased
HBASE-3989 Error occured while RegionServer report to Master "we are up"
should get master address again (Jieshan Bean)
HBASE-3995 HBASE-3946 broke TestMasterFailover
+ HBASE-2077 NullPointerException with an open scanner that expired causing
+ an immediate region server shutdown -- part 2.
IMPROVEMENT
Modified: hbase/branches/0.90/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.90/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java?rev=1138182&r1=1138181&r2=1138182&view=diff
==============================================================================
--- hbase/branches/0.90/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java (original)
+++ hbase/branches/0.90/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java Tue Jun 21 20:58:51 2011
@@ -1804,26 +1804,27 @@ public class HRegionServer implements HR
}
public Result[] next(final long scannerId, int nbRows) throws IOException {
+ String scannerName = String.valueOf(scannerId);
+ InternalScanner s = this.scanners.get(scannerName);
+ if (s == null) throw new UnknownScannerException("Name: " + scannerName);
try {
- String scannerName = String.valueOf(scannerId);
- InternalScanner s = this.scanners.get(scannerName);
- if (s == null) {
- throw new UnknownScannerException("Name: " + scannerName);
- }
+ checkOpen();
+ } catch (IOException e) {
+ // If checkOpen failed, server not running or filesystem gone,
+ // cancel this lease; filesystem is gone or we're closing or something.
try {
- checkOpen();
- } catch (IOException e) {
- // If checkOpen failed, server not running or filesystem gone,
- // cancel this lease; filesystem is gone or we're closing or something.
- try {
- this.leases.cancelLease(scannerName);
- } catch (LeaseException le) {
- LOG.info("Server shutting down and client tried to access missing scanner " +
- scannerName);
- }
- throw e;
+ this.leases.cancelLease(scannerName);
+ } catch (LeaseException le) {
+ LOG.info("Server shutting down and client tried to access missing scanner " +
+ scannerName);
}
- this.leases.renewLease(scannerName);
+ throw e;
+ }
+ Leases.Lease lease = null;
+ try {
+ // Remove lease while its being processed in server; protects against case
+ // where processing of request takes > lease expiration time.
+ lease = this.leases.removeLease(scannerName);
List<Result> results = new ArrayList<Result>(nbRows);
long currentScanResultSize = 0;
List<KeyValue> values = new ArrayList<KeyValue>();
@@ -1853,10 +1854,15 @@ public class HRegionServer implements HR
: results.toArray(new Result[0]);
} catch (Throwable t) {
if (t instanceof NotServingRegionException) {
- String scannerName = String.valueOf(scannerId);
this.scanners.remove(scannerName);
}
throw convertThrowableToIOE(cleanup(t));
+ } finally {
+ // We're done. On way out readd the above removed lease. Adding resets
+ // expiration time on lease.
+ if (this.scanners.containsKey(scannerName)) {
+ if (lease != null) this.leases.addLease(lease);
+ }
}
}
Modified: hbase/branches/0.90/src/main/java/org/apache/hadoop/hbase/regionserver/Leases.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.90/src/main/java/org/apache/hadoop/hbase/regionserver/Leases.java?rev=1138182&r1=1138181&r2=1138182&view=diff
==============================================================================
--- hbase/branches/0.90/src/main/java/org/apache/hadoop/hbase/regionserver/Leases.java (original)
+++ hbase/branches/0.90/src/main/java/org/apache/hadoop/hbase/regionserver/Leases.java Tue Jun 21 20:58:51 2011
@@ -140,16 +140,24 @@ public class Leases extends Thread {
*/
public void createLease(String leaseName, final LeaseListener listener)
throws LeaseStillHeldException {
- if (stopRequested) {
+ addLease(new Lease(leaseName, listener));
+ }
+
+ /**
+ * Inserts lease. Resets expiration before insertion.
+ * @param lease
+ * @throws LeaseStillHeldException
+ */
+ public void addLease(final Lease lease) throws LeaseStillHeldException {
+ if (this.stopRequested) {
return;
}
- Lease lease = new Lease(leaseName, listener,
- System.currentTimeMillis() + leasePeriod);
+ lease.setExpirationTime(System.currentTimeMillis() + this.leasePeriod);
synchronized (leaseQueue) {
- if (leases.containsKey(leaseName)) {
- throw new LeaseStillHeldException(leaseName);
+ if (leases.containsKey(lease.getLeaseName())) {
+ throw new LeaseStillHeldException(lease.getLeaseName());
}
- leases.put(leaseName, lease);
+ leases.put(lease.getLeaseName(), lease);
leaseQueue.add(lease);
}
}
@@ -189,7 +197,7 @@ public class Leases extends Thread {
// in a corrupt leaseQueue.
if (lease == null || !leaseQueue.remove(lease)) {
throw new LeaseException("lease '" + leaseName +
- "' does not exist or has already expired");
+ "' does not exist or has already expired");
}
lease.setExpirationTime(System.currentTimeMillis() + leasePeriod);
leaseQueue.add(lease);
@@ -198,26 +206,44 @@ public class Leases extends Thread {
/**
* Client explicitly cancels a lease.
- *
* @param leaseName name of lease
* @throws LeaseException
*/
public void cancelLease(final String leaseName) throws LeaseException {
+ removeLease(leaseName);
+ }
+
+ /**
+ * Remove named lease.
+ * Lease is removed from the list of leases and removed from the delay queue.
+ * Lease can be resinserted using {@link #addLease(Lease)}
+ *
+ * @param leaseName name of lease
+ * @throws LeaseException
+ * @return Removed lease
+ */
+ Lease removeLease(final String leaseName) throws LeaseException {
+ Lease lease = null;
synchronized (leaseQueue) {
- Lease lease = leases.remove(leaseName);
+ lease = leases.remove(leaseName);
if (lease == null) {
throw new LeaseException("lease '" + leaseName + "' does not exist");
}
leaseQueue.remove(lease);
}
+ return lease;
}
/** This class tracks a single Lease. */
- private static class Lease implements Delayed {
+ static class Lease implements Delayed {
private final String leaseName;
private final LeaseListener listener;
private long expirationTime;
+ Lease(final String leaseName, LeaseListener listener) {
+ this(leaseName, listener, 0);
+ }
+
Lease(final String leaseName, LeaseListener listener, long expirationTime) {
this.leaseName = leaseName;
this.listener = listener;
@@ -269,14 +295,5 @@ public class Leases extends Thread {
public void setExpirationTime(long expirationTime) {
this.expirationTime = expirationTime;
}
-
- /**
- * Get the expiration time for that lease
- * @return expiration time
- */
- public long getExpirationTime() {
- return this.expirationTime;
- }
-
}
-}
+}
\ No newline at end of file