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

Feature/add disable keep alives #47

Conversation

fulldump
Copy link

@fulldump fulldump commented Mar 4, 2019

feat: Add parameter to disable HTTP keep alives (-a and --disableKeepAlives)

* It is only compatible with --http1 and --http2 (not with fasthttp)
* Add `disableKeepAlives` (boolean) to args_parser.go and config.go
* Set parameter to clientOpts (bombardier.go)
* Configure http client transport with `DisableKeepAlives` (clients.go)

codesenberg and others added 3 commits June 29, 2018 21:46
Bump commit date for v1.2 tag, so that people don't get confused
about which version is latest as seen in codesenberg#36.
…Alives)

* It is only compatible with --http1 and --http2 (not with fasthttp)
* Add `disableKeepAlives` (boolean) to args_parser.go and config.go
* Set parameter to clientOpts (bombardier.go)
* Configure http client transport with `DisableKeepAlives` (clients.go)
…sable-keep-alives

# Conflicts:
#	args_parser.go
@codesenberg
Copy link
Owner

Thanks, @fulldump. Very cool.
I suggest that you rebase your commits on top the most recent commit in release-1.2 branch and then we can start working on it, ok?

@codesenberg codesenberg added this to the Unplanned milestone Mar 1, 2020
@@ -108,6 +109,10 @@ func newKingpinParser() argsParser {
" chain and host name").
Short('k').
BoolVar(&kparser.insecure)
app.Flag("disableKeepAlives",
"Disable HTTP KeepAlive").
Copy link
Owner

Choose a reason for hiding this comment

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

I think it should also mention that for fasthttp a header needs to be set. See here.
Something like "Disable HTTP keep-alive. For fasthttp also set -H 'Connection: close'".

@codesenberg
Copy link
Owner

Shame this PR was stale for more than a year and small tweaks preventing it from being merged weren't resolved, but I added this functionality anyway. Wish there was some way to give you credit, but thanks for PR anyways 👍

@codesenberg codesenberg closed this Oct 4, 2020
@fulldump
Copy link
Author

Shame this PR was stale for more than a year and small tweaks preventing it from being merged weren't resolved, but I added this functionality anyway. Wish there was some way to give you credit, but thanks for PR anyways 👍

No worries at all, it is my bad. Btw, I still use bombardier whenever I need to test servers performance, it is a great tool!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants