-
Notifications
You must be signed in to change notification settings - Fork 61
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
Times should be stored as time.Time
#24
Comments
I can prepare a PR if it is agreed on, but it will most likely break the backwards compatibility. |
I think that would be a good improvement. Any opinion, @cgordon? |
That is a great idea. I've been working on a new version of bender that uses more idiomatic Go (and less memory). I have rewritten the core loop (and have been using it for a new project), but I haven't (yet) had time to rewrite the HTTP and Thrift tutorials to use it. You can take a look here: https://github.com/cgordon/bender. As I get time this week, I'll try to add some other new things, including a simple JSON output format for the timing events and some Python tools to generate useful statistics from that output. |
@cgordon since you've been working on a new version could you consider allowing user to provide their own time measure checkpoints (optional)? Setup before sending a request might take a long time and interfere the results. I've encountered that while doing simple UDP loadtests (I've stripped not important parts): func UDPExecutor(_ int64, request interface{}) (interface{}, error) {
datagram := request.(*Datagram)
// This part can take quite long and we'd like to ignore it in time measures
conn, err := net.Dial("udp", addr)
if err != nil {
return nil, err
}
defer conn.Close()
// Start measure here
conn.SetWriteDeadline(time.Now().Add(timeout))
_, err = conn.Write(datagram.Data)
if err != nil {
return nil, err
}
buffer := make([]byte, bufferSize)
conn.SetReadDeadline(time.Now().Add(timeout))
n, err := conn.Read(buffer)
if err != nil {
return nil, err
}
return buffer[:n], nil
} My "temporary fix" was to calculate it manually and return the results as a struct // Response represents a udp response received
type Response struct {
Start time.Time
End time.Time
Elapsed time.Duration
Data []byte
} But it also means that histogram and other recorders need to be adjusted for this new format. It would be really good to have a generic solution to this. |
@earlgreyz That's an interesting idea. I've been having the request generator do all the initialization for our load testing, and passing everything into the executor, but I can see how you may want to avoid that here. It looks like the easiest option to make this work would be to redefine the RequestExecutor to return four values: the response, the start time, the end time and an error. Then, rather than the main loop tracking the start and end time, the request executor would need to do so. In that case, it would also be easy to write a little wrapper function that did the timing (for people who don't need that fine control). Does that sound like it would do what you want? |
@cgordon yes, that seems like a good solution |
Duration should be measured using monotonic clocks, this can be done by using standard
time.Time
andtime.Duration
instead of storingint64
unix epoch times. https://golang.org/pkg/time/#hdr-Monotonic_ClocksThe text was updated successfully, but these errors were encountered: