Separate call rate increase to a dedicated task #107

bklang · 2014-10-06T04:56:40Z

For several load tests, I want to have finer-grained statistics coming from SIPp, so I have the -fd option set to 1s. However, this means that if I want to use the automatic rate increases, I'm stuck with either coarser stats, or a less-useful ramp-up time.

This PR is an attempt to move call rate increases into a separate task.

Note: I have not yet been able to test this because I'm having trouble with the autotools. I'll put those details into a comment.

bklang · 2014-10-06T04:58:48Z

I'm unable to test this because of this error: https://gist.github.com/bklang/b9bf75dbd48991b8b5b0

Specifically, the bottom two lines of that gist:

./configure: line 7781: syntax error near unexpected token `AX_CONFIG_FEATURE_ENABLE'
./configure: line 7781: `AX_HAVE_EPOLL(AX_CONFIG_FEATURE_ENABLE(epoll),'

If someone can help me figure out this problem I'll test and complete this PR.

wdoekes · 2014-10-06T06:52:17Z

That means you used the wrong version of automake/aclocal. You can fetch any version from the gnu site. They're supersmall to install. We use 1.13, IIRC.

bklang · 2014-10-06T12:37:07Z

Thanks @wdoekes. I've tried that, but I'm still getting the same error:

$ AUTOMAKE=/usr/local/bin/automake-1.13 ACLOCAL=/usr/local/bin/aclocal-1.13 sh auto-generate-files.sh
autoreconf: Entering directory `.'
autoreconf: configure.ac: not using Gettext
autoreconf: running: /usr/local/bin/aclocal-1.13 --force
autoreconf: configure.ac: tracing
autoreconf: configure.ac: not using Libtool
autoreconf: running: /opt/local/bin/autoconf --force
autoreconf: running: /opt/local/bin/autoheader --force
autoreconf: running: /usr/local/bin/automake-1.13 --add-missing --copy --force-missing
autoreconf: Leaving directory `.'
help2man: can't get `--help' info from ./sipp
Try `--no-discard-stderr' if option outputs to stderr

$ sh build.sh --with-pcap --with-rtpstream --with-openssl
# <snip configure output...>
checking for pcap.h... yes
checking for library containing pcap_open_offline... -lpcap
checking for library containing pcap_next... none required
checking for library containing pcap_next_ex... none required
checking for library containing pcap_close... none required
./configure: line 7681: syntax error near unexpected token `AX_CONFIG_FEATURE_ENABLE'
./configure: line 7681: `AX_HAVE_EPOLL(AX_CONFIG_FEATURE_ENABLE(epoll),'

bklang · 2014-10-06T15:57:55Z

Ok with the final commit above, I was able to test that this is working now. So the code is ready to be merged, but the build system needs to be updated. I got it working by reverting the changes to ./configure but leaving the other changed files.

I'm still not sure what's wrong with autoconf/aclocal, but I can't get it to produce valid files. Could someone else update them to include the new ratetask file? That's the only thing left as far as I can see. Maybe @rkday?

bklang · 2014-10-14T23:17:34Z

@rkday @wdoekes I've re-run autoreconf and extracted the only changes required to fix the build with the new ratetask files. It doesn't solve the problem that I can't run the autoreconf tool (which was broken, at least for me, before this PR), but it does allow sipp to build successfully. I believe that makes this PR complete and ready to merge. Would you mind reviewing and letting me know if you need anything further?

wdoekes · 2014-10-15T15:39:39Z

src/ratetask.cpp

+        instance = new ratetask();
+    }
+}
+void ratetask::dump()


Please add linefeed between functions.

wdoekes · 2014-10-15T15:42:06Z

Looks good to me. Can you fix those tiny coding style issues and squash all this into a single commit?

bklang · 2014-10-15T16:34:47Z

Absolutely, thanks for the review. Nits fixed.

bklang · 2014-10-15T16:35:40Z

...and squashed.

benlangfeld · 2014-11-05T17:17:22Z

So, @bklang asked for my opinion on this, so here it is:

So, this seems like a good opportunity to prompt for a discussion of SIPp versioning / release policy. There's no changelog, but we should fix that. Does the project intend to follow a documented versioning policy such as SemVer? It seems like without that, these decisions on what can and can't change are made arbitrarily, and we should instead have a written statement on how those decisions get made.

In this specific case, I understand the desire to avoid breaking scripts written to exploit this bug, but at the same time I think it falls distinctly into the category of "bug" in so much as it's a design flaw. Was the buggy behaviour ever documented? If not, then users have no formal basis on which to exploit it, and they're relying on their own experience. This is not a strong defence for including code specifically to recreate a bug.

rkday · 2014-11-05T23:38:36Z

Yes, this behaviour is documented (and this pull request in fact changes the documentation for it - https://github.com/SIPp/sipp/pull/107/files#diff-d4a20109022fdf77b05548a93daf8fedL276). The use of -fd isn't unintentional or a bug - the idea behind linking -rate_increase and -fd is that you'll run load at X level for n seconds, dump out the results of that stage of testing, then run load at X+1 level for n seconds, dump out the results of that stage of testing, and so on. It doesn't design for every use-case, but it's not a bug as such.

Changelogs, SemVer etc. are all valid topics to discuss - but I don't think they're quite relevant to this thread, because whether you have changelogs or a particular versioning scheme wouldn't change the fact that if possible, "-rate_increase 10 -fd 60" should work the same between releases - they just address the question of how to document it when you have to break backwards compatibility. But as far as I can tell - though I'm happy to be corrected if wrong - that's not the case here.

benlangfeld · 2014-11-05T23:41:22Z

Ah, then if this behaviour is intentional, the point of my comment is void and I retract it. Thanks for clearing that up.

kvishnivetsky · 2014-11-30T16:21:59Z

Hellow, Guys.
I'v the issue with:
./configure: line 10602: syntax error near unexpected token AX_CONFIG_FEATURE_ENABLE' ./configure: line 10602:AX_HAVE_EPOLL(AX_CONFIG_FEATURE_ENABLE(epoll),'
trying to build on CentOS 6.5 fully updated.
automake v1.11.1, installed with YUM from standard system repo
Is it possible to make this check(EPOLL) in more back-compatible way or just do it on those OSes, which support this macro?

wdoekes · 2014-11-30T16:45:10Z

@kvishnivetsky: I believe the AX_HAVE_EPOLL issue may have been fixed in master 6f03211.

kvishnivetsky · 2014-11-30T17:06:16Z

Oh! I see now, thanks!

kvishnivetsky · 2014-11-30T18:03:00Z

Thanks, @wdoekes for your advice, but code from master branch fails on autoreconf -if stage with error:

autoreconf -if
configure.ac:6: error: possibly undefined macro: AC_CONFIG_MACRO_DIRS
If this token and others are legitimate, please use m4_pattern_allow.
See the Autoconf documentation.
autoreconf: /usr/bin/autoconf failed with exit status: 1

kvishnivetsky · 2014-11-30T18:44:53Z

I have a solution:
m4_include([m4/ax_config_feature.m4])
m4_include([m4/ax_have_epoll.m4])

Use m4_include instead of AC_CONFIG_MACRO_DIRS([m4])

wdoekes · 2014-11-30T20:46:09Z

B.t.w. you could just run build.sh which would touch the files so configure wouldn't attempt to run autoreconf.

benlangfeld · 2015-03-16T18:44:31Z

Continued at #126.

Via the new `calls_per_second_interval` option. See also SIPp/sipp#107 and SIPp/sipp#126.

bklang mentioned this pull request Oct 6, 2014

Feature: Linear cps increase adhearsion/SIPtreadmill#15

Merged

bklang mentioned this pull request Oct 7, 2014

Support for new -rate_interval option mojolingo/sippy_cup#71

Merged

wdoekes reviewed Oct 15, 2014
View reviewed changes

src/ratetask.cpp

instance = new ratetask();

}

}

void ratetask::dump()

Copy link

Member

wdoekes Oct 15, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add linefeed between functions.

Separate call rate increase to a dedicated task

8cba7d1

bklang force-pushed the master branch from 4691c6c to 8cba7d1 Compare October 15, 2014 16:35

benlangfeld mentioned this pull request Mar 16, 2015

Smooth rampup #126

Merged

bklang closed this Mar 17, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Separate call rate increase to a dedicated task #107

Separate call rate increase to a dedicated task #107

bklang commented Oct 6, 2014

bklang commented Oct 6, 2014

wdoekes commented Oct 6, 2014

bklang commented Oct 6, 2014

bklang commented Oct 6, 2014

bklang commented Oct 14, 2014

wdoekes Oct 15, 2014

wdoekes commented Oct 15, 2014

bklang commented Oct 15, 2014

bklang commented Oct 15, 2014

benlangfeld commented Nov 5, 2014

rkday commented Nov 5, 2014

benlangfeld commented Nov 5, 2014

kvishnivetsky commented Nov 30, 2014

wdoekes commented Nov 30, 2014

kvishnivetsky commented Nov 30, 2014

kvishnivetsky commented Nov 30, 2014

kvishnivetsky commented Nov 30, 2014

wdoekes commented Nov 30, 2014

benlangfeld commented Mar 16, 2015

Separate call rate increase to a dedicated task #107

Separate call rate increase to a dedicated task #107

Conversation

bklang commented Oct 6, 2014

bklang commented Oct 6, 2014

wdoekes commented Oct 6, 2014

bklang commented Oct 6, 2014

bklang commented Oct 6, 2014

bklang commented Oct 14, 2014

wdoekes Oct 15, 2014

Choose a reason for hiding this comment

wdoekes commented Oct 15, 2014

bklang commented Oct 15, 2014

bklang commented Oct 15, 2014

benlangfeld commented Nov 5, 2014

rkday commented Nov 5, 2014

benlangfeld commented Nov 5, 2014

kvishnivetsky commented Nov 30, 2014

wdoekes commented Nov 30, 2014

kvishnivetsky commented Nov 30, 2014

kvishnivetsky commented Nov 30, 2014

kvishnivetsky commented Nov 30, 2014

wdoekes commented Nov 30, 2014

benlangfeld commented Mar 16, 2015