JobStoreSupport.triggersFired not using txValidator compensation logic

[lahma]

I've been digging into a .NET version's problem when database encounters a transient error, like a connectivity problem and thus should be retried. If you look at the code:

quartz/quartz-core/src/main/java/org/quartz/impl/jdbcjobstore/JobStoreSupport.java

Lines 2976 to 3020 in d42fb77

	public List<TriggerFiredResult> triggersFired(final List<OperableTrigger> triggers) throws JobPersistenceException {
	return executeInNonManagedTXLock(LOCK_TRIGGER_ACCESS,
	new TransactionCallback<List<TriggerFiredResult>>() {
	public List<TriggerFiredResult> execute(Connection conn) throws JobPersistenceException {
	List<TriggerFiredResult> results = new ArrayList<TriggerFiredResult>();
	TriggerFiredResult result;
	for (OperableTrigger trigger : triggers) {
	try {
	TriggerFiredBundle bundle = triggerFired(conn, trigger);
	result = new TriggerFiredResult(bundle);
	} catch (JobPersistenceException jpe) {
	result = new TriggerFiredResult(jpe);
	} catch(RuntimeException re) {
	result = new TriggerFiredResult(re);
	}
	results.add(result);
	}
	return results;
	}
	},
	new TransactionValidator<List<TriggerFiredResult>>() {
	@Override
	public Boolean validate(Connection conn, List<TriggerFiredResult> result) throws JobPersistenceException {
	try {
	List<FiredTriggerRecord> acquired = getDelegate().selectInstancesFiredTriggerRecords(conn, getInstanceId());
	Set<String> executingTriggers = new HashSet<String>();
	for (FiredTriggerRecord ft : acquired) {
	if (STATE_EXECUTING.equals(ft.getFireInstanceState())) {
	executingTriggers.add(ft.getFireInstanceId());
	}
	}
	for (TriggerFiredResult tr : result) {
	if (tr.getTriggerFiredBundle() != null && executingTriggers.contains(tr.getTriggerFiredBundle().getTrigger().getFireInstanceId())) {
	return true;
	}
	}
	return false;
	} catch (SQLException e) {
	throw new JobPersistenceException("error validating trigger acquisition", e);
	}
	}
	});
	}

All exceptions are caught in main txCallback and thus I don't see a way how the txValidator logic would ever be called. This will mean that this operation is not retried, unlike the acquireNextTriggers which allows exceptions to bubble up:

quartz/quartz-core/src/main/java/org/quartz/impl/jdbcjobstore/JobStoreSupport.java

Lines 2802 to 2826 in d42fb77

	return executeInNonManagedTXLock(lockName,
	new TransactionCallback<List<OperableTrigger>>() {
	public List<OperableTrigger> execute(Connection conn) throws JobPersistenceException {
	return acquireNextTrigger(conn, noLaterThan, maxCount, timeWindow);
	}
	},
	new TransactionValidator<List<OperableTrigger>>() {
	public Boolean validate(Connection conn, List<OperableTrigger> result) throws JobPersistenceException {
	try {
	List<FiredTriggerRecord> acquired = getDelegate().selectInstancesFiredTriggerRecords(conn, getInstanceId());
	Set<String> fireInstanceIds = new HashSet<String>();
	for (FiredTriggerRecord ft : acquired) {
	fireInstanceIds.add(ft.getFireInstanceId());
	}
	for (OperableTrigger tr : result) {
	if (fireInstanceIds.contains(tr.getFireInstanceId())) {
	return true;
	}
	}
	return false;
	} catch (SQLException e) {
	throw new JobPersistenceException("error validating trigger acquisition", e);
	}
	}
	});

So before changing the behavior on .NET side I was hoping to get some feedback from Java side. Should the errors be allowed to bubble up or is the txValidator just redundant?

[stale]

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

[lahma]

This is relevant and I would like to discuss whether it requires a fix.

[stale]

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward? This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

[lahma]

This is relevant and I would like to discuss whether it requires a fix.

[fil512]

I'm seeing connection pool leaks through this stack. Wonder if it's related?

2021-09-27 05:08:39.015 [admin_json.adminJsonEndpointJetty-1284] DEBUG R:7uJpuSm0o0kFtccm c.c.l.connection_pool_troubleshooting - 	at org.springframework.scheduling.quartz.LocalDataSourceJobStore$2.getConnection(LocalDataSourceJobStore.java:136)
2021-09-27 05:08:39.015 [admin_json.adminJsonEndpointJetty-1284] DEBUG R:7uJpuSm0o0kFtccm c.c.l.connection_pool_troubleshooting - 	at org.quartz.utils.DBConnectionManager.getConnection(DBConnectionManager.java:108)
2021-09-27 05:08:39.015 [admin_json.adminJsonEndpointJetty-1284] DEBUG R:7uJpuSm0o0kFtccm c.c.l.connection_pool_troubleshooting - 	at org.quartz.impl.jdbcjobstore.JobStoreCMT.getNonManagedTXConnection(JobStoreCMT.java:165)
2021-09-27 05:08:39.015 [admin_json.adminJsonEndpointJetty-1284] DEBUG R:7uJpuSm0o0kFtccm c.c.l.connection_pool_troubleshooting - 	at org.quartz.impl.jdbcjobstore.JobStoreSupport.executeInNonManagedTXLock(JobStoreSupport.java:3854)
2021-09-27 05:08:39.015 [admin_json.adminJsonEndpointJetty-1284] DEBUG R:7uJpuSm0o0kFtccm c.c.l.connection_pool_troubleshooting - 	at org.quartz.impl.jdbcjobstore.JobStoreSupport.triggersFired(JobStoreSupport.java:2977)

[stale]

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward? This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

[lahma]

This is relevant and I would like to discuss whether it requires a fix.

[stale]

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward? This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

[lahma]

.

[stale]

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward? This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

[lahma]

.

[lahma]

Oh for fuck's sake.

[vssarahan]

@lahma Is this problem relevant? if not, what solution did you implement? I'm currently using quartz 2.6.2 for .net, and would like to adopt an up-to-date solution.

[lahma]

This hasn't been addressed on either side to date.