Skip to content

[BATCH-2675] Explicitly considering Start Time not being null for running Job Executions #659

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2006-2013 the original author or authors.
* Copyright 2006-2018 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -18,6 +18,7 @@
import java.io.Serializable;
import java.sql.Timestamp;
import java.util.ArrayList;
import java.util.Date;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
Expand Down Expand Up @@ -204,6 +205,11 @@ public void run() {

try {
JobExecution execution = repository.createJobExecution(job.getName(), new JobParameters());

//simulate running execution
execution.setStartTime(new Date());
repository.update(execution);

cacheJobIds(execution);
list.add(execution);
Thread.sleep(1000);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
* @author Lucas Ward
* @author Michael Minella
* @author Mahmoud Ben Hassine
* @author Dimitrios Liapis
*
*/
@SuppressWarnings("serial")
Expand Down Expand Up @@ -234,10 +235,10 @@ public StepExecution createStepExecution(String stepName) {
* be noted that this does not necessarily mean that it has been persisted
* as such yet.
*
* @return true if the end time is null
* @return true if the end time is null and the start time is not null
*/
public boolean isRunning() {
return endTime == null;
return startTime != null && endTime == null;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
* @author Robert Kasanicky
* @author Michael Minella
* @author Mahmoud Ben Hassine
* @author Dimitrios Liapis
*/
public class JdbcJobExecutionDao extends AbstractJdbcBatchMetadataDao implements JobExecutionDao, InitializingBean {

Expand All @@ -85,7 +86,7 @@ public class JdbcJobExecutionDao extends AbstractJdbcBatchMetadataDao implements
+ " from %PREFIX%JOB_EXECUTION where JOB_EXECUTION_ID = ?";

private static final String GET_RUNNING_EXECUTIONS = "SELECT E.JOB_EXECUTION_ID, E.START_TIME, E.END_TIME, E.STATUS, E.EXIT_CODE, E.EXIT_MESSAGE, E.CREATE_TIME, E.LAST_UPDATED, E.VERSION, "
+ "E.JOB_INSTANCE_ID, E.JOB_CONFIGURATION_LOCATION from %PREFIX%JOB_EXECUTION E, %PREFIX%JOB_INSTANCE I where E.JOB_INSTANCE_ID=I.JOB_INSTANCE_ID and I.JOB_NAME=? and E.END_TIME is NULL order by E.JOB_EXECUTION_ID desc";
+ "E.JOB_INSTANCE_ID, E.JOB_CONFIGURATION_LOCATION from %PREFIX%JOB_EXECUTION E, %PREFIX%JOB_INSTANCE I where E.JOB_INSTANCE_ID=I.JOB_INSTANCE_ID and I.JOB_NAME=? and E.START_TIME is not NULL and E.END_TIME is NULL order by E.JOB_EXECUTION_ID desc";

private static final String CURRENT_VERSION_JOB_EXECUTION = "SELECT VERSION FROM %PREFIX%JOB_EXECUTION WHERE JOB_EXECUTION_ID=?";

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2006-2014 the original author or authors.
* Copyright 2006-2018 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -30,13 +30,15 @@

/**
* @author Dave Syer
* @author Dimitrios Liapis
*
*/
public class JobExecutionTests {

private JobExecution execution = new JobExecution(new JobInstance(new Long(11), "foo"),
new Long(12), new JobParameters(), null);


@Test
public void testJobExecution() {
assertNull(new JobExecution(new JobInstance(null, "foo"), null).getId());
Expand Down Expand Up @@ -65,6 +67,7 @@ public void testGetJobConfigurationName() {
*/
@Test
public void testIsRunning() {
execution.setStartTime(new Date());
assertTrue(execution.isRunning());
execution.setEndTime(new Date(100L));
assertFalse(execution.isRunning());
Expand All @@ -76,6 +79,7 @@ public void testIsRunning() {
*/
@Test
public void testIsRunningWithStoppedExecution() {
execution.setStartTime(new Date());
assertTrue(execution.isRunning());
execution.stop();
assertTrue(execution.isRunning());
Expand Down Expand Up @@ -250,4 +254,5 @@ public void testFailureExceptions() {
assertTrue(allExceptions.contains(exception));
assertTrue(allExceptions.contains(stepException1));
}

}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2010-2014 the original author or authors.
* Copyright 2010-2018 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -18,13 +18,18 @@
import static org.junit.Assert.assertEquals;

import org.junit.Test;
import org.springframework.batch.core.JobExecution;
import org.springframework.batch.core.JobParameters;
import org.springframework.batch.core.explore.JobExplorer;
import org.springframework.batch.core.explore.support.MapJobExplorerFactoryBean;
import org.springframework.batch.core.repository.JobRepository;
import org.springframework.batch.core.repository.support.MapJobRepositoryFactoryBean;

import java.util.Date;

/**
* Tests for {@link MapJobExplorerFactoryBean}.
*
* @author Dimitrios Liapis
*/
public class MapJobExplorerFactoryBeanTests {

Expand All @@ -36,8 +41,13 @@ public class MapJobExplorerFactoryBeanTests {
public void testCreateExplorer() throws Exception {

MapJobRepositoryFactoryBean repositoryFactory = new MapJobRepositoryFactoryBean();
repositoryFactory.getObject().createJobExecution("foo", new JobParameters());

JobRepository jobRepository = repositoryFactory.getObject();
JobExecution jobExecution = jobRepository.createJobExecution("foo", new JobParameters());

//simulating a running job execution
jobExecution.setStartTime(new Date());
jobRepository.update(jobExecution);

MapJobExplorerFactoryBean tested = new MapJobExplorerFactoryBean(repositoryFactory);
tested.afterPropertiesSet();

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2013-2017 the original author or authors.
* Copyright 2013-2018 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -74,6 +74,11 @@
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

/**
* Tests for {@link JsrJobOperator}.
*
* @author Dimitrios Liapis
*/
public class JsrJobOperatorTests extends AbstractJsrTestCase {

private JobOperator jsrJobOperator;
Expand Down Expand Up @@ -210,6 +215,8 @@ public void testAbandonNoSuchJob() throws Exception {
@Test(expected=JobExecutionIsRunningException.class)
public void testAbandonJobRunning() throws Exception {
JobExecution jobExecution = new JobExecution(5L);
jobExecution.setStartTime(new Date(1L));

when(jobExplorer.getJobExecution(5L)).thenReturn(jobExecution);

jsrJobOperator.abandon(5L);
Expand Down Expand Up @@ -677,4 +684,5 @@ public JsrJobOperator jobOperator(JobExplorer jobExplorer, JobRepository jobrepo
return new JsrJobOperator(jobExplorer, jobrepository, jobParametersConverter, transactionManager);
}
}

}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2008-2014 the original author or authors.
* Copyright 2008-2018 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -39,6 +39,11 @@
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;

/**
* Parent Test Class for {@link JdbcJobExecutionDao} and {@link MapJobExecutionDao}.
*
* @author Dimitrios Liapis
*/
public abstract class AbstractJobExecutionDaoTests {

protected JobExecutionDao dao;
Expand Down Expand Up @@ -194,14 +199,24 @@ public void testGetMissingLastExecution() {
@Transactional
@Test
public void testFindRunningExecutions() {

//Normally completed JobExecution as EndTime is populated
JobExecution exec = new JobExecution(jobInstance, jobParameters);
exec.setCreateTime(new Date(0));
exec.setEndTime(new Date(1L));
exec.setStartTime(new Date(1L));
exec.setEndTime(new Date(2L));
exec.setLastUpdated(new Date(5L));
dao.saveJobExecution(exec);

//BATCH-2675
//Abnormal JobExecution as both StartTime and EndTime are null
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding this! It illustrates the issue we are addressing 👍

//This can occur when SimpleJobLauncher#run() submission to taskExecutor throws a TaskRejectedException
exec = new JobExecution(jobInstance, jobParameters);
exec.setLastUpdated(new Date(5L));
dao.saveJobExecution(exec);

//Running JobExecution as StartTime is populated but EndTime is null
exec = new JobExecution(jobInstance, jobParameters);
exec.setStartTime(new Date(2L));
exec.setLastUpdated(new Date(5L));
exec.createStepExecution("step");
dao.saveJobExecution(exec);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2014 the original author or authors.
* Copyright 2014-2018 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -73,6 +73,8 @@ public void testAsyncStopOfStartingJob() throws Exception {
.addLong("test", 1L)
.toJobParameters());

Thread.sleep(1000);

while(restartJobExecution.isRunning()) {
// wait for async launched job to complete execution
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2008-2014 the original author or authors.
* Copyright 2008-2018 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -17,15 +17,20 @@

import org.junit.Test;
import org.springframework.batch.core.Job;
import org.springframework.batch.core.JobExecution;
import org.springframework.batch.core.JobParameters;
import org.springframework.batch.core.job.JobSupport;
import org.springframework.batch.core.repository.JobExecutionAlreadyRunningException;
import org.springframework.batch.core.repository.JobRepository;

import java.util.Date;

import static org.junit.Assert.fail;

/**
* Tests for {@link MapJobRepositoryFactoryBean}.
*
* @author Dimitrios Liapis
*/
public class MapJobRepositoryFactoryBeanTests {

Expand All @@ -42,7 +47,11 @@ public void testCreateRepository() throws Exception {
Job job = new JobSupport("jobName");
JobParameters jobParameters = new JobParameters();

repository.createJobExecution(job.getName(), jobParameters);
JobExecution jobExecution = repository.createJobExecution(job.getName(), jobParameters);

// simulate a running execution
jobExecution.setStartTime(new Date());
repository.update(jobExecution);

try {
repository.createJobExecution(job.getName(), jobParameters);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2008-2014 the original author or authors.
* Copyright 2008-2018 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -43,6 +43,7 @@
* Repository tests using JDBC DAOs (rather than mocks).
*
* @author Robert Kasanicky
* @author Dimitrios Liapis
*/
@RunWith(SpringJUnit4ClassRunner.class)
@ContextConfiguration(locations = "/org/springframework/batch/core/repository/dao/sql-dao-test.xml")
Expand Down Expand Up @@ -180,7 +181,11 @@ public void testSaveExecutionContext() throws Exception {
@Test
public void testOnlyOneJobExecutionAllowedRunning() throws Exception {
job.setRestartable(true);
jobRepository.createJobExecution(job.getName(), jobParameters);
JobExecution jobExecution = jobRepository.createJobExecution(job.getName(), jobParameters);

//simulating a running job execution
jobExecution.setStartTime(new Date());
jobRepository.update(jobExecution);

try {
jobRepository.createJobExecution(job.getName(), jobParameters);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2006-2014 the original author or authors.
* Copyright 2006-2018 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -55,6 +55,7 @@
*
* @author Lucas Ward
* @author Will Schipp
* @author Dimitrios Liapis
*
*/
public class SimpleJobRepositoryTests {
Expand Down Expand Up @@ -240,6 +241,7 @@ public void testIsJobInstanceTrue() throws Exception {
@Test(expected = JobExecutionAlreadyRunningException.class)
public void testCreateJobExecutionAlreadyRunning() throws Exception {
jobExecution.setStatus(BatchStatus.STARTED);
jobExecution.setStartTime(new Date());
jobExecution.setEndTime(null);

when(jobInstanceDao.getJobInstance("foo", new JobParameters())).thenReturn(jobInstance);
Expand Down