RESOLVED - issue BATCH-1541, BATCH-1542: Thread safety for map daos
This commit is contained in:
@@ -20,11 +20,13 @@ import java.io.IOException;
|
||||
import java.io.ObjectInputStream;
|
||||
import java.util.ArrayList;
|
||||
import java.util.Collection;
|
||||
import java.util.Collections;
|
||||
import java.util.Date;
|
||||
import java.util.HashSet;
|
||||
import java.util.LinkedHashSet;
|
||||
import java.util.List;
|
||||
import java.util.Set;
|
||||
import java.util.concurrent.CopyOnWriteArrayList;
|
||||
import java.util.concurrent.CopyOnWriteArraySet;
|
||||
|
||||
import org.springframework.batch.item.ExecutionContext;
|
||||
|
||||
@@ -38,7 +40,7 @@ public class JobExecution extends Entity {
|
||||
|
||||
private JobInstance jobInstance;
|
||||
|
||||
private volatile Collection<StepExecution> stepExecutions = new LinkedHashSet<StepExecution>();
|
||||
private volatile Collection<StepExecution> stepExecutions = new CopyOnWriteArraySet<StepExecution>();
|
||||
|
||||
private volatile BatchStatus status = BatchStatus.STARTING;
|
||||
|
||||
@@ -54,7 +56,7 @@ public class JobExecution extends Entity {
|
||||
|
||||
private volatile ExecutionContext executionContext = new ExecutionContext();
|
||||
|
||||
private transient volatile List<Throwable> failureExceptions = new ArrayList<Throwable>();
|
||||
private transient volatile List<Throwable> failureExceptions = new CopyOnWriteArrayList<Throwable>();
|
||||
|
||||
/**
|
||||
* Because a JobExecution isn't valid unless the job is set, this
|
||||
@@ -164,7 +166,7 @@ public class JobExecution extends Entity {
|
||||
* @return the step executions that were registered
|
||||
*/
|
||||
public Collection<StepExecution> getStepExecutions() {
|
||||
return stepExecutions;
|
||||
return Collections.unmodifiableList(new ArrayList<StepExecution>(stepExecutions));
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -278,7 +280,7 @@ public class JobExecution extends Entity {
|
||||
*
|
||||
* @param t
|
||||
*/
|
||||
public void addFailureException(Throwable t) {
|
||||
public synchronized void addFailureException(Throwable t) {
|
||||
this.failureExceptions.add(t);
|
||||
}
|
||||
|
||||
@@ -289,7 +291,7 @@ public class JobExecution extends Entity {
|
||||
* @return List<Throwable> containing all exceptions causing failure for
|
||||
* this JobExecution.
|
||||
*/
|
||||
public List<Throwable> getAllFailureExceptions() {
|
||||
public synchronized List<Throwable> getAllFailureExceptions() {
|
||||
|
||||
Set<Throwable> allExceptions = new HashSet<Throwable>(failureExceptions);
|
||||
for (StepExecution stepExecution : stepExecutions) {
|
||||
@@ -320,8 +322,8 @@ public class JobExecution extends Entity {
|
||||
}
|
||||
|
||||
/**
|
||||
* Setter for the step executions. For internal use only.
|
||||
* @param stepExecutions
|
||||
* Add some step executions. For internal use only.
|
||||
* @param stepExecutions step executions to add to the current list
|
||||
*/
|
||||
public void addStepExecutions(List<StepExecution> stepExecutions) {
|
||||
if (stepExecutions!=null) {
|
||||
|
||||
@@ -21,6 +21,7 @@ import java.io.ObjectInputStream;
|
||||
import java.util.ArrayList;
|
||||
import java.util.Date;
|
||||
import java.util.List;
|
||||
import java.util.concurrent.CopyOnWriteArrayList;
|
||||
|
||||
import org.springframework.batch.item.ExecutionContext;
|
||||
import org.springframework.util.Assert;
|
||||
@@ -70,7 +71,7 @@ public class StepExecution extends Entity {
|
||||
|
||||
private volatile int filterCount;
|
||||
|
||||
private transient volatile List<Throwable> failureExceptions = new ArrayList<Throwable>();
|
||||
private transient volatile List<Throwable> failureExceptions = new CopyOnWriteArrayList<Throwable>();
|
||||
|
||||
/**
|
||||
* Constructor with mandatory properties.
|
||||
|
||||
@@ -23,11 +23,11 @@ import java.util.HashSet;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
import java.util.Set;
|
||||
import java.util.concurrent.ConcurrentHashMap;
|
||||
|
||||
import org.springframework.batch.core.JobExecution;
|
||||
import org.springframework.batch.core.JobInstance;
|
||||
import org.springframework.batch.support.SerializationUtils;
|
||||
import org.springframework.batch.support.transaction.TransactionAwareProxyFactory;
|
||||
import org.springframework.dao.OptimisticLockingFailureException;
|
||||
import org.springframework.util.Assert;
|
||||
|
||||
@@ -36,7 +36,7 @@ import org.springframework.util.Assert;
|
||||
*/
|
||||
public class MapJobExecutionDao implements JobExecutionDao {
|
||||
|
||||
private Map<Long, JobExecution> executionsById = TransactionAwareProxyFactory.createTransactionalMap();
|
||||
private Map<Long, JobExecution> executionsById = new ConcurrentHashMap<Long, JobExecution>();
|
||||
|
||||
private long currentId = 0;
|
||||
|
||||
|
||||
@@ -21,11 +21,11 @@ import java.util.Collection;
|
||||
import java.util.Collections;
|
||||
import java.util.Comparator;
|
||||
import java.util.List;
|
||||
import java.util.concurrent.CopyOnWriteArraySet;
|
||||
|
||||
import org.springframework.batch.core.JobExecution;
|
||||
import org.springframework.batch.core.JobInstance;
|
||||
import org.springframework.batch.core.JobParameters;
|
||||
import org.springframework.batch.support.transaction.TransactionAwareProxyFactory;
|
||||
import org.springframework.util.Assert;
|
||||
|
||||
/**
|
||||
@@ -33,7 +33,7 @@ import org.springframework.util.Assert;
|
||||
*/
|
||||
public class MapJobInstanceDao implements JobInstanceDao {
|
||||
|
||||
private Collection<JobInstance> jobInstances = TransactionAwareProxyFactory.createTransactionalSet();
|
||||
private Collection<JobInstance> jobInstances = new CopyOnWriteArraySet<JobInstance>();
|
||||
|
||||
private long currentId = 0;
|
||||
|
||||
|
||||
@@ -20,12 +20,13 @@ import java.util.Collections;
|
||||
import java.util.Comparator;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
import java.util.concurrent.ConcurrentHashMap;
|
||||
import java.util.concurrent.atomic.AtomicLong;
|
||||
|
||||
import org.springframework.batch.core.Entity;
|
||||
import org.springframework.batch.core.JobExecution;
|
||||
import org.springframework.batch.core.StepExecution;
|
||||
import org.springframework.batch.support.SerializationUtils;
|
||||
import org.springframework.batch.support.transaction.TransactionAwareProxyFactory;
|
||||
import org.springframework.dao.OptimisticLockingFailureException;
|
||||
import org.springframework.util.Assert;
|
||||
|
||||
@@ -34,13 +35,11 @@ import org.springframework.util.Assert;
|
||||
*/
|
||||
public class MapStepExecutionDao implements StepExecutionDao {
|
||||
|
||||
private Map<Long, Map<Long, StepExecution>> executionsByJobExecutionId = TransactionAwareProxyFactory
|
||||
.createTransactionalMap();
|
||||
private Map<Long, Map<Long, StepExecution>> executionsByJobExecutionId = new ConcurrentHashMap<Long, Map<Long, StepExecution>>();
|
||||
|
||||
private Map<Long, StepExecution> executionsByStepExecutionId = TransactionAwareProxyFactory
|
||||
.createTransactionalMap();
|
||||
private Map<Long, StepExecution> executionsByStepExecutionId = new ConcurrentHashMap<Long, StepExecution>();
|
||||
|
||||
private long currentId = 0;
|
||||
private AtomicLong currentId = new AtomicLong();
|
||||
|
||||
public void clear() {
|
||||
executionsByJobExecutionId.clear();
|
||||
@@ -59,11 +58,11 @@ public class MapStepExecutionDao implements StepExecutionDao {
|
||||
|
||||
Map<Long, StepExecution> executions = executionsByJobExecutionId.get(stepExecution.getJobExecutionId());
|
||||
if (executions == null) {
|
||||
executions = TransactionAwareProxyFactory.createTransactionalMap();
|
||||
executions = new ConcurrentHashMap<Long, StepExecution>();
|
||||
executionsByJobExecutionId.put(stepExecution.getJobExecutionId(), executions);
|
||||
}
|
||||
|
||||
stepExecution.setId(currentId++);
|
||||
|
||||
stepExecution.setId(currentId.incrementAndGet());
|
||||
stepExecution.incrementVersion();
|
||||
StepExecution copy = copy(stepExecution);
|
||||
executions.put(stepExecution.getId(), copy);
|
||||
|
||||
@@ -179,6 +179,8 @@ public class SimpleJobRepository implements JobRepository {
|
||||
}
|
||||
|
||||
public void updateExecutionContext(StepExecution stepExecution) {
|
||||
validateStepExecution(stepExecution);
|
||||
Assert.notNull(stepExecution.getId(), "StepExecution must already be saved (have an id assigned)");
|
||||
ecDao.updateExecutionContext(stepExecution);
|
||||
}
|
||||
|
||||
|
||||
@@ -136,7 +136,7 @@ public class FlowJobTests {
|
||||
@Override
|
||||
public FlowExecutionStatus handle(FlowExecutor executor) throws Exception {
|
||||
JobExecution jobExecution = executor.getJobExecution();
|
||||
jobExecution.getStepExecutions().add(new StepExecution(getName(), jobExecution));
|
||||
jobExecution.createStepExecution(getName());
|
||||
if (fail) {
|
||||
return FlowExecutionStatus.FAILED;
|
||||
}
|
||||
|
||||
@@ -16,7 +16,6 @@
|
||||
package org.springframework.batch.core.job.flow;
|
||||
|
||||
import org.springframework.batch.core.JobExecution;
|
||||
import org.springframework.batch.core.StepExecution;
|
||||
import org.springframework.batch.core.job.flow.support.state.AbstractState;
|
||||
|
||||
/**
|
||||
@@ -42,7 +41,7 @@ public class StateSupport extends AbstractState {
|
||||
public FlowExecutionStatus handle(FlowExecutor executor) throws Exception {
|
||||
JobExecution jobExecution = executor.getJobExecution();
|
||||
if (jobExecution != null) {
|
||||
jobExecution.getStepExecutions().add(new StepExecution(getName(), jobExecution));
|
||||
jobExecution.createStepExecution(getName());
|
||||
}
|
||||
return this.status;
|
||||
}
|
||||
|
||||
@@ -9,7 +9,7 @@ import java.util.Set;
|
||||
|
||||
import org.junit.Before;
|
||||
import org.junit.Test;
|
||||
import org.springframework.batch.core.JobExecution;
|
||||
import org.springframework.batch.core.JobParameters;
|
||||
import org.springframework.batch.core.Step;
|
||||
import org.springframework.batch.core.StepExecution;
|
||||
import org.springframework.batch.core.repository.JobRepository;
|
||||
@@ -23,13 +23,15 @@ public class SimpleStepExecutionSplitterTests {
|
||||
|
||||
private JobRepository jobRepository;
|
||||
|
||||
private StepExecution stepExecution = new StepExecution("bar", new JobExecution(11L));
|
||||
private StepExecution stepExecution;
|
||||
|
||||
@Before
|
||||
public void setUp() throws Exception {
|
||||
step = new TaskletStep("step");
|
||||
MapJobRepositoryFactoryBean factory = new MapJobRepositoryFactoryBean();
|
||||
jobRepository = (JobRepository) factory.getObject();
|
||||
stepExecution = jobRepository.createJobExecution("job", new JobParameters()).createStepExecution("bar");
|
||||
jobRepository.add(stepExecution);
|
||||
}
|
||||
|
||||
@Test
|
||||
|
||||
@@ -39,7 +39,7 @@ public class MapExecutionContextDaoTests extends AbstractExecutionContextDaoTest
|
||||
public void testPersistentCopy() throws Exception {
|
||||
MapExecutionContextDao tested = new MapExecutionContextDao();
|
||||
JobExecution jobExecution = new JobExecution((long)1);
|
||||
StepExecution stepExecution = new StepExecution("stepName", jobExecution);
|
||||
StepExecution stepExecution = new StepExecution("stepName", jobExecution, 123L);
|
||||
assertTrue(stepExecution.getExecutionContext().isEmpty());
|
||||
|
||||
tested.updateExecutionContext(stepExecution);
|
||||
|
||||
Reference in New Issue
Block a user