CODENVY-408: Add System RAM check #501

vinokurig · 2016-08-02T06:32:04Z

What does this PR do?

Checks whole system ram before starting workspace and if it is more than 90% of used ram workspace will not be started. When it will be less then 90% of used ram, a message will be sent by websocket.

What issues does this PR fix or reference?

#408

Tests written?

Yes

@garagatyi @evoevodin please review

voievodin · 2016-08-02T15:28:29Z

...latform-api-impl/src/main/java/com/codenvy/api/workspace/LimitsCheckingWorkspaceManager.java

+    }
+
+    private long getBytesAmountFromString(String string) {
+        if (string.contains("KiB")) {


Can you give an example of the output?
It seems that you can reuse: this class for parsing

voievodin · 2016-08-17T15:04:32Z

...latform-api-impl/src/main/java/com/codenvy/api/workspace/LimitsCheckingWorkspaceManager.java

+                startSemaphore.acquire();
+                return checkRamAndPropagateStart(config, envName, namespace, workspaceId, callback);
+            } catch (InterruptedException e) {
+                throw new ServerException("Starting workspace was interrupted", e);


it shouldn't happen normally, can you please just rethrow InterruptedException as ServerException with the same message like throw new ServerException(e.getMessage(), e)

voievodin · 2016-08-17T16:04:54Z

-1 for now.
The design and implementation of the component for ram checking must be reworked.
I didn't check all the code, but i think that i left more than enough comments for now, please check&answer where necessary.

vinokurig · 2016-08-26T11:08:05Z

@evoevodin fixed all notes, please take a look

garagatyi · 2016-09-07T08:16:53Z

...latform-api-impl/src/main/java/com/codenvy/api/workspace/LimitsCheckingWorkspaceManager.java

-                                         config.getDefaultEnv(),
-                                         namespace,
-                                         () -> super.startWorkspace(config, namespace, isTemporary, accountId));
+        return checkRamAndPropagateLimitedStart(config,


Why this method is renamed, new name doesn't add anything meaningful

It is a new method that wraps checkRamAndPropagateStart method

But here limited is not to obvious. Maybe change to limited throughput ?

garagatyi · 2016-09-08T07:09:49Z

...orm-api-impl/src/test/java/com/codenvy/api/workspace/LimitsCheckingWorkspaceManagerTest.java

+    }
+
+    @Test
+    public void shouldSetSemaphoreToNullIfThroughputPropertyIsLessThenZero() throws Exception {


Tests that check throughput limitation are also needed

added test:

codenvy/wsmaster/codenvy-hosted-platform-api-impl/src/test/java/com/codenvy/api/workspace/LimitsCheckingWorkspaceManagerTest.java

Lines 567 to 621 in 2abe1b7

@Test(timeOut = 3000)

public void shouldPermitToCheckRamOnlyForFiveThreadsAtTheSameTime() throws Exception {

when(systemRamInfoProvider.getSystemRamInfo()).thenReturn(new SystemRamInfo(0, parseSize("3 GiB")));

final LimitsCheckingWorkspaceManager manager = spy(new LimitsCheckingWorkspaceManager(2,

"3gb", // <- workspaces ram limit

"1gb",

5,

systemRamInfoProvider,

null,

null,

null,

null,

null,

environmentParser,

false,

false,

2000));

doReturn(singletonList(createRuntime("1gb", "1gb"))).when(manager).getByNamespace(anyString()); // <- currently running 2gb

/*

The count-down latch is needed to reach the throughput limit by acquiring RAM check permits.

The lath is configured to 6 invocations: 5 (number of allowed same time requests) + 1 for main thread

to be able to release the throughput limit.

*/

final CountDownLatch invokeProcessLatch = new CountDownLatch(6);

//Pause 5 threads after they will acquire all permits to check RAM.

doAnswer(invocationOnMock -> {

invokeProcessLatch.countDown();

invokeProcessLatch.await();

return null;

}).when(manager).checkRamAndPropagateStart(anyObject(), anyString(), anyString(), anyObject());

Runnable runnable = () -> {

try {

final WorkspaceCallback callback = mock(WorkspaceCallback.class);

manager.checkRamAndPropagateLimitedThroughputStart(createConfig("1gb"), null, "user123", callback);

} catch (Exception e) {

}

};

//Run 7 threads (more than number of allowed same time requests) that want to request RAM check at the same time.

ExecutorService executor = Executors.newFixedThreadPool(7);

executor.submit(runnable);

executor.submit(runnable);

executor.submit(runnable);

executor.submit(runnable);

executor.submit(runnable);

executor.submit(runnable);

executor.submit(runnable);

//Wait for throughput limit will be reached and check that RAM check was performed only in allowed number of threads.

verify(manager, timeout(300).times(5)).checkRamAndPropagateStart(anyObject(), anyString(), anyString(), anyObject());

//Execute paused threads to release the throughput limit for other threads.

invokeProcessLatch.countDown();

//Wait for throughput limit will be released and check that RAM check was performed in other threads.

verify(manager, timeout(300).times(7)).checkRamAndPropagateStart(anyObject(), anyString(), anyString(), anyObject());

}

garagatyi · 2016-09-09T09:19:53Z

LGTM

skabashnyuk · 2016-09-09T12:04:31Z

ok for me. Final decision up to @evoevodin and @garagatyi

skabashnyuk · 2016-09-09T12:04:43Z

@evoevodin wdyt?

voievodin · 2016-09-09T12:09:44Z

The general approach is okay for me.
@vinokurig it was a long discussion, go ahead and finally merge it

CODENVY-408: Add System RAM check

vinokurig mentioned this pull request Aug 2, 2016

CHE-408: Add ramUsage() method to DockerConnector eclipse-che/che#2024

Closed

voievodin reviewed Aug 2, 2016
View reviewed changes

vinokurig force-pushed the CODENVY-408 branch 2 times, most recently from 1c9d7a0 to 20d7ef2 Compare August 3, 2016 11:08

vinokurig force-pushed the CODENVY-408 branch 6 times, most recently from 3ec7961 to a1ccf0c Compare August 17, 2016 13:03

voievodin reviewed Aug 17, 2016
View reviewed changes

vinokurig force-pushed the CODENVY-408 branch 4 times, most recently from 980c95e to 143c082 Compare August 26, 2016 06:23

garagatyi reviewed Sep 7, 2016
View reviewed changes

vinokurig force-pushed the CODENVY-408 branch 2 times, most recently from 591b57f to edefd63 Compare September 7, 2016 13:07

garagatyi reviewed Sep 8, 2016
View reviewed changes

vinokurig force-pushed the CODENVY-408 branch 7 times, most recently from 99080ac to 9f8cb9e Compare September 9, 2016 09:11

vinokurig force-pushed the CODENVY-408 branch from 5171982 to 2bc0dd9 Compare September 19, 2016 11:51

CODENVY-408: Add System RAM check

835e452

vinokurig force-pushed the CODENVY-408 branch from 34e3e96 to 835e452 Compare September 27, 2016 12:51

Igor Vinokur added 4 commits September 27, 2016 16:12

merge with master

960181a

Merge branch 'master' of github.com:codenvy/codenvy into CODENVY-408

7025e93

Merge branch 'master' of github.com:codenvy/codenvy into CODENVY-408

3758f26

fixup! CODENVY-408: Add System RAM check

0a4d82e

vinokurig merged commit 1c8d378 into master Sep 27, 2016

vinokurig deleted the CODENVY-408 branch September 27, 2016 17:35

mshaposhnik pushed a commit that referenced this pull request Sep 28, 2016

CODENVY-408: Add System RAM check (#501)

d8efede

CODENVY-408: Add System RAM check

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CODENVY-408: Add System RAM check #501

CODENVY-408: Add System RAM check #501

vinokurig commented Aug 2, 2016

voievodin Aug 2, 2016

voievodin Aug 17, 2016

vinokurig Aug 25, 2016

voievodin commented Aug 17, 2016 •

edited

vinokurig commented Aug 26, 2016

garagatyi Sep 7, 2016

vinokurig Sep 7, 2016

garagatyi Sep 7, 2016

vinokurig Sep 7, 2016

garagatyi Sep 8, 2016

vinokurig Sep 9, 2016

garagatyi commented Sep 9, 2016

skabashnyuk commented Sep 9, 2016

skabashnyuk commented Sep 9, 2016

voievodin commented Sep 9, 2016

	@Test(timeOut = 3000)
	public void shouldPermitToCheckRamOnlyForFiveThreadsAtTheSameTime() throws Exception {
	when(systemRamInfoProvider.getSystemRamInfo()).thenReturn(new SystemRamInfo(0, parseSize("3 GiB")));
	final LimitsCheckingWorkspaceManager manager = spy(new LimitsCheckingWorkspaceManager(2,
	"3gb", // <- workspaces ram limit
	"1gb",
	5,
	systemRamInfoProvider,
	null,
	null,
	null,
	null,
	null,
	environmentParser,
	false,
	false,
	2000));
	doReturn(singletonList(createRuntime("1gb", "1gb"))).when(manager).getByNamespace(anyString()); // <- currently running 2gb
	/*
	The count-down latch is needed to reach the throughput limit by acquiring RAM check permits.
	The lath is configured to 6 invocations: 5 (number of allowed same time requests) + 1 for main thread
	to be able to release the throughput limit.
	*/
	final CountDownLatch invokeProcessLatch = new CountDownLatch(6);
	//Pause 5 threads after they will acquire all permits to check RAM.
	doAnswer(invocationOnMock -> {
	invokeProcessLatch.countDown();
	invokeProcessLatch.await();
	return null;
	}).when(manager).checkRamAndPropagateStart(anyObject(), anyString(), anyString(), anyObject());
	Runnable runnable = () -> {
	try {
	final WorkspaceCallback callback = mock(WorkspaceCallback.class);
	manager.checkRamAndPropagateLimitedThroughputStart(createConfig("1gb"), null, "user123", callback);
	} catch (Exception e) {
	}
	};
	//Run 7 threads (more than number of allowed same time requests) that want to request RAM check at the same time.
	ExecutorService executor = Executors.newFixedThreadPool(7);
	executor.submit(runnable);
	executor.submit(runnable);
	executor.submit(runnable);
	executor.submit(runnable);
	executor.submit(runnable);
	executor.submit(runnable);
	executor.submit(runnable);

	//Wait for throughput limit will be reached and check that RAM check was performed only in allowed number of threads.
	verify(manager, timeout(300).times(5)).checkRamAndPropagateStart(anyObject(), anyString(), anyString(), anyObject());

	//Execute paused threads to release the throughput limit for other threads.
	invokeProcessLatch.countDown();
	//Wait for throughput limit will be released and check that RAM check was performed in other threads.
	verify(manager, timeout(300).times(7)).checkRamAndPropagateStart(anyObject(), anyString(), anyString(), anyObject());
	}

CODENVY-408: Add System RAM check #501

CODENVY-408: Add System RAM check #501

Conversation

vinokurig commented Aug 2, 2016

What does this PR do?

What issues does this PR fix or reference?

Tests written?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

voievodin commented Aug 17, 2016 • edited

vinokurig commented Aug 26, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

garagatyi commented Sep 9, 2016

skabashnyuk commented Sep 9, 2016

skabashnyuk commented Sep 9, 2016

voievodin commented Sep 9, 2016

voievodin commented Aug 17, 2016 •

edited