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

net/http: TestTimeoutHandlerSuperfluousLogs is flaky #35051

Open
bcmills opened this issue Oct 21, 2019 · 6 comments

Comments

@bcmills
Copy link
Member

@bcmills bcmills commented Oct 21, 2019

The test added in CL 200518 appears to have flaked on the dragonfly-amd64 builder (https://build.golang.org/log/dbcda17a3616121b8fc8c2efc58cb42e82a6d373):

--- FAIL: TestTimeoutHandlerSuperfluousLogs (1.04s)
    --- FAIL: TestTimeoutHandlerSuperfluousLogs/return_after_timeout (1.04s)
        serve_test.go:6336: Response mismatch
            Got
            "HTTP/1.1 404 Not Found\r\nContent-Length: 0\r\n\r\n"
            
            Want
            "HTTP/1.1 503 Service Unavailable\r\nContent-Length: 15\r\n\r\ntimed out here!"
FAIL
FAIL	net/http	11.471s

I suspect that the problem relates to the use of a hard-coded timeout (here), although I haven't verified that.

CC @odeke-em @bradfitz

@bcmills bcmills added this to the Go1.14 milestone Oct 21, 2019
@odeke-em

This comment has been minimized.

Copy link
Member

@odeke-em odeke-em commented Oct 21, 2019

Thanks for the file @bcmills! Sheesh how does one then test a Timeout if 2X the requested sleep time still doesn't time out? I'll send a CL skipping that test to avoid blocking the builders.

@bcmills

This comment has been minimized.

Copy link
Member Author

@bcmills bcmills commented Oct 21, 2019

The flake rate seems low enough that a Skip probably isn't necessary at this point.

But yeah: never underestimate the slowness of builders.

(It's a corollary of the “infinite monkeys” theorem: “Infinitely many monkeys simulating TryBot runs will eventually produce an antagonistic schedule.”)

@odeke-em

This comment has been minimized.

Copy link
Member

@odeke-em odeke-em commented Oct 21, 2019

The flake rate seems low enough that a Skip probably isn't necessary at this point.

Cool, thanks! However, this would point to being a runtime problem if the timer for the goroutine that fires insider the handler, fires earlier than the timer of the overall handler. We should investigate that angle too.

But yeah: never underestimate the slowness of builders.

(It's a corollary of the “infinite monkeys” theorem: “Infinitely many monkeys simulating TryBot runs will eventually produce an antagonistic schedule.”)

Hehe, indeed!

@bcmills

This comment has been minimized.

Copy link
Member Author

@bcmills bcmills commented Oct 21, 2019

if the timer for the goroutine that fires insider the handler, fires earlier than the timer of the overall handler.

Note that with goroutine preemption (#24543), it is in general possible for the two timers to fire “in order”, but for the next statement(s) after the first timer to be preempted immediately and ultimately execute after the statements immediately after the second timer. (If the two goroutines don't have a “happens-before” edge, their execution order is unspecified regardless of wall time.)

@bcmills

This comment has been minimized.

Copy link
Member Author

@bcmills bcmills commented Nov 7, 2019

freebsd-amd64-race (https://build.golang.org/log/973f4071939084efe638fa859e6473379cdbe698):

--- FAIL: TestTimeoutHandlerSuperfluousLogs (0.55s)
    --- FAIL: TestTimeoutHandlerSuperfluousLogs/return_after_timeout (0.54s)
        serve_test.go:6338: Response mismatch
            Got
            "HTTP/1.1 404 Not Found\r\nContent-Length: 0\r\n\r\n"
            
            Want
            "HTTP/1.1 503 Service Unavailable\r\nContent-Length: 15\r\n\r\ntimed out here!"
FAIL
FAIL	net/http	14.330s
@bcmills bcmills changed the title net/http: TestTimeoutHandlerSuperfluousLogs flake on dragonfly-amd64 net/http: TestTimeoutHandlerSuperfluousLogs is flaky Nov 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.