hbase-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From st...@apache.org
Subject hbase git commit: HBASE-13007 Fix the test timeouts being caused by ChoreService (Jonathan Lawlor); ADDENDUM
Date Thu, 12 Feb 2015 04:46:52 GMT
Repository: hbase
Updated Branches:
  refs/heads/master b51f5dc12 -> 8aa4eeb1c


HBASE-13007 Fix the test timeouts being caused by ChoreService (Jonathan Lawlor); ADDENDUM


Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/8aa4eeb1
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/8aa4eeb1
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/8aa4eeb1

Branch: refs/heads/master
Commit: 8aa4eeb1c87104710fb55c1f0a2bf48644a2735d
Parents: b51f5dc
Author: stack <stack@apache.org>
Authored: Wed Feb 11 20:46:41 2015 -0800
Committer: stack <stack@apache.org>
Committed: Wed Feb 11 20:46:41 2015 -0800

----------------------------------------------------------------------
 .../org/apache/hadoop/hbase/ChoreService.java   |  2 +-
 .../org/apache/hadoop/hbase/ScheduledChore.java | 45 ++++++++-----
 .../apache/hadoop/hbase/TestChoreService.java   | 71 +-------------------
 3 files changed, 32 insertions(+), 86 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/8aa4eeb1/hbase-common/src/main/java/org/apache/hadoop/hbase/ChoreService.java
----------------------------------------------------------------------
diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/ChoreService.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/ChoreService.java
index 5e01c39..f565841 100644
--- a/hbase-common/src/main/java/org/apache/hadoop/hbase/ChoreService.java
+++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/ChoreService.java
@@ -161,7 +161,7 @@ public class ChoreService implements ChoreServicer {
 
   @Override
   public synchronized void cancelChore(ScheduledChore chore) {
-    cancelChore(chore, false);
+    cancelChore(chore, true);
   }
 
   @Override

http://git-wip-us.apache.org/repos/asf/hbase/blob/8aa4eeb1/hbase-common/src/main/java/org/apache/hadoop/hbase/ScheduledChore.java
----------------------------------------------------------------------
diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/ScheduledChore.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/ScheduledChore.java
index ccedcc7..16d080b 100644
--- a/hbase-common/src/main/java/org/apache/hadoop/hbase/ScheduledChore.java
+++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/ScheduledChore.java
@@ -164,26 +164,19 @@ public abstract class ScheduledChore implements Runnable {
     this.timeUnit = unit;
   }
 
-  synchronized void resetState() {
-    timeOfLastRun = -1;
-    timeOfThisRun = -1;
-    initialChoreComplete = false;
-  }
-
   /**
    * @see java.lang.Thread#run()
    */
   @Override
-  public synchronized void run() {
-    timeOfLastRun = timeOfThisRun;
-    timeOfThisRun = System.currentTimeMillis();
+  public void run() {
+    updateTimeTrackingBeforeRun();
     if (missedStartTime() && isScheduled()) {
-      choreServicer.onChoreMissedStartTime(this);
+      onChoreMissedStartTime();
       if (LOG.isInfoEnabled()) LOG.info("Chore: " + getName() + " missed its start time");
-    } else if (stopper.isStopped() || choreServicer == null || !isScheduled()) {
-      cancel();
+    } else if (stopper.isStopped() || !isScheduled()) {
+      cancel(false);
       cleanup();
-      LOG.info("Chore: " + getName() + " was stopped");
+      if (LOG.isInfoEnabled()) LOG.info("Chore: " + getName() + " was stopped");
     } else {
       try {
         if (!initialChoreComplete) {
@@ -192,9 +185,9 @@ public abstract class ScheduledChore implements Runnable {
           chore();
         }
       } catch (Throwable t) {
-        LOG.error("Caught error", t);
+        if (LOG.isErrorEnabled()) LOG.error("Caught error", t);
         if (this.stopper.isStopped()) {
-          cancel();
+          cancel(false);
           cleanup();
         }
       }
@@ -202,6 +195,24 @@ public abstract class ScheduledChore implements Runnable {
   }
 
   /**
+   * Update our time tracking members. Called at the start of an execution of this chore's
run()
+   * method so that a correct decision can be made as to whether or not we missed the start
time
+   */
+  private synchronized void updateTimeTrackingBeforeRun() {
+    timeOfLastRun = timeOfThisRun;
+    timeOfThisRun = System.currentTimeMillis();
+  }
+
+  /**
+   * Notify the ChoreService that this chore has missed its start time. Allows the ChoreService
to
+   * make the decision as to whether or not it would be worthwhile to increase the number
of core
+   * pool threads
+   */
+  private synchronized void onChoreMissedStartTime() {
+    if (choreServicer != null) choreServicer.onChoreMissedStartTime(this);
+  }
+
+  /**
    * @return How long has it been since this chore last run. Useful for checking if the chore
has
    *         missed its scheduled start time by too large of a margin
    */
@@ -248,7 +259,7 @@ public abstract class ScheduledChore implements Runnable {
   }
 
   public synchronized void cancel() {
-    cancel(false);
+    cancel(true);
   }
 
   public synchronized void cancel(boolean mayInterruptIfRunning) {
@@ -317,7 +328,7 @@ public abstract class ScheduledChore implements Runnable {
    * Override to run a task before we start looping.
    * @return true if initial chore was successful
    */
-  protected synchronized boolean initialChore() {
+  protected boolean initialChore() {
     // Default does nothing
     return true;
   }

http://git-wip-us.apache.org/repos/asf/hbase/blob/8aa4eeb1/hbase-common/src/test/java/org/apache/hadoop/hbase/TestChoreService.java
----------------------------------------------------------------------
diff --git a/hbase-common/src/test/java/org/apache/hadoop/hbase/TestChoreService.java b/hbase-common/src/test/java/org/apache/hadoop/hbase/TestChoreService.java
index 811d4d9..3529e94 100644
--- a/hbase-common/src/test/java/org/apache/hadoop/hbase/TestChoreService.java
+++ b/hbase-common/src/test/java/org/apache/hadoop/hbase/TestChoreService.java
@@ -423,7 +423,7 @@ public class TestChoreService {
     shutdownService(service);
   }
 
-  @Test (timeout=20000)
+  @Test(timeout = 30000)
   public void testCorePoolDecrease() throws InterruptedException {
     final int initialCorePoolSize = 3;
     ChoreService service = new ChoreService("testCorePoolDecrease", initialCorePoolSize);
@@ -456,6 +456,8 @@ public class TestChoreService {
       service.getNumberOfScheduledChores(), service.getCorePoolSize());
     assertEquals(service.getNumberOfChoresMissingStartTime(), 5);
 
+    // Now we begin to cancel the chores that caused an increase in the core thread pool
of the
+    // ChoreService. These cancellations should cause a decrease in the core thread pool.
     slowChore5.cancel();
     Thread.sleep(chorePeriod * 10);
     assertEquals(Math.max(ChoreService.MIN_CORE_POOL_SIZE, service.getNumberOfScheduledChores()),
@@ -486,44 +488,6 @@ public class TestChoreService {
       service.getCorePoolSize());
     assertEquals(service.getNumberOfChoresMissingStartTime(), 0);
 
-    slowChore1.resetState();
-    service.scheduleChore(slowChore1);
-    Thread.sleep(chorePeriod * 10);
-    assertEquals(Math.max(ChoreService.MIN_CORE_POOL_SIZE, service.getNumberOfScheduledChores()),
-      service.getCorePoolSize());
-    assertEquals(service.getNumberOfChoresMissingStartTime(), 1);
-
-    slowChore2.resetState();
-    service.scheduleChore(slowChore2);
-    Thread.sleep(chorePeriod * 10);
-    assertEquals(Math.max(ChoreService.MIN_CORE_POOL_SIZE, service.getNumberOfScheduledChores()),
-      service.getCorePoolSize());
-    assertEquals(service.getNumberOfChoresMissingStartTime(), 2);
-
-    DoNothingChore fastChore1 = new DoNothingChore("fastChore1", chorePeriod);
-    service.scheduleChore(fastChore1);
-    Thread.sleep(chorePeriod * 10);
-    assertEquals(service.getNumberOfChoresMissingStartTime(), 2);
-    assertEquals("Should increase", 3, service.getCorePoolSize());
-
-    DoNothingChore fastChore2 = new DoNothingChore("fastChore2", chorePeriod);
-    service.scheduleChore(fastChore2);
-    Thread.sleep(chorePeriod * 10);
-    assertEquals(service.getNumberOfChoresMissingStartTime(), 2);
-    assertEquals("Should increase", 3, service.getCorePoolSize());
-
-    DoNothingChore fastChore3 = new DoNothingChore("fastChore3", chorePeriod);
-    service.scheduleChore(fastChore3);
-    Thread.sleep(chorePeriod * 10);
-    assertEquals(service.getNumberOfChoresMissingStartTime(), 2);
-    assertEquals("Should not change", 3, service.getCorePoolSize());
-
-    DoNothingChore fastChore4 = new DoNothingChore("fastChore4", chorePeriod);
-    service.scheduleChore(fastChore4);
-    Thread.sleep(chorePeriod * 10);
-    assertEquals(service.getNumberOfChoresMissingStartTime(), 2);
-    assertEquals("Should not change", 3, service.getCorePoolSize());
-
     shutdownService(service);
   }
 
@@ -658,35 +622,6 @@ public class TestChoreService {
   }
 
   @Test (timeout=20000)
-  public void testScheduledChoreReset() throws InterruptedException {
-    final int period = 100;
-    ChoreService service = new ChoreService("testScheduledChoreReset");
-    ScheduledChore chore = new DoNothingChore("sampleChore", period);
-
-    // TRUE
-    assertTrue(!chore.isInitialChoreComplete());
-    assertTrue(chore.getTimeOfLastRun() == -1);
-    assertTrue(chore.getTimeOfThisRun() == -1);
-
-    service.scheduleChore(chore);
-    Thread.sleep(5 * period);
-
-    // FALSE
-    assertFalse(!chore.isInitialChoreComplete());
-    assertFalse(chore.getTimeOfLastRun() == -1);
-    assertFalse(chore.getTimeOfThisRun() == -1);
-
-    chore.resetState();
-
-    // TRUE
-    assertTrue(!chore.isInitialChoreComplete());
-    assertTrue(chore.getTimeOfLastRun() == -1);
-    assertTrue(chore.getTimeOfThisRun() == -1);
-
-    shutdownService(service);
-  }
-
-  @Test (timeout=20000)
   public void testChangingChoreServices() throws InterruptedException {
     final int period = 100;
     final int sleepTime = 10;


Mime
View raw message