You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@roller.apache.org by Anil Gangolli <an...@busybuddha.org> on 2007/01/13 17:21:22 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
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>.
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
>>>>
>>>>
>>>>
>>
>