From 3b4c6a7030b8a4f83d51dbfae55cd70fe6afb8a6 Mon Sep 17 00:00:00 2001 From: Lukas Krejci Date: Wed, 16 Jan 2019 16:21:56 +0100 Subject: [PATCH] Added a periodical check to reset the activity records of workspaces that may run out of sync (circumstances are not 100% clear, but we've seen it happening). Signed-off-by: Lukas Krejci --- .../MultiUserWorkspaceActivityManager.java | 4 +- ...MultiUserWorkspaceActivityManagerTest.java | 3 + .../InmemoryWorkspaceActivityDao.java | 10 +++ .../activity/JpaWorkspaceActivityDao.java | 17 +++++ .../activity/WorkspaceActivityDao.java | 10 +++ .../activity/WorkspaceActivityManager.java | 76 ++++++++++++++++++- .../WorkspaceActivityManagerTest.java | 8 +- 7 files changed, 125 insertions(+), 3 deletions(-) diff --git a/multiuser/api/che-multiuser-api-workspace-activity/src/main/java/org/eclipse/che/multiuser/api/workspace/activity/MultiUserWorkspaceActivityManager.java b/multiuser/api/che-multiuser-api-workspace-activity/src/main/java/org/eclipse/che/multiuser/api/workspace/activity/MultiUserWorkspaceActivityManager.java index d98fc45b9f..ca648adb87 100644 --- a/multiuser/api/che-multiuser-api-workspace-activity/src/main/java/org/eclipse/che/multiuser/api/workspace/activity/MultiUserWorkspaceActivityManager.java +++ b/multiuser/api/che-multiuser-api-workspace-activity/src/main/java/org/eclipse/che/multiuser/api/workspace/activity/MultiUserWorkspaceActivityManager.java @@ -24,6 +24,7 @@ import org.eclipse.che.api.core.notification.EventService; import org.eclipse.che.api.workspace.activity.WorkspaceActivityDao; import org.eclipse.che.api.workspace.activity.WorkspaceActivityManager; import org.eclipse.che.api.workspace.server.WorkspaceManager; +import org.eclipse.che.api.workspace.server.WorkspaceRuntimes; import org.eclipse.che.api.workspace.server.model.impl.WorkspaceImpl; import org.eclipse.che.multiuser.resource.api.type.TimeoutResourceType; import org.eclipse.che.multiuser.resource.api.usage.ResourceManager; @@ -50,12 +51,13 @@ public class MultiUserWorkspaceActivityManager extends WorkspaceActivityManager @Inject public MultiUserWorkspaceActivityManager( WorkspaceManager workspaceManager, + WorkspaceRuntimes workspaceRuntimes, WorkspaceActivityDao activityDao, EventService eventService, AccountManager accountManager, ResourceManager resourceManager, @Named("che.limits.workspace.idle.timeout") long defaultTimeout) { - super(workspaceManager, activityDao, eventService, defaultTimeout); + super(workspaceManager, workspaceRuntimes, activityDao, eventService, defaultTimeout); this.accountManager = accountManager; this.resourceManager = resourceManager; this.defaultTimeout = defaultTimeout; diff --git a/multiuser/api/che-multiuser-api-workspace-activity/src/test/java/org/eclipse/che/multiuser/api/workspace/activity/MultiUserWorkspaceActivityManagerTest.java b/multiuser/api/che-multiuser-api-workspace-activity/src/test/java/org/eclipse/che/multiuser/api/workspace/activity/MultiUserWorkspaceActivityManagerTest.java index d2b08d61b0..7e796e345c 100644 --- a/multiuser/api/che-multiuser-api-workspace-activity/src/test/java/org/eclipse/che/multiuser/api/workspace/activity/MultiUserWorkspaceActivityManagerTest.java +++ b/multiuser/api/che-multiuser-api-workspace-activity/src/test/java/org/eclipse/che/multiuser/api/workspace/activity/MultiUserWorkspaceActivityManagerTest.java @@ -22,6 +22,7 @@ import org.eclipse.che.api.core.notification.EventService; import org.eclipse.che.api.core.notification.EventSubscriber; import org.eclipse.che.api.workspace.activity.WorkspaceActivityDao; import org.eclipse.che.api.workspace.server.WorkspaceManager; +import org.eclipse.che.api.workspace.server.WorkspaceRuntimes; import org.eclipse.che.api.workspace.server.model.impl.WorkspaceImpl; import org.eclipse.che.api.workspace.shared.dto.event.WorkspaceStatusEvent; import org.eclipse.che.multiuser.resource.api.type.TimeoutResourceType; @@ -44,6 +45,7 @@ public class MultiUserWorkspaceActivityManagerTest { @Mock private ResourceManager resourceManager; @Mock private WorkspaceManager workspaceManager; + @Mock private WorkspaceRuntimes workspaceRuntimes; @Captor private ArgumentCaptor> captor; @@ -60,6 +62,7 @@ public class MultiUserWorkspaceActivityManagerTest { activityManager = new MultiUserWorkspaceActivityManager( workspaceManager, + workspaceRuntimes, workspaceActivityDao, eventService, accountManager, diff --git a/wsmaster/che-core-api-workspace-activity/src/main/java/org/eclipse/che/api/workspace/activity/InmemoryWorkspaceActivityDao.java b/wsmaster/che-core-api-workspace-activity/src/main/java/org/eclipse/che/api/workspace/activity/InmemoryWorkspaceActivityDao.java index 3730641f9e..0b0ef05bed 100644 --- a/wsmaster/che-core-api-workspace-activity/src/main/java/org/eclipse/che/api/workspace/activity/InmemoryWorkspaceActivityDao.java +++ b/wsmaster/che-core-api-workspace-activity/src/main/java/org/eclipse/che/api/workspace/activity/InmemoryWorkspaceActivityDao.java @@ -16,6 +16,7 @@ import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.stream.Collectors; import javax.inject.Singleton; +import org.eclipse.che.api.core.ConflictException; import org.eclipse.che.api.core.Page; import org.eclipse.che.api.core.ServerException; import org.eclipse.che.api.core.model.workspace.WorkspaceStatus; @@ -139,6 +140,15 @@ public class InmemoryWorkspaceActivityDao implements WorkspaceActivityDao { workspaceActivities.remove(workspaceId); } + @Override + public void createActivity(WorkspaceActivity activity) throws ConflictException { + if (workspaceActivities.containsKey(activity.getWorkspaceId())) { + throw new ConflictException("Already exists."); + } else { + workspaceActivities.put(activity.getWorkspaceId(), activity); + } + } + private boolean isGreater(Long value, long threshold) { return value != null && value > threshold; } diff --git a/wsmaster/che-core-api-workspace-activity/src/main/java/org/eclipse/che/api/workspace/activity/JpaWorkspaceActivityDao.java b/wsmaster/che-core-api-workspace-activity/src/main/java/org/eclipse/che/api/workspace/activity/JpaWorkspaceActivityDao.java index 8eead7389a..862f044a5b 100644 --- a/wsmaster/che-core-api-workspace-activity/src/main/java/org/eclipse/che/api/workspace/activity/JpaWorkspaceActivityDao.java +++ b/wsmaster/che-core-api-workspace-activity/src/main/java/org/eclipse/che/api/workspace/activity/JpaWorkspaceActivityDao.java @@ -20,7 +20,9 @@ import java.util.stream.Collectors; import javax.inject.Inject; import javax.inject.Provider; import javax.inject.Singleton; +import javax.persistence.EntityExistsException; import javax.persistence.EntityManager; +import org.eclipse.che.api.core.ConflictException; import org.eclipse.che.api.core.Page; import org.eclipse.che.api.core.ServerException; import org.eclipse.che.api.core.model.workspace.WorkspaceStatus; @@ -203,6 +205,21 @@ public class JpaWorkspaceActivityDao implements WorkspaceActivityDao { } } + @Override + @Transactional(rollbackOn = {ConflictException.class, ServerException.class}) + public void createActivity(WorkspaceActivity activity) throws ConflictException, ServerException { + try { + EntityManager em = managerProvider.get(); + em.persist(activity); + em.flush(); + } catch (EntityExistsException e) { + throw new ConflictException( + "Activity record for workspace ID " + activity.getWorkspaceId() + " already exists.", e); + } catch (RuntimeException e) { + throw new ServerException(e.getLocalizedMessage(), e); + } + } + @Transactional(rollbackOn = ServerException.class) protected void doUpdate(String workspaceId, Consumer updater) throws ServerException { diff --git a/wsmaster/che-core-api-workspace-activity/src/main/java/org/eclipse/che/api/workspace/activity/WorkspaceActivityDao.java b/wsmaster/che-core-api-workspace-activity/src/main/java/org/eclipse/che/api/workspace/activity/WorkspaceActivityDao.java index dcacda90dc..a98551f20c 100644 --- a/wsmaster/che-core-api-workspace-activity/src/main/java/org/eclipse/che/api/workspace/activity/WorkspaceActivityDao.java +++ b/wsmaster/che-core-api-workspace-activity/src/main/java/org/eclipse/che/api/workspace/activity/WorkspaceActivityDao.java @@ -12,6 +12,7 @@ package org.eclipse.che.api.workspace.activity; import java.util.List; +import org.eclipse.che.api.core.ConflictException; import org.eclipse.che.api.core.Page; import org.eclipse.che.api.core.ServerException; import org.eclipse.che.api.core.model.workspace.WorkspaceStatus; @@ -116,4 +117,13 @@ public interface WorkspaceActivityDao { * @throws ServerException on error */ WorkspaceActivity findActivity(String workspaceId) throws ServerException; + + /** + * Creates a new activity record. Fails if activity record already exists. + * + * @param activity the activity to persist + * @throws ConflictException when activity record exists + * @throws ServerException on other error + */ + void createActivity(WorkspaceActivity activity) throws ConflictException, ServerException; } diff --git a/wsmaster/che-core-api-workspace-activity/src/main/java/org/eclipse/che/api/workspace/activity/WorkspaceActivityManager.java b/wsmaster/che-core-api-workspace-activity/src/main/java/org/eclipse/che/api/workspace/activity/WorkspaceActivityManager.java index 92a2cfea99..25d7b77dba 100644 --- a/wsmaster/che-core-api-workspace-activity/src/main/java/org/eclipse/che/api/workspace/activity/WorkspaceActivityManager.java +++ b/wsmaster/che-core-api-workspace-activity/src/main/java/org/eclipse/che/api/workspace/activity/WorkspaceActivityManager.java @@ -30,6 +30,7 @@ import org.eclipse.che.api.core.model.workspace.WorkspaceStatus; import org.eclipse.che.api.core.notification.EventService; import org.eclipse.che.api.core.notification.EventSubscriber; import org.eclipse.che.api.workspace.server.WorkspaceManager; +import org.eclipse.che.api.workspace.server.WorkspaceRuntimes; import org.eclipse.che.api.workspace.server.event.BeforeWorkspaceRemovedEvent; import org.eclipse.che.api.workspace.shared.Constants; import org.eclipse.che.api.workspace.shared.dto.event.WorkspaceStatusEvent; @@ -67,14 +68,17 @@ public class WorkspaceActivityManager { private final EventSubscriber workspaceActivityRemover; protected final WorkspaceManager workspaceManager; + private final WorkspaceRuntimes workspaceRuntimes; @Inject public WorkspaceActivityManager( WorkspaceManager workspaceManager, + WorkspaceRuntimes workspaceRuntimes, WorkspaceActivityDao activityDao, EventService eventService, @Named("che.limits.workspace.idle.timeout") long timeout) { this.workspaceManager = workspaceManager; + this.workspaceRuntimes = workspaceRuntimes; this.eventService = eventService; this.activityDao = activityDao; this.defaultTimeout = timeout; @@ -167,12 +171,82 @@ public class WorkspaceActivityManager { delayParameterName = "che.workspace.activity_check_scheduler_period_s") private void invalidate() { try { - activityDao.findExpired(System.currentTimeMillis()).forEach(this::stopExpired); + stopAllExpired(); + checkValidExpiration(); } catch (ServerException e) { LOG.error(e.getLocalizedMessage(), e); } } + private void stopAllExpired() throws ServerException { + activityDao.findExpired(System.currentTimeMillis()).forEach(this::stopExpired); + } + + private void checkValidExpiration() throws ServerException { + for (String runningWsId : workspaceRuntimes.getRunning()) { + WorkspaceActivity activity = activityDao.findActivity(runningWsId); + if (activity == null) { + LOG.warn( + "Found a running workspace {} without any activity record. This shouldn't really" + + " happen but is being rectified by adding a new activity record for it.", + runningWsId); + + try { + Workspace workspace = workspaceManager.getWorkspace(runningWsId); + long createdTime = + Long.parseLong(workspace.getAttributes().get(Constants.CREATED_ATTRIBUTE_NAME)); + + activity = new WorkspaceActivity(); + activity.setWorkspaceId(runningWsId); + activity.setCreated(createdTime); + activity.setStatus(WorkspaceStatus.RUNNING); + activity.setLastRunning(System.currentTimeMillis()); + + long idleTimeout = getIdleTimeout(runningWsId); + if (idleTimeout > 0) { + // only set the expiration if it is used... + activity.setExpiration(System.currentTimeMillis() + idleTimeout); + } + + activityDao.createActivity(activity); + } catch (NotFoundException e) { + LOG.error( + "Detected a running workspace {} but could not find its record.", runningWsId, e); + } catch (ConflictException e) { + LOG.debug( + "Activity record created while we were trying to rectify its absence for a" + + " running workspace {}.", + runningWsId); + } + } else { + long idleTimeout = getIdleTimeout(runningWsId); + if (idleTimeout == 0) { + // expiry not used at all... + continue; + } + + long expectedExpiryTime = + valueOrDefault(activity.getLastRunning(), System.currentTimeMillis()) + idleTimeout; + long actualExpiryTime = valueOrDefault(activity.getExpiration(), 0); + + if (actualExpiryTime != expectedExpiryTime) { + LOG.debug( + "Detected expiry time out of sync with the expected. Based on the last time" + + " the workspace {} has started, the expiry should be {} but was found to be {}." + + " Rectifying.", + runningWsId, + expectedExpiryTime, + actualExpiryTime); + update(runningWsId, expectedExpiryTime); + } + } + } + } + + private static long valueOrDefault(Long value, long defaultValue) { + return value == null ? defaultValue : value; + } + private void stopExpired(String workspaceId) { try { Workspace workspace = workspaceManager.getWorkspace(workspaceId); diff --git a/wsmaster/che-core-api-workspace-activity/src/test/java/org/eclipse/che/api/workspace/activity/WorkspaceActivityManagerTest.java b/wsmaster/che-core-api-workspace-activity/src/test/java/org/eclipse/che/api/workspace/activity/WorkspaceActivityManagerTest.java index 21298b42e5..acff5d7a3e 100644 --- a/wsmaster/che-core-api-workspace-activity/src/test/java/org/eclipse/che/api/workspace/activity/WorkspaceActivityManagerTest.java +++ b/wsmaster/che-core-api-workspace-activity/src/test/java/org/eclipse/che/api/workspace/activity/WorkspaceActivityManagerTest.java @@ -28,6 +28,7 @@ import org.eclipse.che.api.core.model.workspace.WorkspaceStatus; import org.eclipse.che.api.core.notification.EventService; import org.eclipse.che.api.core.notification.EventSubscriber; import org.eclipse.che.api.workspace.server.WorkspaceManager; +import org.eclipse.che.api.workspace.server.WorkspaceRuntimes; import org.eclipse.che.api.workspace.server.event.BeforeWorkspaceRemovedEvent; import org.eclipse.che.api.workspace.server.model.impl.WorkspaceImpl; import org.eclipse.che.api.workspace.shared.Constants; @@ -51,6 +52,7 @@ public class WorkspaceActivityManagerTest { private static final long DEFAULT_TIMEOUT = 60_000L; // 1 minute @Mock private WorkspaceManager workspaceManager; + @Mock private WorkspaceRuntimes workspaceRuntimes; @Captor private ArgumentCaptor> createEventCaptor; @Captor private ArgumentCaptor> statusChangeEventCaptor; @@ -68,7 +70,11 @@ public class WorkspaceActivityManagerTest { private void setUp() throws Exception { activityManager = new WorkspaceActivityManager( - workspaceManager, workspaceActivityDao, eventService, DEFAULT_TIMEOUT); + workspaceManager, + workspaceRuntimes, + workspaceActivityDao, + eventService, + DEFAULT_TIMEOUT); lenient().when(account.getName()).thenReturn("accountName"); lenient().when(account.getId()).thenReturn("account123");