svn commit: r1371593 - in /ofbiz/trunk/framework/service/src/org/ofbiz/service/job: JobManager.java PersistedServiceJob.java

[ad...@apache.org]

Author: adrianc
Date: Fri Aug 10 07:50:37 2012
New Revision: 1371593

URL: http://svn.apache.org/viewvc?rev=1371593&view=rev
Log:
JobManager and PersistedServiceJob improvements - based on feedback from Scott.

Modified:
    ofbiz/trunk/framework/service/src/org/ofbiz/service/job/JobManager.java
    ofbiz/trunk/framework/service/src/org/ofbiz/service/job/PersistedServiceJob.java

Modified: ofbiz/trunk/framework/service/src/org/ofbiz/service/job/JobManager.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/service/src/org/ofbiz/service/job/JobManager.java?rev=1371593&r1=1371592&r2=1371593&view=diff
==============================================================================
--- ofbiz/trunk/framework/service/src/org/ofbiz/service/job/JobManager.java (original)
+++ ofbiz/trunk/framework/service/src/org/ofbiz/service/job/JobManager.java Fri Aug 10 07:50:37 2012
@@ -185,11 +185,14 @@ public final class JobManager {
             jobsIterator = delegator.find("JobSandbox", mainCondition, null, null, UtilMisc.toList("runTime"), null);
             GenericValue jobValue = jobsIterator.next();
             while (jobValue != null) {
-                jobValue.set("runByInstanceId", instanceId);  // Claim ownership of this value.
-                jobValue.store();
-                poll.add(new PersistedServiceJob(dctx, jobValue, null));
-                if (poll.size() == limit) {
-                    break;
+                // Claim ownership of this value. Using storeByCondition to avoid a race condition.
+                List<EntityExpr> updateExpression = UtilMisc.toList(EntityCondition.makeCondition("jobId", EntityOperator.EQUALS, jobValue.get("jobId")), EntityCondition.makeCondition("runByInstanceId", EntityOperator.EQUALS, null));
+                int rowsUpdated = delegator.storeByCondition("JobSandbox", UtilMisc.toMap("runByInstanceId", instanceId), EntityCondition.makeCondition(updateExpression));
+                if (rowsUpdated == 1) {
+                    poll.add(new PersistedServiceJob(dctx, jobValue, null));
+                    if (poll.size() == limit) {
+                        break;
+                    }
                 }
                 jobValue = jobsIterator.next();
             }

Modified: ofbiz/trunk/framework/service/src/org/ofbiz/service/job/PersistedServiceJob.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/service/src/org/ofbiz/service/job/PersistedServiceJob.java?rev=1371593&r1=1371592&r2=1371593&view=diff
==============================================================================
--- ofbiz/trunk/framework/service/src/org/ofbiz/service/job/PersistedServiceJob.java (original)
+++ ofbiz/trunk/framework/service/src/org/ofbiz/service/job/PersistedServiceJob.java Fri Aug 10 07:50:37 2012
@@ -31,7 +31,6 @@ import org.apache.commons.lang.StringUti
 import org.ofbiz.base.util.Debug;
 import org.ofbiz.base.util.UtilDateTime;
 import org.ofbiz.base.util.UtilGenerics;
-import org.ofbiz.base.util.UtilProperties;
 import org.ofbiz.base.util.UtilValidate;
 import org.ofbiz.entity.Delegator;
 import org.ofbiz.entity.GenericEntityException;
@@ -94,6 +93,14 @@ public class PersistedServiceJob extends
     @Override
     public void queue() throws InvalidJobException {
         super.queue();
+        try {
+            jobValue.refresh();
+        } catch (GenericEntityException e) {
+            throw new InvalidJobException("Unable to refresh JobSandbox value", e);
+        }
+        if (!JobManager.instanceId.equals(jobValue.getString("runByInstanceId"))) {
+            throw new InvalidJobException("Job has been accepted by a different instance");
+        }
         Timestamp cancelTime = jobValue.getTimestamp("cancelDateTime");
         Timestamp startTime = jobValue.getTimestamp("startDateTime");
         if (cancelTime != null || startTime != null) {
@@ -116,20 +123,17 @@ public class PersistedServiceJob extends
     protected void init() throws InvalidJobException {
         super.init();
         try {
-            // Job might have been canceled after it was placed in the queue.
             jobValue.refresh();
         } catch (GenericEntityException e) {
             throw new InvalidJobException("Unable to refresh JobSandbox value", e);
         }
+        if (!JobManager.instanceId.equals(jobValue.getString("runByInstanceId"))) {
+            throw new InvalidJobException("Job has been accepted by a different instance");
+        }
         if (jobValue.getTimestamp("cancelDateTime") != null) {
             // Job cancelled
             throw new InvalidJobException("Job [" + getJobId() + "] was cancelled");
         }
-        String instanceId = UtilProperties.getPropertyValue("general.properties", "unique.instanceId", "ofbiz0");
-        if (!instanceId.equals(jobValue.getString("runByInstanceId"))) {
-            // This condition isn't possible, but we will leave it here.
-            throw new InvalidJobException("Job has been accepted by a different instance!");
-        }
         jobValue.set("startDateTime", UtilDateTime.nowTimestamp());
         jobValue.set("statusId", "SERVICE_RUNNING");
         try {