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.

Sign up for GitHub

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

Jump to bottom

Print a message on server timeout #6209

Closed
wants to merge 1 commit into from
Closed

Print a message on server timeout #6209

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpDownloader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import com.google.devtools.build.skyframe.SkyFunctionException.Transience;
import java.io.IOException;
import java.io.InterruptedIOException;
import java.net.SocketTimeoutException;
import java.io.OutputStream;
import java.net.MalformedURLException;
import java.net.URL;
Expand Down Expand Up @@ -226,6 +227,14 @@ public Path download(
OutputStream out = destination.getOutputStream()) {
ByteStreams.copy(payload, out);
success = true;
} catch (SocketTimeoutException e) {
// SocketTimeoutException is a subclass of InterruptedIOException.
// If we catch a SocketTimeoutException we want to print an informative message
// like we do for the IOException case.
// This can happen in real life, when bazel is inside a network where any
// connections to the outside world are neither acknowledged nor refused.
throw new IOException(
"Error downloading " + urls + " to " + destination + ": " + e.getMessage());
} catch (InterruptedIOException e) {
throw new InterruptedException();
} catch (IOException e) {
Expand Down
16 changes: 16 additions & 0 deletions src/test/shell/bazel/external_integration_test.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -925,6 +925,22 @@ EOF
}


function test_server_timeout() {
startup_timeout_server $PWD

cat > WORKSPACE <<EOF
load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
http_archive(
name = "repo",
url = "http://127.0.0.1:$fileserver_port/some_archive.zip",
)
EOF
bazel build @repo//... &> $TEST_log 2>&1 && fail "Expected to fail"
Copy link
Contributor

Choose a reason for hiding this comment

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

How long does it wait for a timeout? I'm wondering how bad the effect on our CI is going to be...

Copy link
Author

Choose a reason for hiding this comment

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

This test takes ~3 mins.

I did time bazel build @repo//... &> $TEST_log 2>&1 && fail "Expected to fail"
in line 938 and got:

** test_server_timeout *********************************************************

real    2m56.609s
user    0m0.020s
sys     0m0.016s
PASSED: test_server_timeout

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that's a problem. Having a test wait for 3 minutes doing nothing means our CI will take 3 minutes longer. Is there any way to change the timeout with a flag or something like that?

Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any way to change the timeout with a flag or something like that?

Not at the moment. All the timeouts are hard-coded. But adding a flag to configure those timeouts seems a good idea in general; will work on this.

expect_log "Error downloading \\[http://127.0.0.1:$fileserver_port/some_archive.zip\\] to"
expect_log "Read timed out"
shutdown_server
}


function test_same_name() {
mkdir ext
Expand Down
10 changes: 10 additions & 0 deletions src/test/shell/bazel/remote_helpers.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,16 @@ function startup_auth_server() {
wait_for_server_startup
}

function startup_timeout_server() {
fileserver_port=$(pick_random_unused_tcp_port) || exit 1
# We allow only for one response from the server.
# wait_for_server_startup uses this response to make sure that the
# timeout server is ready.
python $python_server --port=$fileserver_port --max_responses=1 &
fileserver_pid=$!
wait_for_server_startup
}

function shutdown_server() {
# Try to kill nc, otherwise the test will time out if Bazel has a bug and
# didn't make a request to it.
Expand Down
15 changes: 14 additions & 1 deletion src/test/shell/bazel/testing_server.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,11 @@
import os
import SocketServer
import sys
import time

auth = None
max_responses = -1
response_counter = 0


class Handler(BaseHTTPRequestHandler):
Expand All @@ -39,6 +42,13 @@ def do_AUTHHEAD(self): # pylint: disable=invalid-name
self.end_headers()

def do_GET(self): # pylint: disable=invalid-name
global response_counter
if max_responses >= 0 and response_counter >= max_responses:
while True:
time.sleep(1)
else:
response_counter = response_counter + 1

if auth is None:
self.do_HEAD()
self.serve_file()
Expand Down Expand Up @@ -68,7 +78,8 @@ def serve_file(self):

if __name__ == '__main__':
try:
opts, args = getopt.getopt(sys.argv[1:], 'p:a:', ['port=', 'auth='])
opts, args = getopt.getopt(
sys.argv[1:], 'p:a:m:', ['port=', 'auth=', 'max_responses='])
except getopt.GetoptError:
print 'Error parsing args'
sys.exit(1)
Expand All @@ -79,6 +90,8 @@ def serve_file(self):
port = int(a)
if o in ('-a', '--auth'):
auth = a
if o in ('-m', '--max_responses'):
max_responses = int(a)
httpd = SocketServer.TCPServer(('', port), Handler)
try:
print 'Serving forever on %d.' % port
Expand Down