hbase-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From st...@apache.org
Subject 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
Date Tue, 21 Jun 2011 20:58:51 GMT
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



Mime
View raw message