Commits

Anonymous committed c403209

QUARTZ-717: Locking around acquireNextTrigger no longer necessary for JDBC JobStore

git-svn-id: http://svn.opensymphony.com/svn/quartz/trunk@89369f7d36a-ea1c-0410-88ea-9fd03e4c9665

  • Participants
  • Parent commits 024414e

Comments (0)

Files changed (3)

File src/java/org/quartz/impl/jdbcjobstore/DriverDelegate.java

      * @param noEarlierThan 
      *          highest value of <code>getNextFireTime()</code> of the triggers (inclusive)
      *          
-     * @return The next identifier of the next trigger to be fired.
+     * @return A (never null, possibly empty) list of the identifiers (Key objects) of the next triggers to be fired.
      */
-    Key selectTriggerToAcquire(Connection conn, long noLaterThan, long noEarlierThan)
+    List selectTriggerToAcquire(Connection conn, long noLaterThan, long noEarlierThan)
         throws SQLException;
 
     /**

File src/java/org/quartz/impl/jdbcjobstore/JobStoreSupport.java

     
     private boolean setTxIsolationLevelSequential = false;
     
+    private boolean acquireTriggersWithinLock = false;
+    
     private long dbRetryInterval = 10000;
     
     private boolean makeThreadsDaemons = false;
         return setTxIsolationLevelSequential;
     }
 
-    /**
+	/**
      * Set the transaction isolation level of DB connections to sequential.
      * 
      * @param b
         setTxIsolationLevelSequential = b;
     }
 
+    /**
+     * Whether or not the query and update to acquire a Trigger for firing
+     * should be performed after obtaining an explicit DB lock (to avoid 
+     * possible race conditions on the trigger's db row).  This is the
+     * behavior prior to Quartz 1.6.3, but is considered unnecessary for most
+     * databases (due to the nature of the SQL update that is performed), 
+     * and therefore a superfluous performance hit.     
+     */
+    public boolean isAcquireTriggersWithinLock() {
+		return acquireTriggersWithinLock;
+	}
+
+    /**
+     * Whether or not the query and update to acquire a Trigger for firing
+     * should be performed after obtaining an explicit DB lock.  This is the
+     * behavior prior to Quartz 1.6.3, but is considered unnecessary for most
+     * databases, and therefore a superfluous performance hit.     
+     */
+	public void setAcquireTriggersWithinLock(boolean acquireTriggersWithinLock) {
+		this.acquireTriggersWithinLock = acquireTriggersWithinLock;
+	}
+
     
     /**
      * <p>
      */
     public Trigger acquireNextTrigger(final SchedulingContext ctxt, final long noLaterThan)
         throws JobPersistenceException {
-        return (Trigger)executeInNonManagedTXLock(
-                LOCK_TRIGGER_ACCESS,
-                new TransactionCallback() {
-                    public Object execute(Connection conn) throws JobPersistenceException {
-                        return acquireNextTrigger(conn, ctxt, noLaterThan);
-                    }
-                });
+    	
+    	if(isAcquireTriggersWithinLock()) { // behavior before Quartz 1.6.3 release
+	        return (Trigger)executeInNonManagedTXLock(
+	                LOCK_TRIGGER_ACCESS,
+	                new TransactionCallback() {
+	                    public Object execute(Connection conn) throws JobPersistenceException {
+	                        return acquireNextTrigger(conn, ctxt, noLaterThan);
+	                    }
+	                });
+    	}
+    	else { // default behavior since Quartz 1.6.3 release
+	        return (Trigger)executeWithoutLock(
+	                new TransactionCallback() {
+	                    public Object execute(Connection conn) throws JobPersistenceException {
+	                        return acquireNextTrigger(conn, ctxt, noLaterThan);
+	                    }
+	                });
+    	}
     }
     
     // TODO: this really ought to return something like a FiredTriggerBundle,
         throws JobPersistenceException {
         do {
             try {
-                Key triggerKey = getDelegate().selectTriggerToAcquire(conn, noLaterThan, getMisfireTime());
-
-                // No trigger is ready to fire yet.
-                if (triggerKey == null) {
-                    return null;
-                }
-                
-                int rowsUpdated = 
-                    getDelegate().updateTriggerStateFromOtherState(
-                        conn,
-                        triggerKey.getName(), triggerKey.getGroup(), 
-                        STATE_ACQUIRED, STATE_WAITING);
-
-                // If our trigger was no longer in the expected state, try a new one.
-                if (rowsUpdated <= 0) {
-                    continue;
-                }
-
-                Trigger nextTrigger = 
-                    retrieveTrigger(conn, ctxt, triggerKey.getName(), triggerKey.getGroup());
-
-                // If our trigger is no longer available, try a new one.
-                if(nextTrigger == null) {
-                    continue;
-                }
-                  
+            	Trigger nextTrigger = null;
+            	
+            	List keys = getDelegate().selectTriggerToAcquire(conn, noLaterThan, getMisfireTime());
+
+            	// No trigger is ready to fire yet.
+            	if (keys == null || keys.size() == 0)
+            		return null;
+            	
+            	Iterator itr = keys.iterator();
+            	while(itr.hasNext()) {
+	                Key triggerKey = (Key) itr.next();
+	
+	                int rowsUpdated = 
+	                    getDelegate().updateTriggerStateFromOtherState(
+	                        conn,
+	                        triggerKey.getName(), triggerKey.getGroup(), 
+	                        STATE_ACQUIRED, STATE_WAITING);
+	
+	                // If our trigger was no longer in the expected state, try a new one.
+	                if (rowsUpdated <= 0) {
+	                    continue;
+	                }
+	
+	                nextTrigger = 
+	                    retrieveTrigger(conn, ctxt, triggerKey.getName(), triggerKey.getGroup());
+	
+	                // If our trigger is no longer available, try a new one.
+	                if(nextTrigger == null) {
+	                    continue;
+	                }
+	                
+	                break;
+            	}
+            	
                 nextTrigger.setFireInstanceId(getFiredTriggerRecordId());
                 getDelegate().insertFiredTrigger(conn, nextTrigger, STATE_ACQUIRED, null);
                 

File src/java/org/quartz/impl/jdbcjobstore/StdJDBCDelegate.java

      * @param noEarlierThan 
      *          highest value of <code>getNextFireTime()</code> of the triggers (inclusive)
      *          
-     * @return The next identifier of the next trigger to be fired.
+     * @return A (never null, possibly empty) list of the identifiers (Key objects) of the next triggers to be fired.
      */
-    public Key selectTriggerToAcquire(Connection conn, long noLaterThan, long noEarlierThan)
+    public List selectTriggerToAcquire(Connection conn, long noLaterThan, long noEarlierThan)
         throws SQLException {
         PreparedStatement ps = null;
         ResultSet rs = null;
+        List nextTriggers = new LinkedList();
         try {
             ps = conn.prepareStatement(rtp(SELECT_NEXT_TRIGGER_TO_ACQUIRE));
             
             // Try to give jdbc driver a hint to hopefully not pull over 
-            // more than the one row we actually need.
-            ps.setFetchSize(1);
-            ps.setMaxRows(1);
+            // more than the few rows we actually need.
+            ps.setFetchSize(5);
+            ps.setMaxRows(5);
             
             ps.setString(1, STATE_WAITING);
             ps.setBigDecimal(2, new BigDecimal(String.valueOf(noLaterThan)));
             ps.setBigDecimal(3, new BigDecimal(String.valueOf(noEarlierThan)));
             rs = ps.executeQuery();
             
-            if (rs.next()) {
-                return new Key(
+            while (rs.next() && nextTriggers.size() < 5) {
+                nextTriggers.add(new Key(
                         rs.getString(COL_TRIGGER_NAME),
-                        rs.getString(COL_TRIGGER_GROUP));
+                        rs.getString(COL_TRIGGER_GROUP)));
             }
             
-            return null;
+            return nextTriggers;
         } finally {
             closeResultSet(rs);
             closeStatement(ps);