Make snapshots binaries removal asynchronous #3405
Conversation
| @@ -401,7 +402,9 @@ protected void onMessageReceived(WorkspaceStatusEvent statusEvent) { | |||
| break; | |||
|
|
|||
| case RUNNING: | |||
| onWorkspaceStarted(workspaceId); | |||
| if (statusEvent.getPrevStatus() == WorkspaceStatus.STARTING) { | |||
There was a problem hiding this comment.
RUNNING may be received twice, e.g.
1. STOPPED -> STARTING -> RUNNING
2. RUNNING -> SNAPSHOTTING -> RUNNING
onWorkspaceStarted(workspaceId) obviously related to the 1st case
| @@ -825,6 +710,14 @@ void checkWorkspaceIsRunning(WorkspaceImpl workspace, String operation) throws C | |||
| } | |||
| } | |||
|
|
|||
| private void removeTmpWorkspace(Workspace workspace) { | |||
There was a problem hiding this comment.
removeTmpWorkspaceQuietly?
|
Build # 1337 - FAILED Please check console output at https://ci.codenvycorp.com/job/che-pullrequests-build/1337/ to view the results. |
| @@ -59,6 +60,7 @@ public WorkspaceImpl create(WorkspaceImpl workspace) throws ConflictException, S | |||
| requireNonNull(workspace, "Required non-null workspace"); | |||
| try { | |||
| doCreate(workspace); | |||
| eventService.publish(new WorkspaceCreatedEvent(workspace)); | |||
| } catch (DuplicateKeyException dkEx) { | |||
There was a problem hiding this comment.
Are you sure we should do that in DAO? In that case it will reach subscribers even before manager ends execution of method that called DAO.
| public void snapshot(String workspaceId) throws NotFoundException, | ||
| ConflictException, | ||
| ServerException { | ||
| try (@SuppressWarnings("unused") CloseableLock l = locks.acquireWriteLock(workspaceId)) { |
There was a problem hiding this comment.
I didn't know how to suppress that, thanks for tip!
| LOG.info("Shutdown running workspaces, workspaces to shutdown '{}'", idsToStop.size()); | ||
| ExecutorService executor = | ||
| Executors.newFixedThreadPool(2 * Runtime.getRuntime().availableProcessors(), | ||
| new ThreadFactoryBuilder().setNameFormat("StopEnvironmentsPool-%d") |
There was a problem hiding this comment.
Exceptions won't occur, task logs all the exceptiions see below.
| verify(runtimes).publishWorkspaceEvent(EventType.ERROR, | ||
| workspace.getId(), | ||
| e.getLocalizedMessage()); | ||
| DtoFactory.newDto(WorkspaceStatusEvent.class) |
There was a problem hiding this comment.
You probably forgot verification here
voievodin
force-pushed
the
fix_status_publishing
branch
from
da5e733
to
28914da
Compare
|
Build success. https://ci.codenvycorp.com/job/che-pullrequests-build/1340/ |
voievodin
force-pushed
the
fix_status_publishing
branch
from
28914da
to
b495d79
Compare
|
Build success. https://ci.codenvycorp.com/job/che-pullrequests-build/1342/ |
| @@ -633,8 +614,11 @@ void performAsyncStart(WorkspaceImpl workspace, | |||
| workspace.getAttributes().put(UPDATED_ATTRIBUTE_NAME, Long.toString(currentTimeMillis())); | |||
| workspaceDao.update(workspace); | |||
| final String env = firstNonNull(envName, workspace.getConfig().getDefaultEnv()); | |||
|
|
|||
| // barrier, safely doesn't allow to start the workspace twice | |||
There was a problem hiding this comment.
Can you explain about which barrier here is described?
| private final StripedLocks stripedLocks; | ||
| private final CheEnvironmentEngine environmentEngine; | ||
| private final Map<String, WorkspaceState> workspaces; | ||
| private final EventService eventsService; |
There was a problem hiding this comment.
Why it is better to name variable as eventsService here?
voievodin
force-pushed
the
fix_status_publishing
branch
2 times, most recently
from
7741032
to
92a90c2
Compare
benoitf
changed the title
voievodin
force-pushed
the
fix_status_publishing
branch
from
92a90c2
to
cb45fa1
Compare
| @Nullable Boolean restore) throws NotFoundException, | ||
| ServerException, | ||
| ConflictException { | ||
| @Nullable String envName, |
voievodin
force-pushed
the
fix_status_publishing
branch
from
cb45fa1
to
c9ec4b6
Compare
|
Build success. https://ci.codenvycorp.com/job/che-pullrequests-build/1351/ |
What does this PR do?
WorkspaceStatusEvent.RUNNINGafter workspace changes status fromSNAPSHOTTINGback toRUNNINGWorkspaceStatusEventwith previous workspace status fieldWhat issues does this PR fix or reference?
Resolves #3314 & resolves #3258.