Metrics: PSS memory for a particular load level #891

GabyCT · 2017-05-08T16:21:05Z

Proportional Set Size (PSS) memory while running a nuttcp 1 Gb transfer rate.
The PSS measurement is taken by using smem.

Signed-off-by: Gabriela Cervantes [email protected]

chavafg · 2017-05-08T17:48:33Z

qa-failed

chavafg · 2017-05-08T21:10:32Z

There was a swarm failure not part of cc-oci-runtime.

not ok 30 Checking MTU values in different interfaces
# (from function `setup' in test file /home/cloud/go/src/github.com/01org/cc-oci-runtime/cc-oci-runtime-2.1.7/tests/integration/docker/mtu.bats, line 43)
#   `$DOCKER_EXE swarm init ${swarm_interface_arg}' failed
# Error response from daemon: This node is not a swarm manager. Use "docker swarm init" or "docker swarm join" to connect this node to swarm and try again.
# Error response from daemon: error updating cluster settings: rpc error: code = 2 desc = update out of sequence
# Error response from daemon: service testswarm not found
# ID                           HOSTNAME                     STATUS  AVAILABILITY  MANAGER STATUS
# 91zpee0q04dqbwvnp6dwg6kv6 *  fedora-cc-ci-vm.localdomain  Ready   Active        Leader
# Node left the swarm.
# Error response from daemon: This node is not a swarm manager. Use "docker swarm init" or "docker swarm join" to connect this node to swarm and try again.

Searched a little bit about this and found this issue:
moby/moby#30794

chavafg · 2017-05-08T21:37:58Z

qa-passed

jodh-intel · 2017-05-09T07:43:39Z

tests/metrics/network/network-metrics-memory-1g.sh.in

+# Currently default nuttcp has a bug
+# see https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=745051
+# Image name
+image=gabyct/nuttcp


We still don't have the Dockerfile for this image in the repository. I've raised #892 to track this as the image is already being used by another test.

This use of custom image is a concern because we cannot recreate it until we have the Dockerfile.

In my opinion, we should use standard images where available, but if not, we must submit the Dockerfile on the same PR as any new tests that use it.

Please can you ensure #892 and the very similar #848 are resolved (it should be just a matter of raising a PR with a single Dockerfile for each issue I think?)

@jodh-intel here it is the Dockerfile #895

@jodh-intel in that Dockerfile we have the dependencies for gabyct/network and gabyct/nuttcp something that I will rename after it is merged

jodh-intel · 2017-05-09T07:44:30Z

tests/metrics/network/network-metrics-memory-1g.sh.in

+# see https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=745051
+# Image name
+image=gabyct/nuttcp
+# This is required in order to reduce standard deviation


What unit is this time value in (seconds)? What sort of SD are we seeing?

jodh-intel · 2017-05-09T07:49:07Z

tests/metrics/network/network-metrics-memory-1g.sh.in

+
+	server_command="/root/nuttcp -S"
+	$DOCKER_EXE run -tid --name=${server_name} ${image} bash > /dev/null
+        server_address=$($DOCKER_EXE inspect --format "{{.NetworkSettings.IPAddress}}" ${server_name})


We seem to be getting quite a collection of these tests now (good), but they all "look" very similar. I wonder if there is any opportunity to refactor them into some shared functions (which could live in test-common.bash) then the tests themselves could become small stubs that call the shared functions. What do you think?

@grahamwhaley - do you have any thoughts on this?

@jodh-intel @GabyCT Indeed, pretty much all the network tests look almost identical.
I agree that we should be able to refactor a bunch of this code out, with all the benefits that should bring.
I don't think we need to block this PR due to that, but @GabyCT I will make a request - for the next PR, rather than a copy/modify can we do a factor-out of the common code parts into the common library and then backport. I'm happy for that to happen as a pair of PRs - one to add a new test and the common parts and then a follow up one to backport using that common code to the existing tests for instance.

grahamwhaley · 2017-05-10T15:32:59Z

tests/metrics/network/network-metrics-memory-1g.sh.in

+#
+# Description:
+#  Measures Proportional Set Size memory while running an
+#  inter (docker<->docker) 1Gbps network bandwidth using nuttcp


Presuming we chose 1Gbps here for a reason, can we add some explanation into this comment please?

grahamwhaley · 2017-05-10T15:35:07Z

tests/metrics/network/network-metrics-memory-1g.sh.in

+image=gabyct/nuttcp
+# This is required in order to reduce standard deviation
+total_time=6
+# This time (seconds) is required when


Can I suggest a comment rewrite here. Maybe something like:
'We wait for the test system to settle into a steady mode before we measure the PSS. Thus, we have two times - the length of time the test runs for, and the time after which we sample the PSS`

grahamwhaley · 2017-05-10T15:36:10Z

tests/metrics/network/network-metrics-memory-1g.sh.in

+middle_time=3
+
+# Rate limit (speed at which transmitter send data, megabytes)
+rate_limit=10000


Can you explain how this 10,000 translates to the '1Gbps' - I'm not sure I can figure it out?

grahamwhaley · 2017-05-10T15:37:55Z

tests/metrics/network/network-metrics-memory-1g.sh.in

+	$DOCKER_EXE run -tid --name=${server_name} ${image} bash > /dev/null
+        server_address=$($DOCKER_EXE inspect --format "{{.NetworkSettings.IPAddress}}" ${server_name})
+
+        client_command="/root/nuttcp -R${rate_limit}m -T${total_time} ${server_address}"


The diff here shows we have some mixed whitespace - just a couple of lines with spaces instead of tabs - can you fix those up for us please (as I suspect we may get an update submitted anyhow).

grahamwhaley · 2017-05-10T15:39:42Z

tests/metrics/network/network-metrics-memory-1g.sh.in

+        server_address=$($DOCKER_EXE inspect --format "{{.NetworkSettings.IPAddress}}" ${server_name})
+
+        client_command="/root/nuttcp -R${rate_limit}m -T${total_time} ${server_address}"
+	$DOCKER_EXE exec ${server_name} bash -c "${server_command}"


Would this server exec command line be better placed up with the rest of the server bits a few lines up, rather than mixed here amongst the client commands. I know you may be sequencing the run/exec here - but I think this can be placed higher up for better semantic context without impacting the test itself - you think?

grahamwhaley · 2017-05-10T15:41:59Z

tests/metrics/network/network-metrics-memory-1g.sh.in

+	echo >&2 "WARNING: sleeping for $middle_time seconds in order to have server and client stable"
+	sleep ${middle_time}
+
+	${memory_command} -P @QEMU_PATH@ | tail -n 2 > "$total_memory"


I think if you add the --no-header here (as used at this link), you can drop the head maybe:
https://github.com/01org/cc-oci-runtime/blob/master/tests/metrics/density/docker_memory_usage.sh.in#L70
Also, I'm not convinced about the split of the smem options between this line and the pss_memory() function setting up the memory_command variable - it feels a bit 'fragmented', particularly as there is only a single use in the file. If you can think of a way to make that cleaner, please do.

grahamwhaley · 2017-05-10T15:49:41Z

Some comments left for some cleanup and stylistic. Fundamentally the test runs and looks to produce pretty stable results.

GabyCT · 2017-05-15T15:34:25Z

@jodh-intel and @grahamwhaley changes were applied, thanks for the feedback

chavafg · 2017-05-15T16:19:23Z

qa-passed

jodh-intel · 2017-05-15T16:20:50Z

lgtm

I think we might have a bit of a wait on landing this though as we need @grahamwhaley's ack and he's not about atm.

grahamwhaley · 2017-05-17T14:49:29Z

tests/metrics/network/network-metrics-memory-1g.sh.in

+
+# Rate limit (speed at which transmitter send data, megabytes)
+# We will measure PSS with a specific transfer rate
+rate_limit=10000


Hi @GabyCT Thanks for the fixups. I'm still not sure if this 10,000 rate_limit equates to 1Gb(/s) - if it were 1000, then that would make sense (as we are measuring megabytes), or if we were aiming for 1Gbit, and this were 100 Mbytes (taking 1byte on the wire as ~10 bits....), I could understand. So, either I think we may have a typo here (10,000 instead of 1000?), or can we have some more explanation or correction of the text - just so the 1Gb and the 10,000 correlate? :-) Thanks.

GabyCT · 2017-05-17T18:41:59Z

@grahamwhaley thank you for the review, yeah it was a typo, changes applied

Proportional Set Size (PSS) memory while running a nuttcp 1 Gb transfer rate. The PSS measurement is taken by using smem. Signed-off-by: Gabriela Cervantes <[email protected]>

chavafg · 2017-05-17T19:43:31Z

qa-passed

jodh-intel · 2017-05-18T08:12:23Z

lgtm

jodh-intel · 2017-05-31T07:52:35Z

Hi @grahamwhaley - please can you re-review so we can get this landed?

grahamwhaley · 2017-05-31T10:15:31Z

lgtm
but now there is a merge conflict on Makefile.am - @GabyCT , can you do the necessary rebase, and then we can merge!

GabyCT · 2017-06-01T17:20:09Z

@jodh-intel and @grahamwhaley, I had some issues doing the rebase :( besides I noticed that this script was outdated as it was not using the common script like the other ones so I submitted this #939 that is basically the same measurement but now without conflicts in the Makefile and using the common script of the networking tests, sorry for the inconvenience.

chavafg · 2017-06-01T17:35:28Z

qa-passed

GabyCT added the in progress label May 8, 2017

jodh-intel reviewed May 9, 2017

View reviewed changes

grahamwhaley suggested changes May 10, 2017

View reviewed changes

GabyCT force-pushed the topic/1g branch from 836917d to 18d56be Compare May 15, 2017 15:32

GabyCT mentioned this pull request May 15, 2017

Factor-out of the common code parts in networking tests #908

Closed

grahamwhaley reviewed May 17, 2017

View reviewed changes

GabyCT force-pushed the topic/1g branch from 18d56be to 3b61545 Compare May 17, 2017 18:41

Metrics: PSS memory for a particular load level

3b61545

Proportional Set Size (PSS) memory while running a nuttcp 1 Gb transfer rate. The PSS measurement is taken by using smem. Signed-off-by: Gabriela Cervantes <[email protected]>

grahamwhaley approved these changes May 31, 2017

View reviewed changes

Merge branch 'master' into topic/1g

44f622b

GabyCT closed this Jun 1, 2017

GabyCT removed the in progress label Jun 1, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Metrics: PSS memory for a particular load level #891

Metrics: PSS memory for a particular load level #891

GabyCT commented May 8, 2017 •

edited

Loading

chavafg commented May 8, 2017 •

edited by dlespiau

Loading

chavafg commented May 8, 2017

chavafg commented May 8, 2017 •

edited by dlespiau

Loading

jodh-intel May 9, 2017

GabyCT May 9, 2017

GabyCT May 9, 2017

jodh-intel May 9, 2017

jodh-intel May 9, 2017

grahamwhaley May 10, 2017

grahamwhaley May 10, 2017

grahamwhaley May 10, 2017

grahamwhaley May 10, 2017

grahamwhaley May 10, 2017

grahamwhaley May 10, 2017

grahamwhaley May 10, 2017

grahamwhaley commented May 10, 2017

GabyCT commented May 15, 2017

chavafg commented May 15, 2017

jodh-intel commented May 15, 2017 •

edited by dlespiau

Loading

grahamwhaley May 17, 2017

GabyCT commented May 17, 2017

chavafg commented May 17, 2017

jodh-intel commented May 18, 2017

jodh-intel commented May 31, 2017

grahamwhaley commented May 31, 2017

GabyCT commented Jun 1, 2017

chavafg commented Jun 1, 2017

Metrics: PSS memory for a particular load level #891

Metrics: PSS memory for a particular load level #891

Conversation

GabyCT commented May 8, 2017 • edited Loading

chavafg commented May 8, 2017 • edited by dlespiau Loading

chavafg commented May 8, 2017

chavafg commented May 8, 2017 • edited by dlespiau Loading

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

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

grahamwhaley commented May 10, 2017

GabyCT commented May 15, 2017

chavafg commented May 15, 2017

jodh-intel commented May 15, 2017 • edited by dlespiau Loading

Choose a reason for hiding this comment

GabyCT commented May 17, 2017

chavafg commented May 17, 2017

jodh-intel commented May 18, 2017

jodh-intel commented May 31, 2017

grahamwhaley commented May 31, 2017

GabyCT commented Jun 1, 2017

chavafg commented Jun 1, 2017

GabyCT commented May 8, 2017 •

edited

Loading

chavafg commented May 8, 2017 •

edited by dlespiau

Loading

chavafg commented May 8, 2017 •

edited by dlespiau

Loading

jodh-intel commented May 15, 2017 •

edited by dlespiau

Loading