You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@roller.apache.org by ag...@apache.org on 2006/11/07 19:16:12 UTC
svn commit: r472186 - in /incubator/roller/trunk: ./
src/org/apache/roller/business/hibernate/
src/org/apache/roller/business/runnable/ src/org/apache/roller/pojos/
Author: agilliland
Date: Tue Nov 7 10:16:11 2006
New Revision: 472186
URL: http://svn.apache.org/viewvc?view=rev&rev=472186
Log:
fixing some logic bugs in task locking code.
- removed methods getLastRun() and getNextRun() from thread manager
- added methods getNextRun() and getLeaseExpires() to task lock pojo
- fixed method isLocked() in task lock pojo to account for lease expiration
- fixed RollerTask logic to perform proper checking when attempting to acquire a lock
Modified:
incubator/roller/trunk/build.xml
incubator/roller/trunk/src/org/apache/roller/business/hibernate/HibernateThreadManagerImpl.java
incubator/roller/trunk/src/org/apache/roller/business/runnable/RollerTask.java
incubator/roller/trunk/src/org/apache/roller/business/runnable/ThreadManager.java
incubator/roller/trunk/src/org/apache/roller/business/runnable/ThreadManagerImpl.java
incubator/roller/trunk/src/org/apache/roller/pojos/TaskLockData.java
Modified: incubator/roller/trunk/build.xml
URL: http://svn.apache.org/viewvc/incubator/roller/trunk/build.xml?view=diff&rev=472186&r1=472185&r2=472186
==============================================================================
--- incubator/roller/trunk/build.xml (original)
+++ incubator/roller/trunk/build.xml Tue Nov 7 10:16:11 2006
@@ -965,6 +965,7 @@
<include name="org/apache/roller/business/PlanetManagerTest.class"/>
<include name="org/apache/roller/business/RefererTest.class"/>
<include name="org/apache/roller/business/HitCountTest.class"/>
+ <include name="org/apache/roller/business/TaskLockTest.class"/>
</fileset>
</batchtest>
</junit>
Modified: incubator/roller/trunk/src/org/apache/roller/business/hibernate/HibernateThreadManagerImpl.java
URL: http://svn.apache.org/viewvc/incubator/roller/trunk/src/org/apache/roller/business/hibernate/HibernateThreadManagerImpl.java?view=diff&rev=472186&r1=472185&r2=472186
==============================================================================
--- incubator/roller/trunk/src/org/apache/roller/business/hibernate/HibernateThreadManagerImpl.java (original)
+++ incubator/roller/trunk/src/org/apache/roller/business/hibernate/HibernateThreadManagerImpl.java Tue Nov 7 10:16:11 2006
@@ -26,6 +26,7 @@
import org.apache.roller.business.runnable.ThreadManagerImpl;
import org.apache.roller.business.runnable.RollerTask;
import org.apache.roller.business.RollerFactory;
+import org.apache.roller.business.runnable.LockException;
import org.apache.roller.pojos.TaskLockData;
import org.hibernate.Criteria;
import org.hibernate.HibernateException;
@@ -57,6 +58,11 @@
/**
* Try to aquire a lock for a given RollerTask.
+ *
+ * Remember, locks are only given if ...
+ * 1. the task is not currently locked and it's past the next scheduled
+ * run time for the particular task
+ * 2. the task *is* locked, but its lease has expired
*/
public boolean acquireLock(RollerTask task) {
@@ -77,9 +83,11 @@
return false;
}
- if(taskLock != null && !taskLock.isLocked()) {
+ Date now = new Date();
+ Date nextRun = taskLock.getNextRun(task.getInterval());
+ if( !taskLock.isLocked() && (nextRun == null || now.after(nextRun))) {
+
// set appropriate values for TaskLock and save it
- Date now = new Date();
taskLock.setLocked(true);
taskLock.setTimeAquired(now);
taskLock.setTimeLeased(task.getLeaseTime());
@@ -160,39 +168,6 @@
}
return locked;
- }
-
-
- public Date getLastRun(RollerTask task) {
-
- Date lastRun = null;
-
- try {
- TaskLockData taskLock = this.getTaskLockByName(task.getName());
- if(taskLock != null) {
- lastRun = taskLock.getLastRun();
- }
- } catch (RollerException ex) {
- log.warn("Error getting TaskLockData", ex);
- }
-
- return lastRun;
- }
-
-
- public Date getNextRun(RollerTask task) {
-
- Date lastRun = this.getLastRun(task);
- if(lastRun == null) {
- return null;
- }
-
- // calculate next run time
- Calendar cal = Calendar.getInstance();
- cal.setTime(lastRun);
- cal.add(Calendar.MINUTE, task.getInterval());
-
- return cal.getTime();
}
Modified: incubator/roller/trunk/src/org/apache/roller/business/runnable/RollerTask.java
URL: http://svn.apache.org/viewvc/incubator/roller/trunk/src/org/apache/roller/business/runnable/RollerTask.java?view=diff&rev=472186&r1=472185&r2=472186
==============================================================================
--- incubator/roller/trunk/src/org/apache/roller/business/runnable/RollerTask.java (original)
+++ incubator/roller/trunk/src/org/apache/roller/business/runnable/RollerTask.java Tue Nov 7 10:16:11 2006
@@ -122,49 +122,42 @@
boolean lockAcquired = false;
try {
- // is task already locked
+
if(!mgr.isLocked(this)) {
- // have we waited enough time since the last run?
- Date nextRun = mgr.getNextRun(this);
- Date now = new Date();
- if(nextRun == null || now.after(nextRun)) {
-
- log.debug("Attempting to acquire lock");
-
- // acquire lock
- lockAcquired = mgr.acquireLock(this);
-
- if(lockAcquired) {
- log.debug("Lock acquired, about to begin real work");
- } else {
- log.debug("Lock not acquired, assuming race condition");
- return;
- }
+
+ log.debug("Attempting to acquire lock");
+
+ lockAcquired = mgr.acquireLock(this);
+
+ // now if we have a lock then run the task
+ if(lockAcquired) {
+ log.debug("Lock acquired, running task");
+ this.runTask();
} else {
- log.debug("Interval time hasn't elapsed since last run, nothing to do");
+ log.debug("Lock NOT acquired, cannot continue");
+
+ // when we don't have a lock we can't continue, so bail
+ return;
}
+
} else {
log.debug("Task already locked, nothing to do");
}
-
- // now if we have a lock then run the task
- if(lockAcquired) {
- this.runTask();
- }
} catch (Exception ex) {
log.error("Unexpected exception running task", ex);
} finally {
+
if(lockAcquired) {
+
log.debug("Attempting to release lock");
- // release lock
boolean lockReleased = mgr.releaseLock(this);
if(lockReleased) {
log.debug("Lock released, time to sleep");
} else {
- log.error("Lock NOT released, something went wrong");
+ log.debug("Lock NOT released, some kind of problem");
}
}
Modified: incubator/roller/trunk/src/org/apache/roller/business/runnable/ThreadManager.java
URL: http://svn.apache.org/viewvc/incubator/roller/trunk/src/org/apache/roller/business/runnable/ThreadManager.java?view=diff&rev=472186&r1=472185&r2=472186
==============================================================================
--- incubator/roller/trunk/src/org/apache/roller/business/runnable/ThreadManager.java (original)
+++ incubator/roller/trunk/src/org/apache/roller/business/runnable/ThreadManager.java Tue Nov 7 10:16:11 2006
@@ -84,24 +84,6 @@
/**
- * What was the last time a task was run?
- *
- * @param task The RollerTask to check the last run time for.
- * @return Date The last time the task was run, or null if task has never been run.
- */
- public Date getLastRun(RollerTask task);
-
-
- /**
- * What is the next time a task is allowed to run?
- *
- * @param task The RollerTask to calculate the next run time for.
- * @return Date The next time the task is allowed to run, or null if task has never been run.
- */
- public Date getNextRun(RollerTask task);
-
-
- /**
* Shutdown.
*/
public void shutdown();
Modified: incubator/roller/trunk/src/org/apache/roller/business/runnable/ThreadManagerImpl.java
URL: http://svn.apache.org/viewvc/incubator/roller/trunk/src/org/apache/roller/business/runnable/ThreadManagerImpl.java?view=diff&rev=472186&r1=472185&r2=472186
==============================================================================
--- incubator/roller/trunk/src/org/apache/roller/business/runnable/ThreadManagerImpl.java (original)
+++ incubator/roller/trunk/src/org/apache/roller/business/runnable/ThreadManagerImpl.java Tue Nov 7 10:16:11 2006
@@ -119,12 +119,4 @@
return false;
}
- public Date getLastRun(RollerTask task) {
- return null;
- }
-
- public Date getNextRun(RollerTask task) {
- return null;
- }
-
}
Modified: incubator/roller/trunk/src/org/apache/roller/pojos/TaskLockData.java
URL: http://svn.apache.org/viewvc/incubator/roller/trunk/src/org/apache/roller/pojos/TaskLockData.java?view=diff&rev=472186&r1=472185&r2=472186
==============================================================================
--- incubator/roller/trunk/src/org/apache/roller/pojos/TaskLockData.java (original)
+++ incubator/roller/trunk/src/org/apache/roller/pojos/TaskLockData.java Tue Nov 7 10:16:11 2006
@@ -19,6 +19,7 @@
package org.apache.roller.pojos;
import java.io.Serializable;
+import java.util.Calendar;
import java.util.Date;
@@ -42,6 +43,45 @@
public TaskLockData() {}
+ /**
+ * Calculate the next allowed time the task managed by this lock would
+ * be allowed to run. i.e. lastRun + interval
+ */
+ public Date getNextRun(int interval) {
+
+ Date lastRun = this.getLastRun();
+ if(lastRun == null) {
+ return null;
+ }
+
+ // calculate next run time
+ Calendar cal = Calendar.getInstance();
+ cal.setTime(lastRun);
+ cal.add(Calendar.MINUTE, interval);
+
+ return cal.getTime();
+ }
+
+
+ /**
+ * Get the time the lease for this lock will expire, or null if this task
+ * lock is not currently locked.
+ */
+ public Date getLeaseExpires() {
+
+ if(!locked || timeAquired == null) {
+ return null;
+ }
+
+ // calculate lease expiration time
+ Calendar cal = Calendar.getInstance();
+ cal.setTime(timeAquired);
+ cal.add(Calendar.MINUTE, timeLeased);
+
+ return cal.getTime();
+ }
+
+
public void setData(PersistentObject otherData) {
TaskLockData other = (TaskLockData) otherData;
this.id = other.getId();
@@ -120,7 +160,17 @@
* @hibernate.property column="islocked" non-null="false" unique="false"
*/
public boolean isLocked() {
- return locked;
+
+ // this method requires a little extra logic because we return false
+ // even if a task is locked when it's lease has expired
+ if(!locked) {
+ return false;
+ }
+
+ Date now = new Date();
+ Date leaseExpiration = this.getLeaseExpires();
+
+ return now.before(leaseExpiration);
}
public void setLocked(boolean locked) {