Sahina Bose has uploaded a new change for review. Change subject: engine: Clean up orphan gluster task references ......................................................................
engine: Clean up orphan gluster task references Gluster tasks sync job periodically updates status of gluster tasks in engine. However, if for some reason (for instance, if engine was offline for a period of time) gluster does not return a task's information, engine should mark the job ended with task status UNKNOWN Change-Id: I61f921109fb710dfb3f7255b54595c882c6a076c Bug-Url: https://bugzilla.redhat.com/1022996 Signed-off-by: Sahina Bose <sab...@redhat.com> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterTasksSyncJob.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/tasks/GlusterTasksService.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/gluster/GlusterTasksSyncJobTest.java M backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StepDao.java M backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StepDaoDbFacadeImpl.java M backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/StepDaoTest.java M packaging/dbscripts/job_sp.sql 7 files changed, 116 insertions(+), 8 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/54/20954/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterTasksSyncJob.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterTasksSyncJob.java index bc0d382..0cabb80 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterTasksSyncJob.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterTasksSyncJob.java @@ -2,9 +2,11 @@ import java.util.ArrayList; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Map.Entry; +import java.util.Set; import org.ovirt.engine.core.bll.Backend; import org.ovirt.engine.core.bll.gluster.tasks.GlusterTasksService; @@ -22,6 +24,7 @@ import org.ovirt.engine.core.common.businessentities.VDSGroup; import org.ovirt.engine.core.common.businessentities.VdsStatic; import org.ovirt.engine.core.common.businessentities.gluster.GlusterBrickEntity; +import org.ovirt.engine.core.common.businessentities.gluster.GlusterStatus; import org.ovirt.engine.core.common.businessentities.gluster.GlusterVolumeEntity; import org.ovirt.engine.core.common.constants.gluster.GlusterConstants; import org.ovirt.engine.core.common.errors.VdcBLLException; @@ -76,10 +79,16 @@ log.debug("Refreshing gluster tasks list"); List<VDSGroup> clusters = getClusterDao().getAll(); + List<Guid> tasksFromClusters = new ArrayList<>(); for (VDSGroup cluster : clusters) { - updateTasksInCluster(cluster); + Map<Guid, GlusterAsyncTask> runningTasks = updateTasksInCluster(cluster); + if (runningTasks != null) { + tasksFromClusters.addAll(runningTasks.keySet()); + } } + + cleanUpOrphanTasks(tasksFromClusters); } @@ -345,4 +354,34 @@ private boolean supportsGlusterAsyncTasksFeature(VDSGroup cluster) { return cluster.supportsGlusterService() && GlusterFeatureSupported.glusterAsyncTasks(cluster.getcompatibility_version()); } + + private void cleanUpOrphanTasks(List<Guid> runningTasksinCluster) { + //Populate the list of tasks that need to be monitored from database + List<Guid> taskListInDB = getProvider().getMonitoredTaskIDsInDB(); + if (taskListInDB == null) { + taskListInDB = new ArrayList<Guid>(); + } + + //if task is in DB but not in running task list + final Set<Guid> tasksNotRunning = new HashSet<Guid>(taskListInDB); + tasksNotRunning.removeAll(runningTasksinCluster); + + for (Guid taskId: tasksNotRunning) { + GlusterVolumeEntity vol= getVolumeDao().getVolumeByGlusterTask(taskId); + if (vol != null && vol.getStatus() != GlusterStatus.UP) { + //the volume is not UP. Hence gluster may not have been able to return tasks for the volume + continue; + } + + //Volume is up, but gluster does not know of task + //will mark job ended with status unknown. + List<Step> steps = getStepDao().getStepsByExternalId(taskId); + for (Step step: steps) { + step.markStepEnded(JobExecutionStatus.UNKNOWN); + endStepJob(step); + } + releaseVolumeLock(taskId); + } + + } } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/tasks/GlusterTasksService.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/tasks/GlusterTasksService.java index 3c5b2b0..ba5fde5 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/tasks/GlusterTasksService.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/tasks/GlusterTasksService.java @@ -8,11 +8,13 @@ import org.ovirt.engine.core.bll.utils.ClusterUtils; import org.ovirt.engine.core.common.asynctasks.gluster.GlusterAsyncTask; import org.ovirt.engine.core.common.businessentities.VDS; +import org.ovirt.engine.core.common.job.ExternalSystemType; import org.ovirt.engine.core.common.vdscommands.VDSCommandType; import org.ovirt.engine.core.common.vdscommands.VDSParametersBase; import org.ovirt.engine.core.common.vdscommands.VDSReturnValue; import org.ovirt.engine.core.common.vdscommands.VdsIdVDSCommandParametersBase; import org.ovirt.engine.core.compat.Guid; +import org.ovirt.engine.core.dal.dbbroker.DbFacade; import org.ovirt.engine.core.utils.log.Log; import org.ovirt.engine.core.utils.log.LogFactory; @@ -51,9 +53,9 @@ * @return */ public List<Guid> getMonitoredTaskIDsInDB() { - // List<Guid> externalIds = DbFacade.getInstance().getStepDao(). - // getExternalIdsForRunningSteps(ExternalSystemType.GLUSTER); - return null; + List<Guid> externalIds = DbFacade.getInstance().getStepDao(). + getExternalIdsForRunningSteps(ExternalSystemType.GLUSTER); + return externalIds; } private VDSReturnValue runVdsCommand(VDSCommandType commandType, VDSParametersBase params) { diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/gluster/GlusterTasksSyncJobTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/gluster/GlusterTasksSyncJobTest.java index f56b97f..ec385b5 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/gluster/GlusterTasksSyncJobTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/gluster/GlusterTasksSyncJobTest.java @@ -7,6 +7,7 @@ import static org.ovirt.engine.core.utils.MockConfigRule.mockConfig; import java.util.ArrayList; +import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -31,6 +32,7 @@ import org.ovirt.engine.core.common.asynctasks.gluster.GlusterTaskParameters; import org.ovirt.engine.core.common.asynctasks.gluster.GlusterTaskType; import org.ovirt.engine.core.common.businessentities.VDSGroup; +import org.ovirt.engine.core.common.businessentities.gluster.GlusterStatus; import org.ovirt.engine.core.common.businessentities.gluster.GlusterVolumeEntity; import org.ovirt.engine.core.common.config.ConfigValues; import org.ovirt.engine.core.common.job.JobExecutionStatus; @@ -51,10 +53,12 @@ new Guid("CC222222-2222-2222-2222-222222222222")}; private static final Guid[] TASK_GUIDS = {new Guid("EE111111-1111-1111-1111-111111111111"), - new Guid("EE222222-2222-2222-2222-222222222222")}; + new Guid("EE222222-2222-2222-2222-222222222222"), + new Guid("EE333333-3333-3333-3333-333333333333")}; private static final Guid[] VOL_GUIDS = {new Guid("AA111111-1111-1111-1111-111111111111"), - new Guid("AA222222-2222-2222-2222-222222222222")}; + new Guid("AA222222-2222-2222-2222-222222222222"), + new Guid("AA333333-3333-3333-3333-333333333333")}; @ClassRule public static MockEJBStrategyRule ejbRule = new MockEJBStrategyRule(); @@ -105,7 +109,7 @@ doReturn(backend).when(tasksSyncJob).getBackend(); doNothing().when(tasksSyncJob).releaseLock(any(Guid.class)); doNothing().when(tasksSyncJob).endStepJob(any(Step.class)); - + doReturn(null).when(provider).getMonitoredTaskIDsInDB(); } @Test @@ -115,6 +119,28 @@ tasksSyncJob.updateGlusterAsyncTasks(); Mockito.verify(jobRepository, times(1)).updateStep(any(Step.class)); + Mockito.verify(tasksSyncJob, times(1)).endStepJob(any(Step.class)); + } + + @Test + public void cleanOrphanTasks() { + doReturn(getTasks()).when(provider).getTaskListForCluster(CLUSTER_GUIDS[1]); + doReturn(Arrays.asList(TASK_GUIDS[2])).when(provider).getMonitoredTaskIDsInDB(); + prepareMocks(); + + tasksSyncJob.updateGlusterAsyncTasks(); + Mockito.verify(jobRepository, times(1)).updateStep(any(Step.class)); + Mockito.verify(tasksSyncJob, times(2)).endStepJob(any(Step.class)); + } + + @Test + public void cleanOrphanTasksWhenNoVolume() { + doReturn(null).when(provider).getTaskListForCluster(CLUSTER_GUIDS[1]); + doReturn(Arrays.asList(TASK_GUIDS[2])).when(provider).getMonitoredTaskIDsInDB(); + doReturn(null).when(volumeDao).getVolumeByGlusterTask(TASK_GUIDS[2]); + doReturn(getSteps()).when(stepDao).getStepsByExternalId(TASK_GUIDS[2]); + + tasksSyncJob.updateGlusterAsyncTasks(); Mockito.verify(tasksSyncJob, times(1)).endStepJob(any(Step.class)); } @@ -176,9 +202,11 @@ private void prepareMocks() { doReturn(getVolume(0)).when(volumeDao).getVolumeByGlusterTask(TASK_GUIDS[0]); doReturn(getVolume(1)).when(volumeDao).getVolumeByGlusterTask(TASK_GUIDS[1]); + doReturn(getVolume(1)).when(volumeDao).getVolumeByGlusterTask(TASK_GUIDS[2]); doReturn(getSteps()).when(stepDao).getStepsByExternalId(TASK_GUIDS[0]); doReturn(getSteps()).when(stepDao).getStepsByExternalId(TASK_GUIDS[1]); - } + doReturn(getSteps()).when(stepDao).getStepsByExternalId(TASK_GUIDS[2]); + } private void prepareMocksForTasksFromCLI() { doReturn(getVolume(0)).when(volumeDao).getVolumeByGlusterTask(TASK_GUIDS[0]); @@ -202,6 +230,7 @@ private GlusterVolumeEntity getVolume(int i) { GlusterVolumeEntity vol = new GlusterVolumeEntity(); + vol.setStatus(GlusterStatus.UP); vol.setId(VOL_GUIDS[i]); return vol; } diff --git a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StepDao.java b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StepDao.java index c388efb..46e1fb8 100644 --- a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StepDao.java +++ b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StepDao.java @@ -3,6 +3,7 @@ import java.util.Date; import java.util.List; +import org.ovirt.engine.core.common.job.ExternalSystemType; import org.ovirt.engine.core.common.job.JobExecutionStatus; import org.ovirt.engine.core.common.job.Step; import org.ovirt.engine.core.compat.Guid; @@ -51,5 +52,10 @@ * @return */ List<Step> getStepsByExternalId(Guid externalId); + + /** + * Retrieve all external ids for steps that are not yet completed + */ + List<Guid> getExternalIdsForRunningSteps(ExternalSystemType systemType); } diff --git a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StepDaoDbFacadeImpl.java b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StepDaoDbFacadeImpl.java index 5851e55..8c52eb0 100644 --- a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StepDaoDbFacadeImpl.java +++ b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StepDaoDbFacadeImpl.java @@ -107,4 +107,11 @@ return getCallsHandler().executeReadList("GetStepsByExternalTaskId", createEntityRowMapper(), parameterSource); } + @Override + public List<Guid> getExternalIdsForRunningSteps(ExternalSystemType systemType) { + MapSqlParameterSource parameterSource = getCustomMapSqlParameterSource().addValue("external_system_type", systemType.name()) + .addValue("status", JobExecutionStatus.STARTED.name()); + return getCallsHandler().executeReadList("GetExternalIdsFromSteps", createGuidMapper(), parameterSource); + } + } diff --git a/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/StepDaoTest.java b/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/StepDaoTest.java index 43cf4c8..9b81996 100644 --- a/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/StepDaoTest.java +++ b/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/StepDaoTest.java @@ -10,6 +10,7 @@ import org.junit.Before; import org.junit.Test; +import org.ovirt.engine.core.common.job.ExternalSystemType; import org.ovirt.engine.core.common.job.JobExecutionStatus; import org.ovirt.engine.core.common.job.Step; import org.ovirt.engine.core.common.job.StepEnum; @@ -116,4 +117,11 @@ assertTrue("Verify the Step status", JobExecutionStatus.STARTED == step.getStatus()); assertEquals("Invalid Step", REBALANCING_GLUSTER_VOLUME_STEP_ID, step.getId()); } + + @Test + public void getExternalIdsForRunningSteps(){ + List<Guid> externalIds = dao.getExternalIdsForRunningSteps(ExternalSystemType.GLUSTER); + assertEquals("Verify external ids present", 1, externalIds.size()); + assertEquals("Invalid TaskId", IN_PROGRESS_REBALANCING_GLUSTER_VOLUME_TASK_ID, externalIds.get(0)); + } } diff --git a/packaging/dbscripts/job_sp.sql b/packaging/dbscripts/job_sp.sql index 2f13bb3..d151e55 100644 --- a/packaging/dbscripts/job_sp.sql +++ b/packaging/dbscripts/job_sp.sql @@ -518,3 +518,20 @@ ORDER BY parent_step_id nulls first, step_number; END; $procedure$ LANGUAGE plpgsql; + +---------------------------------------------------- +-- Gets list of external task UUIDs from Step table +-- for steps based on external system type and job status +---------------------------------------------------- +Create or replace FUNCTION GetExternalIdsFromSteps(v_status VARCHAR(32), + v_external_system_type VARCHAR(32)) +RETURNS SETOF UUID STABLE +AS $procedure$ +BEGIN + RETURN QUERY SELECT step.external_id + FROM step + INNER JOIN job ON step.job_id = job.job_id + WHERE job.status = v_status + AND step.external_system_type = v_external_system_type; +END; $procedure$ +LANGUAGE plpgsql; -- To view, visit http://gerrit.ovirt.org/20954 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I61f921109fb710dfb3f7255b54595c882c6a076c Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Sahina Bose <sab...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
