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

cli: cockroach quit does not effectively quit #22536

Closed
cockroach-teamcity opened this issue Feb 9, 2018 · 4 comments
Closed

cli: cockroach quit does not effectively quit #22536

cockroach-teamcity opened this issue Feb 9, 2018 · 4 comments
Assignees
Labels
C-test-failure Broken test (automatically or manually discovered). O-robot Originated from a bot.

Comments

@cockroach-teamcity
Copy link
Member

The following tests appear to have failed:

#513287:

--- FAIL: acceptance/TestDockerCLI (172.770s)
test_log_scope.go:81: test logs captured to: /home/agent/work/.go/src/github.com/cockroachdb/cockroach/artifacts/acceptance/logTestDockerCLI666789360
test_log_scope.go:62: use -show-logs to present logs inline
--- FAIL: acceptance/TestDockerCLI: TestDockerCLI/test_missing_log_output.tcl (31.760s)
None
--- FAIL: acceptance/TestDockerCLI (172.770s)
test_log_scope.go:81: test logs captured to: /home/agent/work/.go/src/github.com/cockroachdb/cockroach/artifacts/acceptance/logTestDockerCLI666789360
test_log_scope.go:62: use -show-logs to present logs inline
--- FAIL: acceptance/TestDockerCLI: TestDockerCLI/test_missing_log_output.tcl (31.760s)
None

Please assign, take a look and update the issue accordingly.

@cockroach-teamcity cockroach-teamcity added O-robot Originated from a bot. C-test-failure Broken test (automatically or manually discovered). labels Feb 9, 2018
@knz knz assigned bdarnell and knz Feb 10, 2018
@knz
Copy link
Contributor

knz commented Feb 10, 2018

The error is that cockroach quit fails to actually terminates the server process.

FWIW the cockroach quit command says:

E180210 21:41:51.762099 22 rpc/context.go:516  removing connection to localhost:26257 due to error: redialing

@knz
Copy link
Contributor

knz commented Feb 10, 2018

I'm going to temporarily disable the error on quit until this issue is fixed

@knz knz changed the title teamcity: failed tests on master: acceptance/TestDockerCLI, acceptance/TestDockerCLI: TestDockerCLI/test_missing_log_output.tcl, acceptance/TestDockerCLI, acceptance/TestDockerCLI: TestDockerCLI/test_missing_log_output.tcl cli: cockroach quit does not effectively quit Feb 10, 2018
knz added a commit to knz/cockroach that referenced this issue Feb 10, 2018
- server: fix TestReportUsage (broken by cockroachdb#20790)
- cli/interactive_tests: do not check for proper termination
  by `cockroach quit` until cockroachdb#22536 is fixed.

Release note: none
@knz
Copy link
Contributor

knz commented Feb 10, 2018

@bdarnell the problem disappears if I rewind beyond #22518 merging.

benesch added a commit to benesch/cockroach that referenced this issue Feb 12, 2018
These are broken because quit is broken (cockroachdb#22536).

Release note: None
@bdarnell
Copy link
Contributor

The error is that cockroach quit fails to actually terminates the server process.

Are you sure? I can't tell from the logs whether the server stopped or not. The immediate failure is here:

expect: does "echo marker; /cockroach/cockroach quit 2>&1 | grep -vE '^[IWEF][0-9]+ .+ vend\rdor/google.golang.org/grpc/' | grep -vE '^(ok|Error)'\r\nmarker\r\nE180209 02:37:53.344521 50 rpc/context.go:516  removing connection to localhost:26257 due to error: redialing\r\n:/# " (spawn_id exp3) match glob pattern "marker\r\n:/# "? no
expect: timed out
:/# 
.180209 02:38:13.348859079 EXPECT TEST: TIMEOUT WAITING FOR "marker
:/# "

An error is logged, and the test is trying to verify that no errors are logged (but the way the test is written this turns into a "timeout"). This could just be an issue with the error whitelist in doShutdown (we don't log grpc connection closed errors, but #22518 introduced a new error type). I'm still not entirely sure why we're seeing that error here, though - I don't think quit should be trying to open multiple connections.

bdarnell added a commit to bdarnell/cockroach that referenced this issue Feb 13, 2018
The only difference from the previous version of this change is that
the new error has been moved from pkg/rpc to pkg/util/grpcutil,
and is included in IsClosedConnection. This suppresses the log message
that was causing intermittent failures in test_missing_log_output.tcl

Fixes cockroachdb#22536

This reverts commit 9b20d8a.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-test-failure Broken test (automatically or manually discovered). O-robot Originated from a bot.
Projects
None yet
Development

No branches or pull requests

3 participants