You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@roller.apache.org by Mitesh Meswani <Mi...@Sun.COM> on 2007/01/13 01:00:03 UTC
Re: svn commit: r495640 - in /incubator/roller/trunk:
metadata/database/ src/org/apache/roller/business/hibernate/
src/org/apache/roller/business/pings/ src/org/apache/roller/business/runnable/
src/org/apache/roller/planet/tasks/ tests/org/apache/roller/bu...
Hi Allen,
I think you have missed to checking corresponding changes to
TaskLockData.java. The class needs to have property "client".
Regards,
Mitesh
agilliland@apache.org wrote:
> Author: agilliland
> Date: Fri Jan 12 08:58:10 2007
> New Revision: 495640
>
> URL: http://svn.apache.org/viewvc?view=rev&rev=495640
> Log:
> Fixes for race conditions in lock acquisition via HibernateThreadManagerImpl from Jira issue ROL-1306. A number of fixes are included in this commit ...
>
> 1. Added a database uniqueness constraint on the roller_tasklock.name column to
> prevent any possibility of there being duplicate insertions for a given task lease.
>
> 2. Updated the acquireLock() method to use a more robust acquisition method based on a single sql update statement as suggested by Elias. This should adequately eliminate race conditions when 2 or more members of a cluster attempt to acquire a lock at the same time.
>
> 3. Updated the acquireLock() method to perform all lease activities based on database time, which should negate any potential clock skew issues between cluster
> members.
>
> 4. Introduced a new clientId attribute for a task which makes it possible for each member of a cluster to be uniquely identified with regards to ownership of a
> lease.
>
>
>
> Modified:
> incubator/roller/trunk/metadata/database/310-to-320-migration.vm
> incubator/roller/trunk/metadata/database/createdb.vm
> incubator/roller/trunk/src/org/apache/roller/business/hibernate/HibernateThreadManagerImpl.java
> incubator/roller/trunk/src/org/apache/roller/business/pings/PingQueueTask.java
> incubator/roller/trunk/src/org/apache/roller/business/runnable/ResetHitCountsTask.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/TurnoverReferersTask.java
> incubator/roller/trunk/src/org/apache/roller/planet/tasks/RefreshEntriesTask.java
> incubator/roller/trunk/src/org/apache/roller/planet/tasks/SyncWebsitesTask.java
> incubator/roller/trunk/tests/org/apache/roller/business/TaskLockTest.java
> incubator/roller/trunk/web/WEB-INF/classes/roller.properties
>
> Modified: incubator/roller/trunk/metadata/database/310-to-320-migration.vm
> URL: http://svn.apache.org/viewvc/incubator/roller/trunk/metadata/database/310-to-320-migration.vm?view=diff&rev=495640&r1=495639&r2=495640
> ==============================================================================
> --- incubator/roller/trunk/metadata/database/310-to-320-migration.vm (original)
> +++ incubator/roller/trunk/metadata/database/310-to-320-migration.vm Fri Jan 12 08:58:10 2007
> @@ -33,6 +33,12 @@
> -- add new status option 'SCHEDULED' for future published entries
> update weblogentry set status = 'SCHEDULED' where pubtime > now();
>
> +-- add new client column to roller_tasklock table
> +#addColumnNull("roller_tasklock" "client" "varchar(255)")
> +
> +-- add a uniqueness constraint on roller_tasklock.name
> +alter table roller_tasklock add constraint rtl_name_uq unique ( name$!INDEXSIZE );
> +
> -- some various indexes to improve performance
> create index rhc_dailyhits_idx on roller_hitcounts( dailyhits );
> create index we_combo1_idx on weblogentry(status, pubtime, websiteid);
>
> Modified: incubator/roller/trunk/metadata/database/createdb.vm
> URL: http://svn.apache.org/viewvc/incubator/roller/trunk/metadata/database/createdb.vm?view=diff&rev=495640&r1=495639&r2=495640
> ==============================================================================
> --- incubator/roller/trunk/metadata/database/createdb.vm (original)
> +++ incubator/roller/trunk/metadata/database/createdb.vm Fri Jan 12 08:58:10 2007
> @@ -398,8 +398,10 @@
> islocked $BOOLEAN_SQL_TYPE_FALSE,
> timeacquired $TIMESTAMP_SQL_TYPE_NULL,
> timeleased integer,
> - lastrun $TIMESTAMP_SQL_TYPE_NULL
> + lastrun $TIMESTAMP_SQL_TYPE_NULL,
> + client varchar(255)
> );
> +alter table roller_tasklock add constraint rtl_name_uq unique ( name$!INDEXSIZE );
> create index rtl_taskname_idx on roller_tasklock( name );
>
> create table roller_hitcounts (
>
> 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=495640&r1=495639&r2=495640
> ==============================================================================
> --- incubator/roller/trunk/src/org/apache/roller/business/hibernate/HibernateThreadManagerImpl.java (original)
> +++ incubator/roller/trunk/src/org/apache/roller/business/hibernate/HibernateThreadManagerImpl.java Fri Jan 12 08:58:10 2007
> @@ -18,7 +18,6 @@
>
> package org.apache.roller.business.hibernate;
>
> -import java.util.Calendar;
> import java.util.Date;
> import org.apache.commons.logging.Log;
> import org.apache.commons.logging.LogFactory;
> @@ -29,6 +28,7 @@
> import org.apache.roller.pojos.TaskLockData;
> import org.hibernate.Criteria;
> import org.hibernate.HibernateException;
> +import org.hibernate.Query;
> import org.hibernate.Session;
> import org.hibernate.criterion.Expression;
>
> @@ -57,53 +57,60 @@
>
> /**
> * 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) {
>
> - boolean lockAcquired = false;
> -
> + // query for existing lease record first
> TaskLockData taskLock = null;
> try {
> taskLock = this.getTaskLockByName(task.getName());
>
> - // null here just means hasn't been initialized yet
> if(taskLock == null) {
> + // insert an empty record, then we will actually acquire the
> + // lease below using an update statement
> taskLock = new TaskLockData();
> taskLock.setName(task.getName());
> - taskLock.setLocked(false);
> + taskLock.setTimeAquired(new Date(0));
> + taskLock.setTimeLeased(0);
> +
> + // save it and flush
> + this.saveTaskLock(taskLock);
> + RollerFactory.getRoller().flush();
> }
> +
> } catch (RollerException ex) {
> - log.warn("Error getting TaskLockData", ex);
> + log.warn("Error getting or inserting TaskLockData", ex);
> return false;
> }
>
> - 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
> - taskLock.setLocked(true);
> - taskLock.setTimeAquired(now);
> - taskLock.setTimeLeased(task.getLeaseTime());
> - taskLock.setLastRun(now);
> + // try to acquire lease
> + try {
> + Session session = ((HibernatePersistenceStrategy)this.strategy).getSession();
> + String queryHQL = "update TaskLockData "+
> + "set client=:client, timeacquired=current_timestamp(), timeleased=:timeleased "+
> + "where name=:name and timeacquired=:timeacquired "+
> + "and :leaseends < current_timestamp()";
> + Query query = session.createQuery(queryHQL);
> + query.setString("client", task.getClientId());
> + query.setString("timeleased", ""+task.getLeaseTime());
> + query.setString("name", task.getName());
> + query.setTimestamp("timeacquired", taskLock.getTimeAquired());
> + query.setTimestamp("leaseends", new Date(taskLock.getTimeAquired().getTime()+(60000*taskLock.getTimeLeased())));
> + int result = query.executeUpdate();
>
> - try {
> - // save it *and* flush
> - this.saveTaskLock(taskLock);
> - RollerFactory.getRoller().flush();
> - lockAcquired = true;
> - } catch (RollerException ex) {
> - log.warn("Error saving TaskLockData", ex);
> - lockAcquired = false;
> + // this may not be needed
> + RollerFactory.getRoller().flush();
> +
> + if(result == 1) {
> + return true;
> }
> +
> + } catch (Exception e) {
> + log.warn("Error obtaining lease, assuming race condition.", e);
> + return false;
> }
>
> - return lockAcquired;
> + return false;
> }
>
>
> @@ -112,61 +119,43 @@
> */
> public boolean releaseLock(RollerTask task) {
>
> - boolean lockReleased = false;
> -
> + // query for existing lease record first
> TaskLockData taskLock = null;
> try {
> taskLock = this.getTaskLockByName(task.getName());
> +
> + if(taskLock == null) {
> + return false;
> + }
> +
> } catch (RollerException ex) {
> log.warn("Error getting TaskLockData", ex);
> return false;
> }
>
> - if(taskLock != null && taskLock.isLocked()) {
> - // set appropriate values for TaskLock and save it
> - Date now = new Date();
> - taskLock.setLocked(false);
> -
> - try {
> - // save it *and* flush
> - this.saveTaskLock(taskLock);
> - RollerFactory.getRoller().flush();
> - lockReleased = true;
> - } catch (RollerException ex) {
> - log.warn("Error saving TaskLockData", ex);
> - lockReleased = false;
> - }
> - } else if(taskLock != null && !taskLock.isLocked()) {
> - // if lock is already released then don't fret about it
> - lockReleased = true;
> - }
> -
> - return lockReleased;
> - }
> -
> -
> - /**
> - * Is a task currently locked?
> - */
> - public boolean isLocked(RollerTask task) {
> -
> - // default is "true"!
> - boolean locked = true;
> -
> + // try to release lease, just set lease time to 0
> try {
> - TaskLockData taskLock = this.getTaskLockByName(task.getName());
> - if(taskLock != null) {
> - locked = taskLock.isLocked();
> - } else {
> - // if taskLock is null, but we didn't get an exception then
> - // that means this lock hasn't been initialized yet
> - locked = false;
> + Session session = ((HibernatePersistenceStrategy)this.strategy).getSession();
> + String queryHQL = "update TaskLockData set timeLeased=0 "+
> + "where name=:name and client=:client";
> + Query query = session.createQuery(queryHQL);
> + query.setString("name", task.getName());
> + query.setString("client", task.getClientId());
> + int result = query.executeUpdate();
> +
> + // this may not be needed
> + RollerFactory.getRoller().flush();
> +
> + if(result == 1) {
> + return true;
> }
> - } catch (RollerException ex) {
> - log.warn("Error getting TaskLockData", ex);
> +
> + } catch (Exception e) {
> + log.warn("Error releasing lease.", e);
> + return false;
> }
>
> - return locked;
> + return false;
> }
>
>
>
> Modified: incubator/roller/trunk/src/org/apache/roller/business/pings/PingQueueTask.java
> URL: http://svn.apache.org/viewvc/incubator/roller/trunk/src/org/apache/roller/business/pings/PingQueueTask.java?view=diff&rev=495640&r1=495639&r2=495640
> ==============================================================================
> --- incubator/roller/trunk/src/org/apache/roller/business/pings/PingQueueTask.java (original)
> +++ incubator/roller/trunk/src/org/apache/roller/business/pings/PingQueueTask.java Fri Jan 12 08:58:10 2007
> @@ -41,6 +41,10 @@
>
> private static Log log = LogFactory.getLog(PingQueueTask.class);
>
> + // a unique id for this specific task instance
> + // this is meant to be unique for each client in a clustered environment
> + private String clientId = null;
> +
> // a String description of when to start this task
> private String startTimeDesc = "immediate";
>
> @@ -55,6 +59,10 @@
> return "PingQueueTask";
> }
>
> + public String getClientId() {
> + return clientId;
> + }
> +
> public Date getStartTime(Date currentTime) {
> return getAdjustedTime(currentTime, startTimeDesc);
> }
> @@ -72,6 +80,12 @@
>
> // get relevant props
> Properties props = this.getTaskProperties();
> +
> + // extract clientId
> + String client = props.getProperty("clientId");
> + if(client != null) {
> + this.clientId = client;
> + }
>
> // extract start time
> String startTimeStr = props.getProperty("startTime");
>
> Modified: incubator/roller/trunk/src/org/apache/roller/business/runnable/ResetHitCountsTask.java
> URL: http://svn.apache.org/viewvc/incubator/roller/trunk/src/org/apache/roller/business/runnable/ResetHitCountsTask.java?view=diff&rev=495640&r1=495639&r2=495640
> ==============================================================================
> --- incubator/roller/trunk/src/org/apache/roller/business/runnable/ResetHitCountsTask.java (original)
> +++ incubator/roller/trunk/src/org/apache/roller/business/runnable/ResetHitCountsTask.java Fri Jan 12 08:58:10 2007
> @@ -34,6 +34,10 @@
>
> private static Log log = LogFactory.getLog(ResetHitCountsTask.class);
>
> + // a unique id for this specific task instance
> + // this is meant to be unique for each client in a clustered environment
> + private String clientId = null;
> +
> // a String description of when to start this task
> private String startTimeDesc = "startOfDay";
>
> @@ -48,6 +52,10 @@
> return "ResetHitCountsTask";
> }
>
> + public String getClientId() {
> + return clientId;
> + }
> +
> public Date getStartTime(Date currentTime) {
> return getAdjustedTime(currentTime, startTimeDesc);
> }
> @@ -65,6 +73,12 @@
>
> // get relevant props
> Properties props = this.getTaskProperties();
> +
> + // extract clientId
> + String client = props.getProperty("clientId");
> + if(client != null) {
> + this.clientId = client;
> + }
>
> // extract start time
> String startTimeStr = props.getProperty("startTime");
>
> 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=495640&r1=495639&r2=495640
> ==============================================================================
> --- incubator/roller/trunk/src/org/apache/roller/business/runnable/RollerTask.java (original)
> +++ incubator/roller/trunk/src/org/apache/roller/business/runnable/RollerTask.java Fri Jan 12 08:58:10 2007
> @@ -60,6 +60,16 @@
>
>
> /**
> + * Get the unique id representing a specific instance of a task. This is
> + * important for tasks being run in a clustered environment so that a
> + * lease can be associated with a single cluster member.
> + *
> + * @return The unique client identifier for this task instance.
> + */
> + public abstract String getClientId();
> +
> +
> + /**
> * When should this task be started? The task is given the current time
> * so that it may determine a start time relative to the current time,
> * such as the end of the day or hour.
> @@ -112,36 +122,21 @@
> */
> public final void run() {
>
> - ThreadManager mgr = null;
> - try {
> - mgr = RollerFactory.getRoller().getThreadManager();
> - } catch (Exception ex) {
> - log.fatal("Unable to obtain ThreadManager", ex);
> - return;
> - }
> + ThreadManager mgr = RollerFactory.getRoller().getThreadManager();
>
> boolean lockAcquired = false;
> try {
> + log.debug("Attempting to acquire lock");
>
> - if(!mgr.isLocked(this)) {
> -
> - 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("Lock NOT acquired, cannot continue");
> -
> - // when we don't have a lock we can't continue, so bail
> - return;
> - }
> -
> + 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("Task already locked, nothing to do");
> + log.debug("Lock NOT acquired, cannot continue");
> + return;
> }
>
> } catch (Exception ex) {
> @@ -191,6 +186,9 @@
> RollerConfig.getProperty(key));
> }
> }
> +
> + // special addition for clientId property that applies to all tasks
> + taskProps.setProperty("clientId", RollerConfig.getProperty("tasks.clientId"));
>
> return taskProps;
> }
>
> 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=495640&r1=495639&r2=495640
> ==============================================================================
> --- incubator/roller/trunk/src/org/apache/roller/business/runnable/ThreadManager.java (original)
> +++ incubator/roller/trunk/src/org/apache/roller/business/runnable/ThreadManager.java Fri Jan 12 08:58:10 2007
> @@ -75,15 +75,6 @@
>
>
> /**
> - * Is a task currently locked?
> - *
> - * @param task The RollerTask to check the lock state for.
> - * @return boolean True if task is locked, False otherwise.
> - */
> - public boolean isLocked(RollerTask task);
> -
> -
> - /**
> * Shutdown.
> */
> public void shutdown();
>
> Modified: incubator/roller/trunk/src/org/apache/roller/business/runnable/TurnoverReferersTask.java
> URL: http://svn.apache.org/viewvc/incubator/roller/trunk/src/org/apache/roller/business/runnable/TurnoverReferersTask.java?view=diff&rev=495640&r1=495639&r2=495640
> ==============================================================================
> --- incubator/roller/trunk/src/org/apache/roller/business/runnable/TurnoverReferersTask.java (original)
> +++ incubator/roller/trunk/src/org/apache/roller/business/runnable/TurnoverReferersTask.java Fri Jan 12 08:58:10 2007
> @@ -34,6 +34,10 @@
>
> private static Log log = LogFactory.getLog(TurnoverReferersTask.class);
>
> + // a unique id for this specific task instance
> + // this is meant to be unique for each client in a clustered environment
> + private String clientId = null;
> +
> // a String description of when to start this task
> private String startTimeDesc = "startOfDay";
>
> @@ -48,6 +52,10 @@
> return "TurnoverReferersTask";
> }
>
> + public String getClientId() {
> + return clientId;
> + }
> +
> public Date getStartTime(Date currentTime) {
> return getAdjustedTime(currentTime, startTimeDesc);
> }
> @@ -65,6 +73,12 @@
>
> // get relevant props
> Properties props = this.getTaskProperties();
> +
> + // extract clientId
> + String client = props.getProperty("clientId");
> + if(client != null) {
> + this.clientId = client;
> + }
>
> // extract start time
> String startTimeStr = props.getProperty("startTime");
>
> Modified: incubator/roller/trunk/src/org/apache/roller/planet/tasks/RefreshEntriesTask.java
> URL: http://svn.apache.org/viewvc/incubator/roller/trunk/src/org/apache/roller/planet/tasks/RefreshEntriesTask.java?view=diff&rev=495640&r1=495639&r2=495640
> ==============================================================================
> --- incubator/roller/trunk/src/org/apache/roller/planet/tasks/RefreshEntriesTask.java (original)
> +++ incubator/roller/trunk/src/org/apache/roller/planet/tasks/RefreshEntriesTask.java Fri Jan 12 08:58:10 2007
> @@ -35,6 +35,10 @@
>
> private static Log log = LogFactory.getLog(RefreshEntriesTask.class);
>
> + // a unique id for this specific task instance
> + // this is meant to be unique for each client in a clustered environment
> + private String clientId = null;
> +
> // a String description of when to start this task
> private String startTimeDesc = "startOfHour";
>
> @@ -49,6 +53,10 @@
> return "RefreshEntriesTask";
> }
>
> + public String getClientId() {
> + return clientId;
> + }
> +
> public Date getStartTime(Date currentTime) {
> return getAdjustedTime(currentTime, startTimeDesc);
> }
> @@ -66,6 +74,12 @@
>
> // get relevant props
> Properties props = this.getTaskProperties();
> +
> + // extract clientId
> + String client = props.getProperty("clientId");
> + if(client != null) {
> + this.clientId = client;
> + }
>
> // extract start time
> String startTimeStr = props.getProperty("startTime");
>
> Modified: incubator/roller/trunk/src/org/apache/roller/planet/tasks/SyncWebsitesTask.java
> URL: http://svn.apache.org/viewvc/incubator/roller/trunk/src/org/apache/roller/planet/tasks/SyncWebsitesTask.java?view=diff&rev=495640&r1=495639&r2=495640
> ==============================================================================
> --- incubator/roller/trunk/src/org/apache/roller/planet/tasks/SyncWebsitesTask.java (original)
> +++ incubator/roller/trunk/src/org/apache/roller/planet/tasks/SyncWebsitesTask.java Fri Jan 12 08:58:10 2007
> @@ -47,6 +47,10 @@
>
> private static Log log = LogFactory.getLog(SyncWebsitesTask.class);
>
> + // a unique id for this specific task instance
> + // this is meant to be unique for each client in a clustered environment
> + private String clientId = "unspecifiedClientId";
> +
> // a String description of when to start this task
> private String startTimeDesc = "startOfDay";
>
> @@ -61,6 +65,10 @@
> return "SyncWebsitesTask";
> }
>
> + public String getClientId() {
> + return clientId;
> + }
> +
> public Date getStartTime(Date currentTime) {
> return getAdjustedTime(currentTime, startTimeDesc);
> }
> @@ -78,6 +86,12 @@
>
> // get relevant props
> Properties props = this.getTaskProperties();
> +
> + // extract clientId
> + String client = props.getProperty("clientId");
> + if(client != null) {
> + this.clientId = client;
> + }
>
> // extract start time
> String startTimeStr = props.getProperty("startTime");
>
> Modified: incubator/roller/trunk/tests/org/apache/roller/business/TaskLockTest.java
> URL: http://svn.apache.org/viewvc/incubator/roller/trunk/tests/org/apache/roller/business/TaskLockTest.java?view=diff&rev=495640&r1=495639&r2=495640
> ==============================================================================
> --- incubator/roller/trunk/tests/org/apache/roller/business/TaskLockTest.java (original)
> +++ incubator/roller/trunk/tests/org/apache/roller/business/TaskLockTest.java Fri Jan 12 08:58:10 2007
> @@ -66,23 +66,15 @@
> RollerTask task = new TestTask();
>
> // try to acquire a lock
> - boolean lockAcquired = mgr.acquireLock(task);
> - assertTrue(lockAcquired);
> + assertTrue(mgr.acquireLock(task));
> TestUtils.endSession(true);
>
> // make sure task is locked
> - boolean stillLocked = mgr.isLocked(task);
> - assertTrue(stillLocked);
> + assertFalse(mgr.acquireLock(task));
> TestUtils.endSession(true);
>
> // try to release a lock
> - boolean lockReleased = mgr.releaseLock(task);
> - assertTrue(lockReleased);
> - TestUtils.endSession(true);
> -
> - // make sure task is not locked
> - stillLocked = mgr.isLocked(task);
> - assertFalse(stillLocked);
> + assertTrue(mgr.releaseLock(task));
> TestUtils.endSession(true);
> }
>
> @@ -90,6 +82,7 @@
> class TestTask extends RollerTask {
>
> public String getName() { return "TestTask"; }
> + public String getClientId() { return "TestTaskClientId"; }
> public Date getStartTime(Date current) { return current; }
> public int getLeaseTime() { return 300; }
> public int getInterval() { return 1800; }
>
> Modified: incubator/roller/trunk/web/WEB-INF/classes/roller.properties
> URL: http://svn.apache.org/viewvc/incubator/roller/trunk/web/WEB-INF/classes/roller.properties?view=diff&rev=495640&r1=495639&r2=495640
> ==============================================================================
> --- incubator/roller/trunk/web/WEB-INF/classes/roller.properties (original)
> +++ incubator/roller/trunk/web/WEB-INF/classes/roller.properties Fri Jan 12 08:58:10 2007
> @@ -269,6 +269,9 @@
> # Tasks which are enabled. Only tasks listed here will be run.
> tasks.enabled=ResetHitCountsTask,TurnoverReferersTask,PingQueueTask
>
> +# client identifier. should be unique for each instance in a cluster.
> +tasks.clientId=defaultClientId
> +
> # Reset hit counts
> tasks.ResetHitCountsTask.class=org.apache.roller.business.runnable.ResetHitCountsTask
> tasks.ResetHitCountsTask.startTime=startOfDay
>
>
>
Re: svn commit: r495640 - in /incubator/roller/trunk:
metadata/database/ src/org/apache/roller/business/hibernate/
src/org/apache/roller/business/pings/ src/org/apache/roller/business/runnable/
src/org/apache/roller/planet/tasks/ tests/org/apache/roller/bu
Posted by Allen Gilliland <al...@sun.com>.
I think those messages should be okay actually, because they are only
logged in the event that an exception is thrown. Under normal
conditions when any client attempts to obtain a lease and fails because
another client has the lease, that is noted by the fact that the result
of the sql update call is 0 and no message is logged. Only if there is
an actual error does it result in a WARN message.
-- Allen
Anil Gangolli wrote:
>
> I'm still reviewing the detailed semantics, but I noticed that there are
> several messages logged at "warn" level; won't some of these occur
> regularly on the hosts that just happen not to have the lease?
>
> --a.
>
>
> ----- Original Message ----- From: "Allen Gilliland"
> <al...@sun.com>
> To: <ro...@incubator.apache.org>
> Sent: Friday, January 12, 2007 4:02 PM
> Subject: Re: svn commit: r495640 - in /incubator/roller/trunk:
> metadata/database/ src/org/apache/roller/business/hibernate/
> src/org/apache/roller/business/pings/
> src/org/apache/roller/business/runnable/
> src/org/apache/roller/planet/tasks/ tests/org/apache/roller/bu...
>
>
>> actually, there isn't a current need for that property to be accessed
>> via the TaskLockData pojo, but there's no harm in having it so I just
>> added it now.
>>
>> -- Allen
>>
>>
>> Mitesh Meswani wrote:
>>> Hi Allen,
>>>
>>> I think you have missed to checking corresponding changes to
>>> TaskLockData.java. The class needs to have property "client".
>>>
>>> Regards,
>>> Mitesh
>>>
>>> agilliland@apache.org wrote:
>>>> Author: agilliland
>>>> Date: Fri Jan 12 08:58:10 2007
>>>> New Revision: 495640
>>>>
>>>> URL: http://svn.apache.org/viewvc?view=rev&rev=495640
>>>> Log:
>>>> Fixes for race conditions in lock acquisition via
>>>> HibernateThreadManagerImpl from Jira issue ROL-1306. A number of
>>>> fixes are included in this commit ...
>>>>
>>>> 1. Added a database uniqueness constraint on the
>>>> roller_tasklock.name column to
>>>> prevent any possibility of there being duplicate insertions for a
>>>> given task lease.
>>>>
>>>> 2. Updated the acquireLock() method to use a more robust acquisition
>>>> method based on a single sql update statement as suggested by Elias.
>>>> This should adequately eliminate race conditions when 2 or more
>>>> members of a cluster attempt to acquire a lock at the same time.
>>>>
>>>> 3. Updated the acquireLock() method to perform all lease activities
>>>> based on database time, which should negate any potential clock skew
>>>> issues between cluster
>>>> members.
>>>>
>>>> 4. Introduced a new clientId attribute for a task which makes it
>>>> possible for each member of a cluster to be uniquely identified with
>>>> regards to ownership of a
>>>> lease.
>>>>
>>>>
>>>>
>>>> Modified:
>>>> incubator/roller/trunk/metadata/database/310-to-320-migration.vm
>>>> incubator/roller/trunk/metadata/database/createdb.vm
>>>>
>>>> incubator/roller/trunk/src/org/apache/roller/business/hibernate/HibernateThreadManagerImpl.java
>>>>
>>>>
>>>> incubator/roller/trunk/src/org/apache/roller/business/pings/PingQueueTask.java
>>>>
>>>>
>>>> incubator/roller/trunk/src/org/apache/roller/business/runnable/ResetHitCountsTask.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/TurnoverReferersTask.java
>>>>
>>>>
>>>> incubator/roller/trunk/src/org/apache/roller/planet/tasks/RefreshEntriesTask.java
>>>>
>>>>
>>>> incubator/roller/trunk/src/org/apache/roller/planet/tasks/SyncWebsitesTask.java
>>>>
>>>>
>>>> incubator/roller/trunk/tests/org/apache/roller/business/TaskLockTest.java
>>>>
>>>> incubator/roller/trunk/web/WEB-INF/classes/roller.properties
>>>>
>>>> Modified:
>>>> incubator/roller/trunk/metadata/database/310-to-320-migration.vm
>>>> URL:
>>>> http://svn.apache.org/viewvc/incubator/roller/trunk/metadata/database/310-to-320-migration.vm?view=diff&rev=495640&r1=495639&r2=495640
>>>>
>>>> ==============================================================================
>>>>
>>>> --- incubator/roller/trunk/metadata/database/310-to-320-migration.vm
>>>> (original)
>>>> +++ incubator/roller/trunk/metadata/database/310-to-320-migration.vm
>>>> Fri Jan 12 08:58:10 2007
>>>> @@ -33,6 +33,12 @@
>>>> -- add new status option 'SCHEDULED' for future published entries
>>>> update weblogentry set status = 'SCHEDULED' where pubtime > now();
>>>> +-- add new client column to roller_tasklock table
>>>> +#addColumnNull("roller_tasklock" "client" "varchar(255)")
>>>> +
>>>> +-- add a uniqueness constraint on roller_tasklock.name
>>>> +alter table roller_tasklock add constraint rtl_name_uq unique (
>>>> name$!INDEXSIZE );
>>>> +
>>>> -- some various indexes to improve performance
>>>> create index rhc_dailyhits_idx on roller_hitcounts( dailyhits );
>>>> create index we_combo1_idx on weblogentry(status, pubtime, websiteid);
>>>>
>>>> Modified: incubator/roller/trunk/metadata/database/createdb.vm
>>>> URL:
>>>> http://svn.apache.org/viewvc/incubator/roller/trunk/metadata/database/createdb.vm?view=diff&rev=495640&r1=495639&r2=495640
>>>>
>>>> ==============================================================================
>>>>
>>>> --- incubator/roller/trunk/metadata/database/createdb.vm (original)
>>>> +++ incubator/roller/trunk/metadata/database/createdb.vm Fri Jan 12
>>>> 08:58:10 2007
>>>> @@ -398,8 +398,10 @@
>>>> islocked $BOOLEAN_SQL_TYPE_FALSE,
>>>> timeacquired $TIMESTAMP_SQL_TYPE_NULL,
>>>> timeleased integer,
>>>> - lastrun $TIMESTAMP_SQL_TYPE_NULL
>>>> + lastrun $TIMESTAMP_SQL_TYPE_NULL,
>>>> + client varchar(255)
>>>> );
>>>> +alter table roller_tasklock add constraint rtl_name_uq unique (
>>>> name$!INDEXSIZE );
>>>> create index rtl_taskname_idx on roller_tasklock( name );
>>>> create table roller_hitcounts (
>>>>
>>>> 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=495640&r1=495639&r2=495640
>>>>
>>>> ==============================================================================
>>>>
>>>> ---
>>>> incubator/roller/trunk/src/org/apache/roller/business/hibernate/HibernateThreadManagerImpl.java
>>>> (original)
>>>> +++
>>>> incubator/roller/trunk/src/org/apache/roller/business/hibernate/HibernateThreadManagerImpl.java
>>>> Fri Jan 12 08:58:10 2007
>>>> @@ -18,7 +18,6 @@
>>>> package org.apache.roller.business.hibernate;
>>>> -import java.util.Calendar;
>>>> import java.util.Date;
>>>> import org.apache.commons.logging.Log;
>>>> import org.apache.commons.logging.LogFactory;
>>>> @@ -29,6 +28,7 @@
>>>> import org.apache.roller.pojos.TaskLockData;
>>>> import org.hibernate.Criteria;
>>>> import org.hibernate.HibernateException;
>>>> +import org.hibernate.Query;
>>>> import org.hibernate.Session;
>>>> import org.hibernate.criterion.Expression;
>>>> @@ -57,53 +57,60 @@
>>>> /**
>>>> * 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) {
>>>> - boolean lockAcquired = false;
>>>> - + // query for existing lease record first
>>>> TaskLockData taskLock = null;
>>>> try {
>>>> taskLock = this.getTaskLockByName(task.getName());
>>>> - // null here just means hasn't been
>>>> initialized yet
>>>> if(taskLock == null) {
>>>> + // insert an empty record, then we will actually
>>>> acquire the
>>>> + // lease below using an update statement taskLock =
>>>> new TaskLockData();
>>>> taskLock.setName(task.getName());
>>>> - taskLock.setLocked(false);
>>>> + taskLock.setTimeAquired(new Date(0));
>>>> + taskLock.setTimeLeased(0);
>>>> + + // save it and flush
>>>> + this.saveTaskLock(taskLock);
>>>> + RollerFactory.getRoller().flush();
>>>> }
>>>> + } catch (RollerException ex) {
>>>> - log.warn("Error getting TaskLockData", ex);
>>>> + log.warn("Error getting or inserting TaskLockData", ex);
>>>> return false;
>>>> }
>>>> - 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
>>>> - taskLock.setLocked(true);
>>>> - taskLock.setTimeAquired(now);
>>>> - taskLock.setTimeLeased(task.getLeaseTime());
>>>> - taskLock.setLastRun(now);
>>>> + // try to acquire lease
>>>> + try {
>>>> + Session session =
>>>> ((HibernatePersistenceStrategy)this.strategy).getSession();
>>>> + String queryHQL = "update TaskLockData "+
>>>> + "set client=:client,
>>>> timeacquired=current_timestamp(), timeleased=:timeleased "+
>>>> + "where name=:name and
>>>> timeacquired=:timeacquired "+
>>>> + "and :leaseends < current_timestamp()";
>>>> + Query query = session.createQuery(queryHQL);
>>>> + query.setString("client", task.getClientId());
>>>> + query.setString("timeleased", ""+task.getLeaseTime());
>>>> + query.setString("name", task.getName());
>>>> + query.setTimestamp("timeacquired",
>>>> taskLock.getTimeAquired());
>>>> + query.setTimestamp("leaseends", new
>>>> Date(taskLock.getTimeAquired().getTime()+(60000*taskLock.getTimeLeased())));
>>>>
>>>> + int result = query.executeUpdate();
>>>> - try {
>>>> - // save it *and* flush
>>>> - this.saveTaskLock(taskLock);
>>>> - RollerFactory.getRoller().flush();
>>>> - lockAcquired = true;
>>>> - } catch (RollerException ex) {
>>>> - log.warn("Error saving TaskLockData", ex);
>>>> - lockAcquired = false;
>>>> + // this may not be needed
>>>> + RollerFactory.getRoller().flush();
>>>> + + if(result == 1) {
>>>> + return true;
>>>> }
>>>> + + } catch (Exception e) {
>>>> + log.warn("Error obtaining lease, assuming race
>>>> condition.", e);
>>>> + return false;
>>>> }
>>>> - return lockAcquired;
>>>> + return false;
>>>> }
>>>> @@ -112,61 +119,43 @@
>>>> */
>>>> public boolean releaseLock(RollerTask task) {
>>>> - boolean lockReleased = false;
>>>> - + // query for existing lease record first
>>>> TaskLockData taskLock = null;
>>>> try {
>>>> taskLock = this.getTaskLockByName(task.getName());
>>>> + + if(taskLock == null) {
>>>> + return false;
>>>> + }
>>>> + } catch (RollerException ex) {
>>>> log.warn("Error getting TaskLockData", ex);
>>>> return false;
>>>> }
>>>> - if(taskLock != null && taskLock.isLocked()) {
>>>> - // set appropriate values for TaskLock and save it
>>>> - Date now = new Date();
>>>> - taskLock.setLocked(false);
>>>> - - try {
>>>> - // save it *and* flush
>>>> - this.saveTaskLock(taskLock);
>>>> - RollerFactory.getRoller().flush();
>>>> - lockReleased = true;
>>>> - } catch (RollerException ex) {
>>>> - log.warn("Error saving TaskLockData", ex);
>>>> - lockReleased = false;
>>>> - }
>>>> - } else if(taskLock != null && !taskLock.isLocked()) {
>>>> - // if lock is already released then don't fret about it
>>>> - lockReleased = true;
>>>> - }
>>>> - - return lockReleased;
>>>> - }
>>>> - - - /**
>>>> - * Is a task currently locked?
>>>> - */
>>>> - public boolean isLocked(RollerTask task) {
>>>> - - // default is "true"!
>>>> - boolean locked = true;
>>>> - + // try to release lease, just set lease time to 0
>>>> try {
>>>> - TaskLockData taskLock =
>>>> this.getTaskLockByName(task.getName());
>>>> - if(taskLock != null) {
>>>> - locked = taskLock.isLocked();
>>>> - } else {
>>>> - // if taskLock is null, but we didn't get an
>>>> exception then
>>>> - // that means this lock hasn't been initialized yet
>>>> - locked = false;
>>>> + Session session =
>>>> ((HibernatePersistenceStrategy)this.strategy).getSession();
>>>> + String queryHQL = "update TaskLockData set timeLeased=0 "+
>>>> + "where name=:name and client=:client";
>>>> + Query query = session.createQuery(queryHQL);
>>>> + query.setString("name", task.getName());
>>>> + query.setString("client", task.getClientId());
>>>> + int result = query.executeUpdate();
>>>> + + // this may not be needed
>>>> + RollerFactory.getRoller().flush();
>>>> + + if(result == 1) {
>>>> + return true;
>>>> }
>>>> - } catch (RollerException ex) {
>>>> - log.warn("Error getting TaskLockData", ex);
>>>> + + } catch (Exception e) {
>>>> + log.warn("Error releasing lease.", e);
>>>> + return false;
>>>> }
>>>> - return locked;
>>>> + return false;
>>>> }
>>>> Modified:
>>>> incubator/roller/trunk/src/org/apache/roller/business/pings/PingQueueTask.java
>>>>
>>>> URL:
>>>> http://svn.apache.org/viewvc/incubator/roller/trunk/src/org/apache/roller/business/pings/PingQueueTask.java?view=diff&rev=495640&r1=495639&r2=495640
>>>>
>>>> ==============================================================================
>>>>
>>>> ---
>>>> incubator/roller/trunk/src/org/apache/roller/business/pings/PingQueueTask.java
>>>> (original)
>>>> +++
>>>> incubator/roller/trunk/src/org/apache/roller/business/pings/PingQueueTask.java
>>>> Fri Jan 12 08:58:10 2007
>>>> @@ -41,6 +41,10 @@
>>>> private static Log log =
>>>> LogFactory.getLog(PingQueueTask.class);
>>>> + // a unique id for this specific task instance
>>>> + // this is meant to be unique for each client in a clustered
>>>> environment
>>>> + private String clientId = null;
>>>> + // a String description of when to start this task
>>>> private String startTimeDesc = "immediate";
>>>> @@ -55,6 +59,10 @@
>>>> return "PingQueueTask";
>>>> }
>>>> + public String getClientId() {
>>>> + return clientId;
>>>> + }
>>>> + public Date getStartTime(Date currentTime) {
>>>> return getAdjustedTime(currentTime, startTimeDesc);
>>>> }
>>>> @@ -72,6 +80,12 @@
>>>> // get relevant props
>>>> Properties props = this.getTaskProperties();
>>>> + + // extract clientId
>>>> + String client = props.getProperty("clientId");
>>>> + if(client != null) {
>>>> + this.clientId = client;
>>>> + }
>>>> // extract start time
>>>> String startTimeStr = props.getProperty("startTime");
>>>>
>>>> Modified:
>>>> incubator/roller/trunk/src/org/apache/roller/business/runnable/ResetHitCountsTask.java
>>>>
>>>> URL:
>>>> http://svn.apache.org/viewvc/incubator/roller/trunk/src/org/apache/roller/business/runnable/ResetHitCountsTask.java?view=diff&rev=495640&r1=495639&r2=495640
>>>>
>>>> ==============================================================================
>>>>
>>>> ---
>>>> incubator/roller/trunk/src/org/apache/roller/business/runnable/ResetHitCountsTask.java
>>>> (original)
>>>> +++
>>>> incubator/roller/trunk/src/org/apache/roller/business/runnable/ResetHitCountsTask.java
>>>> Fri Jan 12 08:58:10 2007
>>>> @@ -34,6 +34,10 @@
>>>> private static Log log =
>>>> LogFactory.getLog(ResetHitCountsTask.class);
>>>> + // a unique id for this specific task instance
>>>> + // this is meant to be unique for each client in a clustered
>>>> environment
>>>> + private String clientId = null;
>>>> + // a String description of when to start this task
>>>> private String startTimeDesc = "startOfDay";
>>>> @@ -48,6 +52,10 @@
>>>> return "ResetHitCountsTask";
>>>> }
>>>> + public String getClientId() {
>>>> + return clientId;
>>>> + }
>>>> + public Date getStartTime(Date currentTime) {
>>>> return getAdjustedTime(currentTime, startTimeDesc);
>>>> }
>>>> @@ -65,6 +73,12 @@
>>>> // get relevant props
>>>> Properties props = this.getTaskProperties();
>>>> + + // extract clientId
>>>> + String client = props.getProperty("clientId");
>>>> + if(client != null) {
>>>> + this.clientId = client;
>>>> + }
>>>> // extract start time
>>>> String startTimeStr = props.getProperty("startTime");
>>>>
>>>> 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=495640&r1=495639&r2=495640
>>>>
>>>> ==============================================================================
>>>>
>>>> ---
>>>> incubator/roller/trunk/src/org/apache/roller/business/runnable/RollerTask.java
>>>> (original)
>>>> +++
>>>> incubator/roller/trunk/src/org/apache/roller/business/runnable/RollerTask.java
>>>> Fri Jan 12 08:58:10 2007
>>>> @@ -60,6 +60,16 @@
>>>> /**
>>>> + * Get the unique id representing a specific instance of a
>>>> task. This is
>>>> + * important for tasks being run in a clustered environment so
>>>> that a + * lease can be associated with a single cluster member.
>>>> + *
>>>> + * @return The unique client identifier for this task instance.
>>>> + */
>>>> + public abstract String getClientId();
>>>> + + + /**
>>>> * When should this task be started? The task is given the
>>>> current time
>>>> * so that it may determine a start time relative to the
>>>> current time, * such as the end of the day or hour.
>>>> @@ -112,36 +122,21 @@
>>>> */
>>>> public final void run() {
>>>> - ThreadManager mgr = null;
>>>> - try {
>>>> - mgr = RollerFactory.getRoller().getThreadManager();
>>>> - } catch (Exception ex) {
>>>> - log.fatal("Unable to obtain ThreadManager", ex);
>>>> - return;
>>>> - }
>>>> + ThreadManager mgr =
>>>> RollerFactory.getRoller().getThreadManager();
>>>> boolean lockAcquired = false;
>>>> try {
>>>> + log.debug("Attempting to acquire lock");
>>>> - if(!mgr.isLocked(this)) {
>>>> - - 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("Lock NOT acquired, cannot continue");
>>>> - - // when we don't have a
>>>> lock we can't continue, so bail
>>>> - return;
>>>> - }
>>>> - + 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("Task already locked, nothing to do");
>>>> + log.debug("Lock NOT acquired, cannot continue");
>>>> + return;
>>>> }
>>>> } catch (Exception ex) {
>>>> @@ -191,6 +186,9 @@
>>>> RollerConfig.getProperty(key));
>>>> }
>>>> }
>>>> + + // special addition for clientId property that
>>>> applies to all tasks
>>>> + taskProps.setProperty("clientId",
>>>> RollerConfig.getProperty("tasks.clientId"));
>>>> return taskProps;
>>>> }
>>>>
>>>> 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=495640&r1=495639&r2=495640
>>>>
>>>> ==============================================================================
>>>>
>>>> ---
>>>> incubator/roller/trunk/src/org/apache/roller/business/runnable/ThreadManager.java
>>>> (original)
>>>> +++
>>>> incubator/roller/trunk/src/org/apache/roller/business/runnable/ThreadManager.java
>>>> Fri Jan 12 08:58:10 2007
>>>> @@ -75,15 +75,6 @@
>>>> /**
>>>> - * Is a task currently locked?
>>>> - * - * @param task The RollerTask to check the lock state for.
>>>> - * @return boolean True if task is locked, False otherwise.
>>>> - */
>>>> - public boolean isLocked(RollerTask task);
>>>> - - - /**
>>>> * Shutdown.
>>>> */
>>>> public void shutdown();
>>>>
>>>> Modified:
>>>> incubator/roller/trunk/src/org/apache/roller/business/runnable/TurnoverReferersTask.java
>>>>
>>>> URL:
>>>> http://svn.apache.org/viewvc/incubator/roller/trunk/src/org/apache/roller/business/runnable/TurnoverReferersTask.java?view=diff&rev=495640&r1=495639&r2=495640
>>>>
>>>> ==============================================================================
>>>>
>>>> ---
>>>> incubator/roller/trunk/src/org/apache/roller/business/runnable/TurnoverReferersTask.java
>>>> (original)
>>>> +++
>>>> incubator/roller/trunk/src/org/apache/roller/business/runnable/TurnoverReferersTask.java
>>>> Fri Jan 12 08:58:10 2007
>>>> @@ -34,6 +34,10 @@
>>>> private static Log log =
>>>> LogFactory.getLog(TurnoverReferersTask.class);
>>>> + // a unique id for this specific task instance
>>>> + // this is meant to be unique for each client in a clustered
>>>> environment
>>>> + private String clientId = null;
>>>> + // a String description of when to start this task
>>>> private String startTimeDesc = "startOfDay";
>>>> @@ -48,6 +52,10 @@
>>>> return "TurnoverReferersTask";
>>>> }
>>>> + public String getClientId() {
>>>> + return clientId;
>>>> + }
>>>> + public Date getStartTime(Date currentTime) {
>>>> return getAdjustedTime(currentTime, startTimeDesc);
>>>> }
>>>> @@ -65,6 +73,12 @@
>>>> // get relevant props
>>>> Properties props = this.getTaskProperties();
>>>> + + // extract clientId
>>>> + String client = props.getProperty("clientId");
>>>> + if(client != null) {
>>>> + this.clientId = client;
>>>> + }
>>>> // extract start time
>>>> String startTimeStr = props.getProperty("startTime");
>>>>
>>>> Modified:
>>>> incubator/roller/trunk/src/org/apache/roller/planet/tasks/RefreshEntriesTask.java
>>>>
>>>> URL:
>>>> http://svn.apache.org/viewvc/incubator/roller/trunk/src/org/apache/roller/planet/tasks/RefreshEntriesTask.java?view=diff&rev=495640&r1=495639&r2=495640
>>>>
>>>> ==============================================================================
>>>>
>>>> ---
>>>> incubator/roller/trunk/src/org/apache/roller/planet/tasks/RefreshEntriesTask.java
>>>> (original)
>>>> +++
>>>> incubator/roller/trunk/src/org/apache/roller/planet/tasks/RefreshEntriesTask.java
>>>> Fri Jan 12 08:58:10 2007
>>>> @@ -35,6 +35,10 @@
>>>> private static Log log =
>>>> LogFactory.getLog(RefreshEntriesTask.class);
>>>> + // a unique id for this specific task instance
>>>> + // this is meant to be unique for each client in a clustered
>>>> environment
>>>> + private String clientId = null;
>>>> + // a String description of when to start this task
>>>> private String startTimeDesc = "startOfHour";
>>>> @@ -49,6 +53,10 @@
>>>> return "RefreshEntriesTask";
>>>> }
>>>> + public String getClientId() {
>>>> + return clientId;
>>>> + }
>>>> + public Date getStartTime(Date currentTime) {
>>>> return getAdjustedTime(currentTime, startTimeDesc);
>>>> }
>>>> @@ -66,6 +74,12 @@
>>>> // get relevant props
>>>> Properties props = this.getTaskProperties();
>>>> + + // extract clientId
>>>> + String client = props.getProperty("clientId");
>>>> + if(client != null) {
>>>> + this.clientId = client;
>>>> + }
>>>> // extract start time
>>>> String startTimeStr = props.getProperty("startTime");
>>>>
>>>> Modified:
>>>> incubator/roller/trunk/src/org/apache/roller/planet/tasks/SyncWebsitesTask.java
>>>>
>>>> URL:
>>>> http://svn.apache.org/viewvc/incubator/roller/trunk/src/org/apache/roller/planet/tasks/SyncWebsitesTask.java?view=diff&rev=495640&r1=495639&r2=495640
>>>>
>>>> ==============================================================================
>>>>
>>>> ---
>>>> incubator/roller/trunk/src/org/apache/roller/planet/tasks/SyncWebsitesTask.java
>>>> (original)
>>>> +++
>>>> incubator/roller/trunk/src/org/apache/roller/planet/tasks/SyncWebsitesTask.java
>>>> Fri Jan 12 08:58:10 2007
>>>> @@ -47,6 +47,10 @@
>>>> private static Log log =
>>>> LogFactory.getLog(SyncWebsitesTask.class);
>>>> + // a unique id for this specific task instance
>>>> + // this is meant to be unique for each client in a clustered
>>>> environment
>>>> + private String clientId = "unspecifiedClientId";
>>>> + // a String description of when to start this task
>>>> private String startTimeDesc = "startOfDay";
>>>> @@ -61,6 +65,10 @@
>>>> return "SyncWebsitesTask";
>>>> }
>>>> + public String getClientId() {
>>>> + return clientId;
>>>> + }
>>>> + public Date getStartTime(Date currentTime) {
>>>> return getAdjustedTime(currentTime, startTimeDesc);
>>>> }
>>>> @@ -78,6 +86,12 @@
>>>> // get relevant props
>>>> Properties props = this.getTaskProperties();
>>>> + + // extract clientId
>>>> + String client = props.getProperty("clientId");
>>>> + if(client != null) {
>>>> + this.clientId = client;
>>>> + }
>>>> // extract start time
>>>> String startTimeStr = props.getProperty("startTime");
>>>>
>>>> Modified:
>>>> incubator/roller/trunk/tests/org/apache/roller/business/TaskLockTest.java
>>>>
>>>> URL:
>>>> http://svn.apache.org/viewvc/incubator/roller/trunk/tests/org/apache/roller/business/TaskLockTest.java?view=diff&rev=495640&r1=495639&r2=495640
>>>>
>>>> ==============================================================================
>>>>
>>>> ---
>>>> incubator/roller/trunk/tests/org/apache/roller/business/TaskLockTest.java
>>>> (original)
>>>> +++
>>>> incubator/roller/trunk/tests/org/apache/roller/business/TaskLockTest.java
>>>> Fri Jan 12 08:58:10 2007
>>>> @@ -66,23 +66,15 @@
>>>> RollerTask task = new TestTask();
>>>> // try to acquire a lock
>>>> - boolean lockAcquired = mgr.acquireLock(task);
>>>> - assertTrue(lockAcquired);
>>>> + assertTrue(mgr.acquireLock(task));
>>>> TestUtils.endSession(true);
>>>> // make sure task is locked
>>>> - boolean stillLocked = mgr.isLocked(task);
>>>> - assertTrue(stillLocked);
>>>> + assertFalse(mgr.acquireLock(task));
>>>> TestUtils.endSession(true);
>>>> // try to release a lock
>>>> - boolean lockReleased = mgr.releaseLock(task);
>>>> - assertTrue(lockReleased);
>>>> - TestUtils.endSession(true);
>>>> - - // make sure task is not locked
>>>> - stillLocked = mgr.isLocked(task);
>>>> - assertFalse(stillLocked);
>>>> + assertTrue(mgr.releaseLock(task));
>>>> TestUtils.endSession(true);
>>>> }
>>>> @@ -90,6 +82,7 @@
>>>> class TestTask extends RollerTask {
>>>> public String getName() { return "TestTask"; }
>>>> + public String getClientId() { return "TestTaskClientId"; }
>>>> public Date getStartTime(Date current) { return current; }
>>>> public int getLeaseTime() { return 300; }
>>>> public int getInterval() { return 1800; }
>>>>
>>>> Modified: incubator/roller/trunk/web/WEB-INF/classes/roller.properties
>>>> URL:
>>>> http://svn.apache.org/viewvc/incubator/roller/trunk/web/WEB-INF/classes/roller.properties?view=diff&rev=495640&r1=495639&r2=495640
>>>>
>>>> ==============================================================================
>>>>
>>>> --- incubator/roller/trunk/web/WEB-INF/classes/roller.properties
>>>> (original)
>>>> +++ incubator/roller/trunk/web/WEB-INF/classes/roller.properties Fri
>>>> Jan 12 08:58:10 2007
>>>> @@ -269,6 +269,9 @@
>>>> # Tasks which are enabled. Only tasks listed here will be run.
>>>> tasks.enabled=ResetHitCountsTask,TurnoverReferersTask,PingQueueTask
>>>> +# client identifier. should be unique for each instance in a
>>>> cluster.
>>>> +tasks.clientId=defaultClientId
>>>> +
>>>> # Reset hit counts
>>>>
>>>> tasks.ResetHitCountsTask.class=org.apache.roller.business.runnable.ResetHitCountsTask
>>>>
>>>> tasks.ResetHitCountsTask.startTime=startOfDay
>>>>
>>>>
>>>>
>>
>
Re: svn commit: r495640 - in /incubator/roller/trunk: metadata/database/ src/org/apache/roller/business/hibernate/ src/org/apache/roller/business/pings/ src/org/apache/roller/business/runnable/ src/org/apache/roller/planet/tasks/ tests/org/apache/roller/bu
Posted by Anil Gangolli <an...@busybuddha.org>.
I'm still reviewing the detailed semantics, but I noticed that there are
several messages logged at "warn" level; won't some of these occur
regularly on the hosts that just happen not to have the lease?
--a.
----- Original Message -----
From: "Allen Gilliland" <al...@sun.com>
To: <ro...@incubator.apache.org>
Sent: Friday, January 12, 2007 4:02 PM
Subject: Re: svn commit: r495640 - in /incubator/roller/trunk:
metadata/database/ src/org/apache/roller/business/hibernate/
src/org/apache/roller/business/pings/
src/org/apache/roller/business/runnable/ src/org/apache/roller/planet/tasks/
tests/org/apache/roller/bu...
> actually, there isn't a current need for that property to be accessed via
> the TaskLockData pojo, but there's no harm in having it so I just added it
> now.
>
> -- Allen
>
>
> Mitesh Meswani wrote:
>> Hi Allen,
>>
>> I think you have missed to checking corresponding changes to
>> TaskLockData.java. The class needs to have property "client".
>>
>> Regards,
>> Mitesh
>>
>> agilliland@apache.org wrote:
>>> Author: agilliland
>>> Date: Fri Jan 12 08:58:10 2007
>>> New Revision: 495640
>>>
>>> URL: http://svn.apache.org/viewvc?view=rev&rev=495640
>>> Log:
>>> Fixes for race conditions in lock acquisition via
>>> HibernateThreadManagerImpl from Jira issue ROL-1306. A number of fixes
>>> are included in this commit ...
>>>
>>> 1. Added a database uniqueness constraint on the roller_tasklock.name
>>> column to
>>> prevent any possibility of there being duplicate insertions for a given
>>> task lease.
>>>
>>> 2. Updated the acquireLock() method to use a more robust acquisition
>>> method based on a single sql update statement as suggested by Elias.
>>> This should adequately eliminate race conditions when 2 or more members
>>> of a cluster attempt to acquire a lock at the same time.
>>>
>>> 3. Updated the acquireLock() method to perform all lease activities
>>> based on database time, which should negate any potential clock skew
>>> issues between cluster
>>> members.
>>>
>>> 4. Introduced a new clientId attribute for a task which makes it
>>> possible for each member of a cluster to be uniquely identified with
>>> regards to ownership of a
>>> lease.
>>>
>>>
>>>
>>> Modified:
>>> incubator/roller/trunk/metadata/database/310-to-320-migration.vm
>>> incubator/roller/trunk/metadata/database/createdb.vm
>>>
>>> incubator/roller/trunk/src/org/apache/roller/business/hibernate/HibernateThreadManagerImpl.java
>>>
>>> incubator/roller/trunk/src/org/apache/roller/business/pings/PingQueueTask.java
>>>
>>> incubator/roller/trunk/src/org/apache/roller/business/runnable/ResetHitCountsTask.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/TurnoverReferersTask.java
>>>
>>> incubator/roller/trunk/src/org/apache/roller/planet/tasks/RefreshEntriesTask.java
>>>
>>> incubator/roller/trunk/src/org/apache/roller/planet/tasks/SyncWebsitesTask.java
>>>
>>> incubator/roller/trunk/tests/org/apache/roller/business/TaskLockTest.java
>>> incubator/roller/trunk/web/WEB-INF/classes/roller.properties
>>>
>>> Modified:
>>> incubator/roller/trunk/metadata/database/310-to-320-migration.vm
>>> URL:
>>> http://svn.apache.org/viewvc/incubator/roller/trunk/metadata/database/310-to-320-migration.vm?view=diff&rev=495640&r1=495639&r2=495640
>>> ==============================================================================
>>> --- incubator/roller/trunk/metadata/database/310-to-320-migration.vm
>>> (original)
>>> +++ incubator/roller/trunk/metadata/database/310-to-320-migration.vm Fri
>>> Jan 12 08:58:10 2007
>>> @@ -33,6 +33,12 @@
>>> -- add new status option 'SCHEDULED' for future published entries
>>> update weblogentry set status = 'SCHEDULED' where pubtime > now();
>>> +-- add new client column to roller_tasklock table
>>> +#addColumnNull("roller_tasklock" "client" "varchar(255)")
>>> +
>>> +-- add a uniqueness constraint on roller_tasklock.name
>>> +alter table roller_tasklock add constraint rtl_name_uq unique (
>>> name$!INDEXSIZE );
>>> +
>>> -- some various indexes to improve performance
>>> create index rhc_dailyhits_idx on roller_hitcounts( dailyhits );
>>> create index we_combo1_idx on weblogentry(status, pubtime, websiteid);
>>>
>>> Modified: incubator/roller/trunk/metadata/database/createdb.vm
>>> URL:
>>> http://svn.apache.org/viewvc/incubator/roller/trunk/metadata/database/createdb.vm?view=diff&rev=495640&r1=495639&r2=495640
>>> ==============================================================================
>>> --- incubator/roller/trunk/metadata/database/createdb.vm (original)
>>> +++ incubator/roller/trunk/metadata/database/createdb.vm Fri Jan 12
>>> 08:58:10 2007
>>> @@ -398,8 +398,10 @@
>>> islocked $BOOLEAN_SQL_TYPE_FALSE,
>>> timeacquired $TIMESTAMP_SQL_TYPE_NULL,
>>> timeleased integer,
>>> - lastrun $TIMESTAMP_SQL_TYPE_NULL
>>> + lastrun $TIMESTAMP_SQL_TYPE_NULL,
>>> + client varchar(255)
>>> );
>>> +alter table roller_tasklock add constraint rtl_name_uq unique (
>>> name$!INDEXSIZE );
>>> create index rtl_taskname_idx on roller_tasklock( name );
>>> create table roller_hitcounts (
>>>
>>> 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=495640&r1=495639&r2=495640
>>> ==============================================================================
>>> ---
>>> incubator/roller/trunk/src/org/apache/roller/business/hibernate/HibernateThreadManagerImpl.java
>>> (original)
>>> +++
>>> incubator/roller/trunk/src/org/apache/roller/business/hibernate/HibernateThreadManagerImpl.java
>>> Fri Jan 12 08:58:10 2007
>>> @@ -18,7 +18,6 @@
>>> package org.apache.roller.business.hibernate;
>>> -import java.util.Calendar;
>>> import java.util.Date;
>>> import org.apache.commons.logging.Log;
>>> import org.apache.commons.logging.LogFactory;
>>> @@ -29,6 +28,7 @@
>>> import org.apache.roller.pojos.TaskLockData;
>>> import org.hibernate.Criteria;
>>> import org.hibernate.HibernateException;
>>> +import org.hibernate.Query;
>>> import org.hibernate.Session;
>>> import org.hibernate.criterion.Expression;
>>> @@ -57,53 +57,60 @@
>>> /**
>>> * 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) {
>>> - boolean lockAcquired = false;
>>> - + // query for existing lease record first
>>> TaskLockData taskLock = null;
>>> try {
>>> taskLock = this.getTaskLockByName(task.getName());
>>> - // null here just means hasn't been
>>> initialized yet
>>> if(taskLock == null) {
>>> + // insert an empty record, then we will actually
>>> acquire the
>>> + // lease below using an update statement taskLock = new
>>> TaskLockData();
>>> taskLock.setName(task.getName());
>>> - taskLock.setLocked(false);
>>> + taskLock.setTimeAquired(new Date(0));
>>> + taskLock.setTimeLeased(0);
>>> + + // save it and flush
>>> + this.saveTaskLock(taskLock);
>>> + RollerFactory.getRoller().flush();
>>> }
>>> + } catch (RollerException ex) {
>>> - log.warn("Error getting TaskLockData", ex);
>>> + log.warn("Error getting or inserting TaskLockData", ex);
>>> return false;
>>> }
>>> - 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
>>> - taskLock.setLocked(true);
>>> - taskLock.setTimeAquired(now);
>>> - taskLock.setTimeLeased(task.getLeaseTime());
>>> - taskLock.setLastRun(now);
>>> + // try to acquire lease
>>> + try {
>>> + Session session =
>>> ((HibernatePersistenceStrategy)this.strategy).getSession();
>>> + String queryHQL = "update TaskLockData "+
>>> + "set client=:client,
>>> timeacquired=current_timestamp(), timeleased=:timeleased "+
>>> + "where name=:name and timeacquired=:timeacquired "+
>>> + "and :leaseends < current_timestamp()";
>>> + Query query = session.createQuery(queryHQL);
>>> + query.setString("client", task.getClientId());
>>> + query.setString("timeleased", ""+task.getLeaseTime());
>>> + query.setString("name", task.getName());
>>> + query.setTimestamp("timeacquired",
>>> taskLock.getTimeAquired());
>>> + query.setTimestamp("leaseends", new
>>> Date(taskLock.getTimeAquired().getTime()+(60000*taskLock.getTimeLeased())));
>>> + int result = query.executeUpdate();
>>> - try {
>>> - // save it *and* flush
>>> - this.saveTaskLock(taskLock);
>>> - RollerFactory.getRoller().flush();
>>> - lockAcquired = true;
>>> - } catch (RollerException ex) {
>>> - log.warn("Error saving TaskLockData", ex);
>>> - lockAcquired = false;
>>> + // this may not be needed
>>> + RollerFactory.getRoller().flush();
>>> + + if(result == 1) {
>>> + return true;
>>> }
>>> + + } catch (Exception e) {
>>> + log.warn("Error obtaining lease, assuming race condition.",
>>> e);
>>> + return false;
>>> }
>>> - return lockAcquired;
>>> + return false;
>>> }
>>> @@ -112,61 +119,43 @@
>>> */
>>> public boolean releaseLock(RollerTask task) {
>>> - boolean lockReleased = false;
>>> - + // query for existing lease record first
>>> TaskLockData taskLock = null;
>>> try {
>>> taskLock = this.getTaskLockByName(task.getName());
>>> + + if(taskLock == null) {
>>> + return false;
>>> + }
>>> + } catch (RollerException ex) {
>>> log.warn("Error getting TaskLockData", ex);
>>> return false;
>>> }
>>> - if(taskLock != null && taskLock.isLocked()) {
>>> - // set appropriate values for TaskLock and save it
>>> - Date now = new Date();
>>> - taskLock.setLocked(false);
>>> - - try {
>>> - // save it *and* flush
>>> - this.saveTaskLock(taskLock);
>>> - RollerFactory.getRoller().flush();
>>> - lockReleased = true;
>>> - } catch (RollerException ex) {
>>> - log.warn("Error saving TaskLockData", ex);
>>> - lockReleased = false;
>>> - }
>>> - } else if(taskLock != null && !taskLock.isLocked()) {
>>> - // if lock is already released then don't fret about it
>>> - lockReleased = true;
>>> - }
>>> - - return lockReleased;
>>> - }
>>> - - - /**
>>> - * Is a task currently locked?
>>> - */
>>> - public boolean isLocked(RollerTask task) {
>>> - - // default is "true"!
>>> - boolean locked = true;
>>> - + // try to release lease, just set lease time to 0
>>> try {
>>> - TaskLockData taskLock =
>>> this.getTaskLockByName(task.getName());
>>> - if(taskLock != null) {
>>> - locked = taskLock.isLocked();
>>> - } else {
>>> - // if taskLock is null, but we didn't get an exception
>>> then
>>> - // that means this lock hasn't been initialized yet
>>> - locked = false;
>>> + Session session =
>>> ((HibernatePersistenceStrategy)this.strategy).getSession();
>>> + String queryHQL = "update TaskLockData set timeLeased=0 "+
>>> + "where name=:name and client=:client";
>>> + Query query = session.createQuery(queryHQL);
>>> + query.setString("name", task.getName());
>>> + query.setString("client", task.getClientId());
>>> + int result = query.executeUpdate();
>>> + + // this may not be needed
>>> + RollerFactory.getRoller().flush();
>>> + + if(result == 1) {
>>> + return true;
>>> }
>>> - } catch (RollerException ex) {
>>> - log.warn("Error getting TaskLockData", ex);
>>> + + } catch (Exception e) {
>>> + log.warn("Error releasing lease.", e);
>>> + return false;
>>> }
>>> - return locked;
>>> + return false;
>>> }
>>> Modified:
>>> incubator/roller/trunk/src/org/apache/roller/business/pings/PingQueueTask.java
>>> URL:
>>> http://svn.apache.org/viewvc/incubator/roller/trunk/src/org/apache/roller/business/pings/PingQueueTask.java?view=diff&rev=495640&r1=495639&r2=495640
>>> ==============================================================================
>>> ---
>>> incubator/roller/trunk/src/org/apache/roller/business/pings/PingQueueTask.java
>>> (original)
>>> +++
>>> incubator/roller/trunk/src/org/apache/roller/business/pings/PingQueueTask.java
>>> Fri Jan 12 08:58:10 2007
>>> @@ -41,6 +41,10 @@
>>> private static Log log =
>>> LogFactory.getLog(PingQueueTask.class);
>>> + // a unique id for this specific task instance
>>> + // this is meant to be unique for each client in a clustered
>>> environment
>>> + private String clientId = null;
>>> + // a String description of when to start this task
>>> private String startTimeDesc = "immediate";
>>> @@ -55,6 +59,10 @@
>>> return "PingQueueTask";
>>> }
>>> + public String getClientId() {
>>> + return clientId;
>>> + }
>>> + public Date getStartTime(Date currentTime) {
>>> return getAdjustedTime(currentTime, startTimeDesc);
>>> }
>>> @@ -72,6 +80,12 @@
>>> // get relevant props
>>> Properties props = this.getTaskProperties();
>>> + + // extract clientId
>>> + String client = props.getProperty("clientId");
>>> + if(client != null) {
>>> + this.clientId = client;
>>> + }
>>> // extract start time
>>> String startTimeStr = props.getProperty("startTime");
>>>
>>> Modified:
>>> incubator/roller/trunk/src/org/apache/roller/business/runnable/ResetHitCountsTask.java
>>> URL:
>>> http://svn.apache.org/viewvc/incubator/roller/trunk/src/org/apache/roller/business/runnable/ResetHitCountsTask.java?view=diff&rev=495640&r1=495639&r2=495640
>>> ==============================================================================
>>> ---
>>> incubator/roller/trunk/src/org/apache/roller/business/runnable/ResetHitCountsTask.java
>>> (original)
>>> +++
>>> incubator/roller/trunk/src/org/apache/roller/business/runnable/ResetHitCountsTask.java
>>> Fri Jan 12 08:58:10 2007
>>> @@ -34,6 +34,10 @@
>>> private static Log log =
>>> LogFactory.getLog(ResetHitCountsTask.class);
>>> + // a unique id for this specific task instance
>>> + // this is meant to be unique for each client in a clustered
>>> environment
>>> + private String clientId = null;
>>> + // a String description of when to start this task
>>> private String startTimeDesc = "startOfDay";
>>> @@ -48,6 +52,10 @@
>>> return "ResetHitCountsTask";
>>> }
>>> + public String getClientId() {
>>> + return clientId;
>>> + }
>>> + public Date getStartTime(Date currentTime) {
>>> return getAdjustedTime(currentTime, startTimeDesc);
>>> }
>>> @@ -65,6 +73,12 @@
>>> // get relevant props
>>> Properties props = this.getTaskProperties();
>>> + + // extract clientId
>>> + String client = props.getProperty("clientId");
>>> + if(client != null) {
>>> + this.clientId = client;
>>> + }
>>> // extract start time
>>> String startTimeStr = props.getProperty("startTime");
>>>
>>> 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=495640&r1=495639&r2=495640
>>> ==============================================================================
>>> ---
>>> incubator/roller/trunk/src/org/apache/roller/business/runnable/RollerTask.java
>>> (original)
>>> +++
>>> incubator/roller/trunk/src/org/apache/roller/business/runnable/RollerTask.java
>>> Fri Jan 12 08:58:10 2007
>>> @@ -60,6 +60,16 @@
>>> /**
>>> + * Get the unique id representing a specific instance of a task.
>>> This is
>>> + * important for tasks being run in a clustered environment so that
>>> a + * lease can be associated with a single cluster member.
>>> + *
>>> + * @return The unique client identifier for this task instance.
>>> + */
>>> + public abstract String getClientId();
>>> + + + /**
>>> * When should this task be started? The task is given the current
>>> time
>>> * so that it may determine a start time relative to the current
>>> time, * such as the end of the day or hour.
>>> @@ -112,36 +122,21 @@
>>> */
>>> public final void run() {
>>> - ThreadManager mgr = null;
>>> - try {
>>> - mgr = RollerFactory.getRoller().getThreadManager();
>>> - } catch (Exception ex) {
>>> - log.fatal("Unable to obtain ThreadManager", ex);
>>> - return;
>>> - }
>>> + ThreadManager mgr =
>>> RollerFactory.getRoller().getThreadManager();
>>> boolean lockAcquired = false;
>>> try {
>>> + log.debug("Attempting to acquire lock");
>>> - if(!mgr.isLocked(this)) {
>>> - - 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("Lock NOT acquired, cannot continue");
>>> - - // when we don't have a lock
>>> we can't continue, so bail
>>> - return;
>>> - }
>>> - + 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("Task already locked, nothing to do");
>>> + log.debug("Lock NOT acquired, cannot continue");
>>> + return;
>>> }
>>> } catch (Exception ex) {
>>> @@ -191,6 +186,9 @@
>>> RollerConfig.getProperty(key));
>>> }
>>> }
>>> + + // special addition for clientId property that applies
>>> to all tasks
>>> + taskProps.setProperty("clientId",
>>> RollerConfig.getProperty("tasks.clientId"));
>>> return taskProps;
>>> }
>>>
>>> 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=495640&r1=495639&r2=495640
>>> ==============================================================================
>>> ---
>>> incubator/roller/trunk/src/org/apache/roller/business/runnable/ThreadManager.java
>>> (original)
>>> +++
>>> incubator/roller/trunk/src/org/apache/roller/business/runnable/ThreadManager.java
>>> Fri Jan 12 08:58:10 2007
>>> @@ -75,15 +75,6 @@
>>> /**
>>> - * Is a task currently locked?
>>> - * - * @param task The RollerTask to check the lock state for.
>>> - * @return boolean True if task is locked, False otherwise.
>>> - */
>>> - public boolean isLocked(RollerTask task);
>>> - - - /**
>>> * Shutdown.
>>> */
>>> public void shutdown();
>>>
>>> Modified:
>>> incubator/roller/trunk/src/org/apache/roller/business/runnable/TurnoverReferersTask.java
>>> URL:
>>> http://svn.apache.org/viewvc/incubator/roller/trunk/src/org/apache/roller/business/runnable/TurnoverReferersTask.java?view=diff&rev=495640&r1=495639&r2=495640
>>> ==============================================================================
>>> ---
>>> incubator/roller/trunk/src/org/apache/roller/business/runnable/TurnoverReferersTask.java
>>> (original)
>>> +++
>>> incubator/roller/trunk/src/org/apache/roller/business/runnable/TurnoverReferersTask.java
>>> Fri Jan 12 08:58:10 2007
>>> @@ -34,6 +34,10 @@
>>> private static Log log =
>>> LogFactory.getLog(TurnoverReferersTask.class);
>>> + // a unique id for this specific task instance
>>> + // this is meant to be unique for each client in a clustered
>>> environment
>>> + private String clientId = null;
>>> + // a String description of when to start this task
>>> private String startTimeDesc = "startOfDay";
>>> @@ -48,6 +52,10 @@
>>> return "TurnoverReferersTask";
>>> }
>>> + public String getClientId() {
>>> + return clientId;
>>> + }
>>> + public Date getStartTime(Date currentTime) {
>>> return getAdjustedTime(currentTime, startTimeDesc);
>>> }
>>> @@ -65,6 +73,12 @@
>>> // get relevant props
>>> Properties props = this.getTaskProperties();
>>> + + // extract clientId
>>> + String client = props.getProperty("clientId");
>>> + if(client != null) {
>>> + this.clientId = client;
>>> + }
>>> // extract start time
>>> String startTimeStr = props.getProperty("startTime");
>>>
>>> Modified:
>>> incubator/roller/trunk/src/org/apache/roller/planet/tasks/RefreshEntriesTask.java
>>> URL:
>>> http://svn.apache.org/viewvc/incubator/roller/trunk/src/org/apache/roller/planet/tasks/RefreshEntriesTask.java?view=diff&rev=495640&r1=495639&r2=495640
>>> ==============================================================================
>>> ---
>>> incubator/roller/trunk/src/org/apache/roller/planet/tasks/RefreshEntriesTask.java
>>> (original)
>>> +++
>>> incubator/roller/trunk/src/org/apache/roller/planet/tasks/RefreshEntriesTask.java
>>> Fri Jan 12 08:58:10 2007
>>> @@ -35,6 +35,10 @@
>>> private static Log log =
>>> LogFactory.getLog(RefreshEntriesTask.class);
>>> + // a unique id for this specific task instance
>>> + // this is meant to be unique for each client in a clustered
>>> environment
>>> + private String clientId = null;
>>> + // a String description of when to start this task
>>> private String startTimeDesc = "startOfHour";
>>> @@ -49,6 +53,10 @@
>>> return "RefreshEntriesTask";
>>> }
>>> + public String getClientId() {
>>> + return clientId;
>>> + }
>>> + public Date getStartTime(Date currentTime) {
>>> return getAdjustedTime(currentTime, startTimeDesc);
>>> }
>>> @@ -66,6 +74,12 @@
>>> // get relevant props
>>> Properties props = this.getTaskProperties();
>>> + + // extract clientId
>>> + String client = props.getProperty("clientId");
>>> + if(client != null) {
>>> + this.clientId = client;
>>> + }
>>> // extract start time
>>> String startTimeStr = props.getProperty("startTime");
>>>
>>> Modified:
>>> incubator/roller/trunk/src/org/apache/roller/planet/tasks/SyncWebsitesTask.java
>>> URL:
>>> http://svn.apache.org/viewvc/incubator/roller/trunk/src/org/apache/roller/planet/tasks/SyncWebsitesTask.java?view=diff&rev=495640&r1=495639&r2=495640
>>> ==============================================================================
>>> ---
>>> incubator/roller/trunk/src/org/apache/roller/planet/tasks/SyncWebsitesTask.java
>>> (original)
>>> +++
>>> incubator/roller/trunk/src/org/apache/roller/planet/tasks/SyncWebsitesTask.java
>>> Fri Jan 12 08:58:10 2007
>>> @@ -47,6 +47,10 @@
>>> private static Log log =
>>> LogFactory.getLog(SyncWebsitesTask.class);
>>> + // a unique id for this specific task instance
>>> + // this is meant to be unique for each client in a clustered
>>> environment
>>> + private String clientId = "unspecifiedClientId";
>>> + // a String description of when to start this task
>>> private String startTimeDesc = "startOfDay";
>>> @@ -61,6 +65,10 @@
>>> return "SyncWebsitesTask";
>>> }
>>> + public String getClientId() {
>>> + return clientId;
>>> + }
>>> + public Date getStartTime(Date currentTime) {
>>> return getAdjustedTime(currentTime, startTimeDesc);
>>> }
>>> @@ -78,6 +86,12 @@
>>> // get relevant props
>>> Properties props = this.getTaskProperties();
>>> + + // extract clientId
>>> + String client = props.getProperty("clientId");
>>> + if(client != null) {
>>> + this.clientId = client;
>>> + }
>>> // extract start time
>>> String startTimeStr = props.getProperty("startTime");
>>>
>>> Modified:
>>> incubator/roller/trunk/tests/org/apache/roller/business/TaskLockTest.java
>>> URL:
>>> http://svn.apache.org/viewvc/incubator/roller/trunk/tests/org/apache/roller/business/TaskLockTest.java?view=diff&rev=495640&r1=495639&r2=495640
>>> ==============================================================================
>>> ---
>>> incubator/roller/trunk/tests/org/apache/roller/business/TaskLockTest.java
>>> (original)
>>> +++
>>> incubator/roller/trunk/tests/org/apache/roller/business/TaskLockTest.java
>>> Fri Jan 12 08:58:10 2007
>>> @@ -66,23 +66,15 @@
>>> RollerTask task = new TestTask();
>>> // try to acquire a lock
>>> - boolean lockAcquired = mgr.acquireLock(task);
>>> - assertTrue(lockAcquired);
>>> + assertTrue(mgr.acquireLock(task));
>>> TestUtils.endSession(true);
>>> // make sure task is locked
>>> - boolean stillLocked = mgr.isLocked(task);
>>> - assertTrue(stillLocked);
>>> + assertFalse(mgr.acquireLock(task));
>>> TestUtils.endSession(true);
>>> // try to release a lock
>>> - boolean lockReleased = mgr.releaseLock(task);
>>> - assertTrue(lockReleased);
>>> - TestUtils.endSession(true);
>>> - - // make sure task is not locked
>>> - stillLocked = mgr.isLocked(task);
>>> - assertFalse(stillLocked);
>>> + assertTrue(mgr.releaseLock(task));
>>> TestUtils.endSession(true);
>>> }
>>> @@ -90,6 +82,7 @@
>>> class TestTask extends RollerTask {
>>> public String getName() { return "TestTask"; }
>>> + public String getClientId() { return "TestTaskClientId"; }
>>> public Date getStartTime(Date current) { return current; }
>>> public int getLeaseTime() { return 300; }
>>> public int getInterval() { return 1800; }
>>>
>>> Modified: incubator/roller/trunk/web/WEB-INF/classes/roller.properties
>>> URL:
>>> http://svn.apache.org/viewvc/incubator/roller/trunk/web/WEB-INF/classes/roller.properties?view=diff&rev=495640&r1=495639&r2=495640
>>> ==============================================================================
>>> --- incubator/roller/trunk/web/WEB-INF/classes/roller.properties
>>> (original)
>>> +++ incubator/roller/trunk/web/WEB-INF/classes/roller.properties Fri Jan
>>> 12 08:58:10 2007
>>> @@ -269,6 +269,9 @@
>>> # Tasks which are enabled. Only tasks listed here will be run.
>>> tasks.enabled=ResetHitCountsTask,TurnoverReferersTask,PingQueueTask
>>> +# client identifier. should be unique for each instance in a cluster.
>>> +tasks.clientId=defaultClientId
>>> +
>>> # Reset hit counts
>>>
>>> tasks.ResetHitCountsTask.class=org.apache.roller.business.runnable.ResetHitCountsTask
>>> tasks.ResetHitCountsTask.startTime=startOfDay
>>>
>>>
>>>
>
Re: svn commit: r495640 - in /incubator/roller/trunk:
metadata/database/ src/org/apache/roller/business/hibernate/
src/org/apache/roller/business/pings/ src/org/apache/roller/business/runnable/
src/org/apache/roller/planet/tasks/ tests/org/apache/roller/bu...
Posted by Allen Gilliland <al...@sun.com>.
actually, there isn't a current need for that property to be accessed
via the TaskLockData pojo, but there's no harm in having it so I just
added it now.
-- Allen
Mitesh Meswani wrote:
> Hi Allen,
>
> I think you have missed to checking corresponding changes to
> TaskLockData.java. The class needs to have property "client".
>
> Regards,
> Mitesh
>
> agilliland@apache.org wrote:
>> Author: agilliland
>> Date: Fri Jan 12 08:58:10 2007
>> New Revision: 495640
>>
>> URL: http://svn.apache.org/viewvc?view=rev&rev=495640
>> Log:
>> Fixes for race conditions in lock acquisition via
>> HibernateThreadManagerImpl from Jira issue ROL-1306. A number of
>> fixes are included in this commit ...
>>
>> 1. Added a database uniqueness constraint on the roller_tasklock.name
>> column to
>> prevent any possibility of there being duplicate insertions for a
>> given task lease.
>>
>> 2. Updated the acquireLock() method to use a more robust acquisition
>> method based on a single sql update statement as suggested by Elias.
>> This should adequately eliminate race conditions when 2 or more
>> members of a cluster attempt to acquire a lock at the same time.
>>
>> 3. Updated the acquireLock() method to perform all lease activities
>> based on database time, which should negate any potential clock skew
>> issues between cluster
>> members.
>>
>> 4. Introduced a new clientId attribute for a task which makes it
>> possible for each member of a cluster to be uniquely identified with
>> regards to ownership of a
>> lease.
>>
>>
>>
>> Modified:
>> incubator/roller/trunk/metadata/database/310-to-320-migration.vm
>> incubator/roller/trunk/metadata/database/createdb.vm
>>
>> incubator/roller/trunk/src/org/apache/roller/business/hibernate/HibernateThreadManagerImpl.java
>>
>>
>> incubator/roller/trunk/src/org/apache/roller/business/pings/PingQueueTask.java
>>
>>
>> incubator/roller/trunk/src/org/apache/roller/business/runnable/ResetHitCountsTask.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/TurnoverReferersTask.java
>>
>>
>> incubator/roller/trunk/src/org/apache/roller/planet/tasks/RefreshEntriesTask.java
>>
>>
>> incubator/roller/trunk/src/org/apache/roller/planet/tasks/SyncWebsitesTask.java
>>
>>
>> incubator/roller/trunk/tests/org/apache/roller/business/TaskLockTest.java
>> incubator/roller/trunk/web/WEB-INF/classes/roller.properties
>>
>> Modified:
>> incubator/roller/trunk/metadata/database/310-to-320-migration.vm
>> URL:
>> http://svn.apache.org/viewvc/incubator/roller/trunk/metadata/database/310-to-320-migration.vm?view=diff&rev=495640&r1=495639&r2=495640
>>
>> ==============================================================================
>>
>> --- incubator/roller/trunk/metadata/database/310-to-320-migration.vm
>> (original)
>> +++ incubator/roller/trunk/metadata/database/310-to-320-migration.vm
>> Fri Jan 12 08:58:10 2007
>> @@ -33,6 +33,12 @@
>> -- add new status option 'SCHEDULED' for future published entries
>> update weblogentry set status = 'SCHEDULED' where pubtime > now();
>>
>> +-- add new client column to roller_tasklock table
>> +#addColumnNull("roller_tasklock" "client" "varchar(255)")
>> +
>> +-- add a uniqueness constraint on roller_tasklock.name
>> +alter table roller_tasklock add constraint rtl_name_uq unique (
>> name$!INDEXSIZE );
>> +
>> -- some various indexes to improve performance
>> create index rhc_dailyhits_idx on roller_hitcounts( dailyhits );
>> create index we_combo1_idx on weblogentry(status, pubtime, websiteid);
>>
>> Modified: incubator/roller/trunk/metadata/database/createdb.vm
>> URL:
>> http://svn.apache.org/viewvc/incubator/roller/trunk/metadata/database/createdb.vm?view=diff&rev=495640&r1=495639&r2=495640
>>
>> ==============================================================================
>>
>> --- incubator/roller/trunk/metadata/database/createdb.vm (original)
>> +++ incubator/roller/trunk/metadata/database/createdb.vm Fri Jan 12
>> 08:58:10 2007
>> @@ -398,8 +398,10 @@
>> islocked $BOOLEAN_SQL_TYPE_FALSE,
>> timeacquired $TIMESTAMP_SQL_TYPE_NULL,
>> timeleased integer,
>> - lastrun $TIMESTAMP_SQL_TYPE_NULL
>> + lastrun $TIMESTAMP_SQL_TYPE_NULL,
>> + client varchar(255)
>> );
>> +alter table roller_tasklock add constraint rtl_name_uq unique (
>> name$!INDEXSIZE );
>> create index rtl_taskname_idx on roller_tasklock( name );
>>
>> create table roller_hitcounts (
>>
>> 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=495640&r1=495639&r2=495640
>>
>> ==============================================================================
>>
>> ---
>> incubator/roller/trunk/src/org/apache/roller/business/hibernate/HibernateThreadManagerImpl.java
>> (original)
>> +++
>> incubator/roller/trunk/src/org/apache/roller/business/hibernate/HibernateThreadManagerImpl.java
>> Fri Jan 12 08:58:10 2007
>> @@ -18,7 +18,6 @@
>>
>> package org.apache.roller.business.hibernate;
>>
>> -import java.util.Calendar;
>> import java.util.Date;
>> import org.apache.commons.logging.Log;
>> import org.apache.commons.logging.LogFactory;
>> @@ -29,6 +28,7 @@
>> import org.apache.roller.pojos.TaskLockData;
>> import org.hibernate.Criteria;
>> import org.hibernate.HibernateException;
>> +import org.hibernate.Query;
>> import org.hibernate.Session;
>> import org.hibernate.criterion.Expression;
>>
>> @@ -57,53 +57,60 @@
>> /**
>> * 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) {
>> - boolean lockAcquired = false;
>> - + // query for existing lease record first
>> TaskLockData taskLock = null;
>> try {
>> taskLock = this.getTaskLockByName(task.getName());
>> - // null here just means hasn't been
>> initialized yet
>> if(taskLock == null) {
>> + // insert an empty record, then we will actually
>> acquire the
>> + // lease below using an update statement
>> taskLock = new TaskLockData();
>> taskLock.setName(task.getName());
>> - taskLock.setLocked(false);
>> + taskLock.setTimeAquired(new Date(0));
>> + taskLock.setTimeLeased(0);
>> + + // save it and flush
>> + this.saveTaskLock(taskLock);
>> + RollerFactory.getRoller().flush();
>> }
>> + } catch (RollerException ex) {
>> - log.warn("Error getting TaskLockData", ex);
>> + log.warn("Error getting or inserting TaskLockData", ex);
>> return false;
>> }
>> - 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
>> - taskLock.setLocked(true);
>> - taskLock.setTimeAquired(now);
>> - taskLock.setTimeLeased(task.getLeaseTime());
>> - taskLock.setLastRun(now);
>> + // try to acquire lease
>> + try {
>> + Session session =
>> ((HibernatePersistenceStrategy)this.strategy).getSession();
>> + String queryHQL = "update TaskLockData "+
>> + "set client=:client,
>> timeacquired=current_timestamp(), timeleased=:timeleased "+
>> + "where name=:name and timeacquired=:timeacquired "+
>> + "and :leaseends < current_timestamp()";
>> + Query query = session.createQuery(queryHQL);
>> + query.setString("client", task.getClientId());
>> + query.setString("timeleased", ""+task.getLeaseTime());
>> + query.setString("name", task.getName());
>> + query.setTimestamp("timeacquired",
>> taskLock.getTimeAquired());
>> + query.setTimestamp("leaseends", new
>> Date(taskLock.getTimeAquired().getTime()+(60000*taskLock.getTimeLeased())));
>>
>> + int result = query.executeUpdate();
>> - try {
>> - // save it *and* flush
>> - this.saveTaskLock(taskLock);
>> - RollerFactory.getRoller().flush();
>> - lockAcquired = true;
>> - } catch (RollerException ex) {
>> - log.warn("Error saving TaskLockData", ex);
>> - lockAcquired = false;
>> + // this may not be needed
>> + RollerFactory.getRoller().flush();
>> + + if(result == 1) {
>> + return true;
>> }
>> + + } catch (Exception e) {
>> + log.warn("Error obtaining lease, assuming race
>> condition.", e);
>> + return false;
>> }
>> - return lockAcquired;
>> + return false;
>> }
>> @@ -112,61 +119,43 @@
>> */
>> public boolean releaseLock(RollerTask task) {
>> - boolean lockReleased = false;
>> - + // query for existing lease record first
>> TaskLockData taskLock = null;
>> try {
>> taskLock = this.getTaskLockByName(task.getName());
>> + + if(taskLock == null) {
>> + return false;
>> + }
>> + } catch (RollerException ex) {
>> log.warn("Error getting TaskLockData", ex);
>> return false;
>> }
>> - if(taskLock != null && taskLock.isLocked()) {
>> - // set appropriate values for TaskLock and save it
>> - Date now = new Date();
>> - taskLock.setLocked(false);
>> - - try {
>> - // save it *and* flush
>> - this.saveTaskLock(taskLock);
>> - RollerFactory.getRoller().flush();
>> - lockReleased = true;
>> - } catch (RollerException ex) {
>> - log.warn("Error saving TaskLockData", ex);
>> - lockReleased = false;
>> - }
>> - } else if(taskLock != null && !taskLock.isLocked()) {
>> - // if lock is already released then don't fret about it
>> - lockReleased = true;
>> - }
>> - - return lockReleased;
>> - }
>> - - - /**
>> - * Is a task currently locked?
>> - */
>> - public boolean isLocked(RollerTask task) {
>> - - // default is "true"!
>> - boolean locked = true;
>> - + // try to release lease, just set lease time to 0
>> try {
>> - TaskLockData taskLock =
>> this.getTaskLockByName(task.getName());
>> - if(taskLock != null) {
>> - locked = taskLock.isLocked();
>> - } else {
>> - // if taskLock is null, but we didn't get an
>> exception then
>> - // that means this lock hasn't been initialized yet
>> - locked = false;
>> + Session session =
>> ((HibernatePersistenceStrategy)this.strategy).getSession();
>> + String queryHQL = "update TaskLockData set timeLeased=0 "+
>> + "where name=:name and client=:client";
>> + Query query = session.createQuery(queryHQL);
>> + query.setString("name", task.getName());
>> + query.setString("client", task.getClientId());
>> + int result = query.executeUpdate();
>> + + // this may not be needed
>> + RollerFactory.getRoller().flush();
>> + + if(result == 1) {
>> + return true;
>> }
>> - } catch (RollerException ex) {
>> - log.warn("Error getting TaskLockData", ex);
>> + + } catch (Exception e) {
>> + log.warn("Error releasing lease.", e);
>> + return false;
>> }
>> - return locked;
>> + return false;
>> }
>>
>> Modified:
>> incubator/roller/trunk/src/org/apache/roller/business/pings/PingQueueTask.java
>>
>> URL:
>> http://svn.apache.org/viewvc/incubator/roller/trunk/src/org/apache/roller/business/pings/PingQueueTask.java?view=diff&rev=495640&r1=495639&r2=495640
>>
>> ==============================================================================
>>
>> ---
>> incubator/roller/trunk/src/org/apache/roller/business/pings/PingQueueTask.java
>> (original)
>> +++
>> incubator/roller/trunk/src/org/apache/roller/business/pings/PingQueueTask.java
>> Fri Jan 12 08:58:10 2007
>> @@ -41,6 +41,10 @@
>> private static Log log =
>> LogFactory.getLog(PingQueueTask.class);
>> + // a unique id for this specific task instance
>> + // this is meant to be unique for each client in a clustered
>> environment
>> + private String clientId = null;
>> + // a String description of when to start this task
>> private String startTimeDesc = "immediate";
>> @@ -55,6 +59,10 @@
>> return "PingQueueTask";
>> }
>> + public String getClientId() {
>> + return clientId;
>> + }
>> + public Date getStartTime(Date currentTime) {
>> return getAdjustedTime(currentTime, startTimeDesc);
>> }
>> @@ -72,6 +80,12 @@
>> // get relevant props
>> Properties props = this.getTaskProperties();
>> + + // extract clientId
>> + String client = props.getProperty("clientId");
>> + if(client != null) {
>> + this.clientId = client;
>> + }
>> // extract start time
>> String startTimeStr = props.getProperty("startTime");
>>
>> Modified:
>> incubator/roller/trunk/src/org/apache/roller/business/runnable/ResetHitCountsTask.java
>>
>> URL:
>> http://svn.apache.org/viewvc/incubator/roller/trunk/src/org/apache/roller/business/runnable/ResetHitCountsTask.java?view=diff&rev=495640&r1=495639&r2=495640
>>
>> ==============================================================================
>>
>> ---
>> incubator/roller/trunk/src/org/apache/roller/business/runnable/ResetHitCountsTask.java
>> (original)
>> +++
>> incubator/roller/trunk/src/org/apache/roller/business/runnable/ResetHitCountsTask.java
>> Fri Jan 12 08:58:10 2007
>> @@ -34,6 +34,10 @@
>> private static Log log =
>> LogFactory.getLog(ResetHitCountsTask.class);
>> + // a unique id for this specific task instance
>> + // this is meant to be unique for each client in a clustered
>> environment
>> + private String clientId = null;
>> + // a String description of when to start this task
>> private String startTimeDesc = "startOfDay";
>> @@ -48,6 +52,10 @@
>> return "ResetHitCountsTask";
>> }
>> + public String getClientId() {
>> + return clientId;
>> + }
>> + public Date getStartTime(Date currentTime) {
>> return getAdjustedTime(currentTime, startTimeDesc);
>> }
>> @@ -65,6 +73,12 @@
>> // get relevant props
>> Properties props = this.getTaskProperties();
>> + + // extract clientId
>> + String client = props.getProperty("clientId");
>> + if(client != null) {
>> + this.clientId = client;
>> + }
>> // extract start time
>> String startTimeStr = props.getProperty("startTime");
>>
>> 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=495640&r1=495639&r2=495640
>>
>> ==============================================================================
>>
>> ---
>> incubator/roller/trunk/src/org/apache/roller/business/runnable/RollerTask.java
>> (original)
>> +++
>> incubator/roller/trunk/src/org/apache/roller/business/runnable/RollerTask.java
>> Fri Jan 12 08:58:10 2007
>> @@ -60,6 +60,16 @@
>> /**
>> + * Get the unique id representing a specific instance of a task.
>> This is
>> + * important for tasks being run in a clustered environment so
>> that a + * lease can be associated with a single cluster member.
>> + *
>> + * @return The unique client identifier for this task instance.
>> + */
>> + public abstract String getClientId();
>> + + + /**
>> * When should this task be started? The task is given the
>> current time
>> * so that it may determine a start time relative to the current
>> time, * such as the end of the day or hour.
>> @@ -112,36 +122,21 @@
>> */
>> public final void run() {
>> - ThreadManager mgr = null;
>> - try {
>> - mgr = RollerFactory.getRoller().getThreadManager();
>> - } catch (Exception ex) {
>> - log.fatal("Unable to obtain ThreadManager", ex);
>> - return;
>> - }
>> + ThreadManager mgr =
>> RollerFactory.getRoller().getThreadManager();
>> boolean lockAcquired = false;
>> try {
>> + log.debug("Attempting to acquire lock");
>> - if(!mgr.isLocked(this)) {
>> - - 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("Lock NOT acquired, cannot continue");
>> - - // when we don't have a lock
>> we can't continue, so bail
>> - return;
>> - }
>> - + 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("Task already locked, nothing to do");
>> + log.debug("Lock NOT acquired, cannot continue");
>> + return;
>> }
>> } catch (Exception ex) {
>> @@ -191,6 +186,9 @@
>> RollerConfig.getProperty(key));
>> }
>> }
>> + + // special addition for clientId property that
>> applies to all tasks
>> + taskProps.setProperty("clientId",
>> RollerConfig.getProperty("tasks.clientId"));
>> return taskProps;
>> }
>>
>> 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=495640&r1=495639&r2=495640
>>
>> ==============================================================================
>>
>> ---
>> incubator/roller/trunk/src/org/apache/roller/business/runnable/ThreadManager.java
>> (original)
>> +++
>> incubator/roller/trunk/src/org/apache/roller/business/runnable/ThreadManager.java
>> Fri Jan 12 08:58:10 2007
>> @@ -75,15 +75,6 @@
>> /**
>> - * Is a task currently locked?
>> - * - * @param task The RollerTask to check the lock state for.
>> - * @return boolean True if task is locked, False otherwise.
>> - */
>> - public boolean isLocked(RollerTask task);
>> - - - /**
>> * Shutdown.
>> */
>> public void shutdown();
>>
>> Modified:
>> incubator/roller/trunk/src/org/apache/roller/business/runnable/TurnoverReferersTask.java
>>
>> URL:
>> http://svn.apache.org/viewvc/incubator/roller/trunk/src/org/apache/roller/business/runnable/TurnoverReferersTask.java?view=diff&rev=495640&r1=495639&r2=495640
>>
>> ==============================================================================
>>
>> ---
>> incubator/roller/trunk/src/org/apache/roller/business/runnable/TurnoverReferersTask.java
>> (original)
>> +++
>> incubator/roller/trunk/src/org/apache/roller/business/runnable/TurnoverReferersTask.java
>> Fri Jan 12 08:58:10 2007
>> @@ -34,6 +34,10 @@
>> private static Log log =
>> LogFactory.getLog(TurnoverReferersTask.class);
>> + // a unique id for this specific task instance
>> + // this is meant to be unique for each client in a clustered
>> environment
>> + private String clientId = null;
>> + // a String description of when to start this task
>> private String startTimeDesc = "startOfDay";
>> @@ -48,6 +52,10 @@
>> return "TurnoverReferersTask";
>> }
>> + public String getClientId() {
>> + return clientId;
>> + }
>> + public Date getStartTime(Date currentTime) {
>> return getAdjustedTime(currentTime, startTimeDesc);
>> }
>> @@ -65,6 +73,12 @@
>> // get relevant props
>> Properties props = this.getTaskProperties();
>> + + // extract clientId
>> + String client = props.getProperty("clientId");
>> + if(client != null) {
>> + this.clientId = client;
>> + }
>> // extract start time
>> String startTimeStr = props.getProperty("startTime");
>>
>> Modified:
>> incubator/roller/trunk/src/org/apache/roller/planet/tasks/RefreshEntriesTask.java
>>
>> URL:
>> http://svn.apache.org/viewvc/incubator/roller/trunk/src/org/apache/roller/planet/tasks/RefreshEntriesTask.java?view=diff&rev=495640&r1=495639&r2=495640
>>
>> ==============================================================================
>>
>> ---
>> incubator/roller/trunk/src/org/apache/roller/planet/tasks/RefreshEntriesTask.java
>> (original)
>> +++
>> incubator/roller/trunk/src/org/apache/roller/planet/tasks/RefreshEntriesTask.java
>> Fri Jan 12 08:58:10 2007
>> @@ -35,6 +35,10 @@
>> private static Log log =
>> LogFactory.getLog(RefreshEntriesTask.class);
>> + // a unique id for this specific task instance
>> + // this is meant to be unique for each client in a clustered
>> environment
>> + private String clientId = null;
>> + // a String description of when to start this task
>> private String startTimeDesc = "startOfHour";
>> @@ -49,6 +53,10 @@
>> return "RefreshEntriesTask";
>> }
>> + public String getClientId() {
>> + return clientId;
>> + }
>> + public Date getStartTime(Date currentTime) {
>> return getAdjustedTime(currentTime, startTimeDesc);
>> }
>> @@ -66,6 +74,12 @@
>> // get relevant props
>> Properties props = this.getTaskProperties();
>> + + // extract clientId
>> + String client = props.getProperty("clientId");
>> + if(client != null) {
>> + this.clientId = client;
>> + }
>> // extract start time
>> String startTimeStr = props.getProperty("startTime");
>>
>> Modified:
>> incubator/roller/trunk/src/org/apache/roller/planet/tasks/SyncWebsitesTask.java
>>
>> URL:
>> http://svn.apache.org/viewvc/incubator/roller/trunk/src/org/apache/roller/planet/tasks/SyncWebsitesTask.java?view=diff&rev=495640&r1=495639&r2=495640
>>
>> ==============================================================================
>>
>> ---
>> incubator/roller/trunk/src/org/apache/roller/planet/tasks/SyncWebsitesTask.java
>> (original)
>> +++
>> incubator/roller/trunk/src/org/apache/roller/planet/tasks/SyncWebsitesTask.java
>> Fri Jan 12 08:58:10 2007
>> @@ -47,6 +47,10 @@
>> private static Log log =
>> LogFactory.getLog(SyncWebsitesTask.class);
>> + // a unique id for this specific task instance
>> + // this is meant to be unique for each client in a clustered
>> environment
>> + private String clientId = "unspecifiedClientId";
>> + // a String description of when to start this task
>> private String startTimeDesc = "startOfDay";
>> @@ -61,6 +65,10 @@
>> return "SyncWebsitesTask";
>> }
>> + public String getClientId() {
>> + return clientId;
>> + }
>> + public Date getStartTime(Date currentTime) {
>> return getAdjustedTime(currentTime, startTimeDesc);
>> }
>> @@ -78,6 +86,12 @@
>> // get relevant props
>> Properties props = this.getTaskProperties();
>> + + // extract clientId
>> + String client = props.getProperty("clientId");
>> + if(client != null) {
>> + this.clientId = client;
>> + }
>> // extract start time
>> String startTimeStr = props.getProperty("startTime");
>>
>> Modified:
>> incubator/roller/trunk/tests/org/apache/roller/business/TaskLockTest.java
>> URL:
>> http://svn.apache.org/viewvc/incubator/roller/trunk/tests/org/apache/roller/business/TaskLockTest.java?view=diff&rev=495640&r1=495639&r2=495640
>>
>> ==============================================================================
>>
>> ---
>> incubator/roller/trunk/tests/org/apache/roller/business/TaskLockTest.java
>> (original)
>> +++
>> incubator/roller/trunk/tests/org/apache/roller/business/TaskLockTest.java
>> Fri Jan 12 08:58:10 2007
>> @@ -66,23 +66,15 @@
>> RollerTask task = new TestTask();
>> // try to acquire a lock
>> - boolean lockAcquired = mgr.acquireLock(task);
>> - assertTrue(lockAcquired);
>> + assertTrue(mgr.acquireLock(task));
>> TestUtils.endSession(true);
>> // make sure task is locked
>> - boolean stillLocked = mgr.isLocked(task);
>> - assertTrue(stillLocked);
>> + assertFalse(mgr.acquireLock(task));
>> TestUtils.endSession(true);
>> // try to release a lock
>> - boolean lockReleased = mgr.releaseLock(task);
>> - assertTrue(lockReleased);
>> - TestUtils.endSession(true);
>> - - // make sure task is not locked
>> - stillLocked = mgr.isLocked(task);
>> - assertFalse(stillLocked);
>> + assertTrue(mgr.releaseLock(task));
>> TestUtils.endSession(true);
>> }
>> @@ -90,6 +82,7 @@
>> class TestTask extends RollerTask {
>> public String getName() { return "TestTask"; }
>> + public String getClientId() { return "TestTaskClientId"; }
>> public Date getStartTime(Date current) { return current; }
>> public int getLeaseTime() { return 300; }
>> public int getInterval() { return 1800; }
>>
>> Modified: incubator/roller/trunk/web/WEB-INF/classes/roller.properties
>> URL:
>> http://svn.apache.org/viewvc/incubator/roller/trunk/web/WEB-INF/classes/roller.properties?view=diff&rev=495640&r1=495639&r2=495640
>>
>> ==============================================================================
>>
>> --- incubator/roller/trunk/web/WEB-INF/classes/roller.properties
>> (original)
>> +++ incubator/roller/trunk/web/WEB-INF/classes/roller.properties Fri
>> Jan 12 08:58:10 2007
>> @@ -269,6 +269,9 @@
>> # Tasks which are enabled. Only tasks listed here will be run.
>> tasks.enabled=ResetHitCountsTask,TurnoverReferersTask,PingQueueTask
>>
>> +# client identifier. should be unique for each instance in a cluster.
>> +tasks.clientId=defaultClientId
>> +
>> # Reset hit counts
>> tasks.ResetHitCountsTask.class=org.apache.roller.business.runnable.ResetHitCountsTask
>>
>> tasks.ResetHitCountsTask.startTime=startOfDay
>>
>>
>>