Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Put a 900-sec timeout on the perf test. #5364

Merged
merged 3 commits into from
Feb 23, 2016
Merged

Conversation

vjpai
Copy link
Member

@vjpai vjpai commented Feb 23, 2016

No description provided.

@grpc-kokoro
Copy link

Can one of the admins verify this patch?

1 similar comment
@grpc-kokoro
Copy link

Can one of the admins verify this patch?

@jtattermusch
Copy link
Contributor

this is ok to test

@jtattermusch
Copy link
Contributor

LGTM, fine to merge once performance tests are green.

One observation I'd like to note is that it looks like the shell script will get out of hand fairly quickly and we might need to switch to something a bit more manageable in the future (e.g. some python script to drive this)

jtattermusch added a commit that referenced this pull request Feb 23, 2016
Put a 900-sec timeout on the perf test.
@jtattermusch jtattermusch merged commit 11a915f into grpc:master Feb 23, 2016
@vjpai vjpai deleted the timeout branch February 23, 2016 17:53
@pgrosu
Copy link

pgrosu commented Feb 23, 2016

It passes, but 900sec is 15min which could be a while for later tests. For instance, if we look at the pattern of build times over time, with the following script:

$ cat perf-tests.sh
#!/bin/bash
testtimes=" "
for ((t=500;t<=700;t++));
do
  testtime=$(curl --silent https://grpc-testing.appspot.com/job/gRPC_performance_pull_requests/$t/ | grep Took | cut -f2 -d"<" | cut -f2 -d">")
  if [ $(expr "$testtime" : '.*') -gt 1 ]; then
    echo "BUILD "$t" -> "$testtime
  fi
done
$
$ ./perf-tests.sh
BUILD 500 -> 2 min 36 sec
BUILD 501 -> 2 min 23 sec
BUILD 502 -> 2 min 36 sec
...

If I look at the average, it's 2 min 22 sec. If I plot the trend, it seems to expand quite dramatically lately by almost 4x than the average.

build_time_vs_build

It might be good to keep this in mind, and maybe optimize around it so that it doesn't become a bottleneck later on.

~p

@pgrosu
Copy link

pgrosu commented Feb 23, 2016

I guess that's what Jan was suggesting too :)

@lock lock bot locked as resolved and limited conversation to collaborators Jan 28, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants