New issue
Advanced search Search tips

Issue 786142 link

Starred by 2 users

Issue metadata

Status: Untriaged
Owner: ----
Cc:
Components:
EstimatedDays: 1
NextAction: ----
OS: Windows
Pri: 2
Type: Bug



Sign in to add a comment

logdog.exe doesn't halt on Ctrl+C

Project Member Reported by no...@chromium.org, Nov 16 2017

Issue description

from https://bugs.chromium.org/p/chromium/issues/detail?id=698429#c20 by brucedawson@:

When using a python script to retrieve logs with logdog.exe I often want it to keep going until there are no more logs. However the "logdog cat" command doesn't seem to have an option to return immediately when there is no data. Instead it repeatedly prints this:

[I2017-11-16T11:27:37.895249-08:00 99932 0 fetcher.go:366] No logs returned. Sleeping...               {"delay":"5s", "index":4266}

Is there a recommended way of doing this? An option to the cat command to tell it to return immediately would work. Alternately, if logdog.exe would respond to Ctrl+C then I could abort it.
 

Comment 1 by d...@chromium.org, Nov 16 2017

Well, firstly it should respond to Ctrl+C and terminate. If it doesn't, that's a separate bug and worth filing.

LogDog also has a "-timeout" flag (run `logdog -help`) that allows you to specify the maximum amount of time to wait for a new log entry before self-termination. The command would look something like:

$ logdog -timeout 5s cat <flags...>

The deeper problem is that the tool, by design, accommodates ingest latency, so there's no real distinction between:

- There are no more logs in the stream, ever.
- There are more logs, but LogDog is slow / backed up / etc. and you can't view them yet, but if you try a bit longer you will be able to.
- There will be more logs, but the producer hasn't produced them yet, and you have to wait for them.

If you want to check if the log stream exists at all, you can use the "query" command first to fetch its state. This will return a distinctive "does not exist" if the stream hasn't been registered, as well as let you know if the stream is still streaming or has terminated. "query" has a "-json" flag that emits stream status in an application-friendly JSON format.
I totally failed to see the -timeout option - that helps greatly.

I'll file a bug for Ctrl+C not working.

Comment 3 by no...@chromium.org, Nov 16 2017

if Ctrl-C is a separate bug, what's left here?

Comment 4 by d...@chromium.org, Nov 16 2017

Cool! FYI here's the code that's *supposed* to terminate when Ctrl+C is hit:

https://chromium.googlesource.com/infra/luci/luci-go/+/master/logdog/client/cli/main.go#200
Labels: OS-Windows
Summary: logdog.exe doesn't halt on Ctrl+C (was: logdog cat waits for logs ~forever)
Well let's just rename the bug then. Done.

Note that the failure to respond to Ctrl+C is seen with the windows-amd64 build. I have not tested it with any other versions. It fails to respond to Ctrl+C both when run from python and when run standalone.

BTW, I figured out why I didn't see the -timeout command' I was running "logdog help". This claims to list all of the commands, including itself, but it doesn't list the -help command. That is why I never saw -timeout. I filed bug 786176 for this. If help listed all of the commands and options then I would have found -timeout on my own.

Status: Available (was: Untriaged)

Comment 7 by efoo@chromium.org, Dec 6 2017

Labels: LUCI-Afterglow
I don't know how that code is supposed to handle Ctrl+C so I can't guess as to why it might not be working.

Explicit handling of Ctrl+C is demonstrated here:

https://cs.chromium.org/chromium/src/tools/win/IdleWakeups/idle_wakeups.cpp?l=269

In this case it was used to do an orderly shutdown with time to print summary information, but the handler could also just call TerminateProcess. I haven't written any Go code so if there's somebody else who knows how to interop from Go to Win32 that would be ideal.

Comment 9 by d...@chromium.org, Dec 7 2017

This is explicit Ctrl+C handling. Ctrl+C manifests as an OS signal, and the code section that I linked in #4 installs a signal handler for those signals and responds by beginning a termination process.

C++ handles signals differently than Go, for sure.
Cc: mar...@chromium.org
I went pretty deep into the Go standard library until I got to this:
https://go.googlesource.com/go/+/go1.9.2/src/runtime/signal_windows.go#192
¯\_(ツ)_/¯

I'm sure this is fixable, it may be more "work".
"// Following are not implemented."

Sigh...

The Go runtime and I really don't get along well. It's a good thing the -timeout option is available (although it would still be good to have it easier to find - see bug 786176) since that makes Ctrl+C support less critical.
This thread: https://github.com/golang/go/issues/6720

says ctrl-C should still work correctly on windows.

After more looking the docs say we should be using a buffered channel to receive the signals or could miss one. It's a 3 character change but I'd need to get my dev environment set up on my home windows box to test it out, so it might take some time before I get to it.
Project Member

Comment 13 by bugdroid1@chromium.org, Dec 15 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/luci/luci-go.git/+/24fefb7f5dc6ae4811bc0fa027a4c9e928fea40d

commit 24fefb7f5dc6ae4811bc0fa027a4c9e928fea40d
Author: Nodir Turakulov <nodir@google.com>
Date: Fri Dec 15 06:29:37 2017

[logdog] use buffered channel for signals

Bug: 786142
Change-Id: Ib5342dd4bac640c6df8294c77074e446c0787bd0
Reviewed-on: https://chromium-review.googlesource.com/826320
Reviewed-by: Erik Staab <estaab@chromium.org>
Commit-Queue: Nodir Turakulov <nodir@chromium.org>

[modify] https://crrev.com/24fefb7f5dc6ae4811bc0fa027a4c9e928fea40d/logdog/client/cli/main.go

Project Member

Comment 14 by bugdroid1@chromium.org, Dec 16 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/infra/+/d546a30c7365c7b1838832d9a9c750a91d1d395f

commit d546a30c7365c7b1838832d9a9c750a91d1d395f
Author: Nodir Turakulov <nodir@google.com>
Date: Sat Dec 16 01:05:20 2017

Roll infra/go/src/go.chromium.org/luci/ 79172f5b4..94f3a61cb (5 commits)

https://chromium.googlesource.com/infra/luci/luci-go/+log/79172f5b4ee3..94f3a61cb309

$ git log 79172f5b4..94f3a61cb --date=short --no-merges --format='%ad %ae %s'
2017-12-14 nodir [bqschemaupdater] allow python projects to pass -proto-path flag
2017-12-14 nodir [logdog] use buffered channel for signals
2017-12-14 tandrii milo: link to the right Gerrit patch in tryjobs.
2017-12-13 smut [Machine Database] Create client which can retrieve datacenters
2017-12-12 smut [Machine Database] Ensure switches match the config

Created with:
  roll-dep infra/go/src/go.chromium.org/luci

Bug: 786142
Change-Id: I0d588606fdc51eea8e0b6e7bc7f5bf08cf86c386
Reviewed-on: https://chromium-review.googlesource.com/828502
Reviewed-by: Erik Staab <estaab@chromium.org>
Commit-Queue: Nodir Turakulov <nodir@chromium.org>

[modify] https://crrev.com/d546a30c7365c7b1838832d9a9c750a91d1d395f/DEPS

latest version of logdog uses a buffered channel for signals. I don't see how it may help for this problem, but please give it a try. Where is your logdog binary is coming from? Did you compile it yourself?
Project Member

Comment 16 by sheriffbot@chromium.org, Jan 7

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Sign in to add a comment