dotnet / dnceng

.NET Engineering Services
MIT License
24 stars 19 forks source link

Generate dumps for hanging tests #1216

Open danmoseley opened 4 years ago

danmoseley commented 4 years ago

Right now when a test hangs we do not get a dump file. Much of the time we need a dump (can't repro locally and code inspection doesn't show an obvious problem)

This mechanism needs to be a generic Helix mechanism - not tied to a particular repo's test infrastructure.

[Exception: In a few cases, in the libraries tests, we spawn a child process ("RemoteExec") and in those cases we can improve that infrastructure ourselves to get dumps on hangs. We have done this already on Windows and Linux is tracked here. ]

But in most cases, it's Helix infrastructure that needs to collect the dump.

Mono needs this also apparently: https://github.com/dotnet/runtime/issues/32325

danmoseley commented 4 years ago

cc @stephentoub - I'm not really adding any value by opening this issue just FYI we now have one.

danmoseley commented 4 years ago

Perhaps requires https://github.com/dotnet/core-eng/issues/9533#issuecomment-610595478

ViktorHofer commented 4 years ago

cc @davidfowl

michellemcdaniel commented 2 years ago

Closing because the parent Epic was closed. If you believe this issue should still be worked on, please re-open.

agocke commented 2 years ago

Looks like this still hasn't been completed.

MattGal commented 2 years ago

Current ask: it's very cheap for us to call our timeout kill() with SIGQUIT on non-Windows systems; marking for triage discussion

davidfowl commented 2 years ago

AFAIK we did this. What doesn't work exactly?

MichalStrehovsky commented 2 years ago

AFAIK we did this. What doesn't work exactly?

E.g. this job: https://dev.azure.com/dnceng/public/_build/results?buildId=1838524&view=logs&j=b2c675c1-1c32-50e9-7c40-e762459300db&t=4f51db95-f7de-5a4e-cd10-f078782dd75b

ended with

...
[EXECUTION TIMED OUT]
Exit Code:-3Executor timed out after 2700 seconds and was killed
['System.Runtime.Tests' END OF WORK ITEM LOG: Command timed out, and was killed]

But there's no crashdump for it to see where the process got stuck.

davidfowl commented 2 years ago

Oh wait, the runtime uses a jank version of xunit right? It doesn't use the dotnet test runner.

MichalStrehovsky commented 2 years ago

Yes, there's a custom runner. But unless the timeouting logic of dotnet test is in native code, I don't think that would help in situations like this (we suspect this one is a hang in GC suspension, which means any timeouting logic written in managed code likely wouldn't get a chance to run). We basically need the process to be killed in a way that produced a crashdump.

davidfowl commented 2 years ago

@MichalStrehovsky the test host is a separate process so it would work just fine since it's the one collect a dump from the user process hosting test cases. The runtime doesn't use this currently and I thought there was a plan to move it over but I don't know where we are there.

cc @ViktorHofer

MichalStrehovsky commented 2 years ago

We won't be able to use the test host for Native AOT anyway, at least not in near future.

I'm trying to chase it in low tech way here: https://github.com/dotnet/runtime/pull/71177. It's not great.

MattGal commented 2 years ago

We discussed this today in triage, and will not be moving forward with the "SIGQUIT" idea. While Helix machines will continue to happily collect dumps and provide them along with logs and result XMLs, the ROI and challenges of doing it from the Helix client remain too high to be worth it.

Some thoughts:

As runtime has already shown a way forward, and dotnet test can provide information (not just dumps) when tests hang, it does not make sense to try to force this behavior on every helix work item universally.

ViktorHofer commented 2 years ago

@davidfowl

The runtime doesn't use this currently and I thought there was a plan to move it over but I don't know where we are there.

Unfortunately as long as VSTest depends on functionality that we would like avoid to depend on, i.e. the networking stack, we won't be able to use it.

See https://github.com/microsoft/vstest/issues/3595#issuecomment-1124921764 in which other devs like @jkotas shared concerns with depending on VSTest in its current state and what we would need.

IMO in an ideal world we would have an in-proc test runner entry point, generated with a source generator based on the test framework's attributes (i.e. Fact/Theory for xunit) to make a test invocation fast and reflection free. In addition to that, there should be an option to just print to stdout to avoid the IO dependency when it's not strictly needed.

davidfowl commented 2 years ago

I don’t know if I agree with that. How does that work with VS? Is this a command line only story? Is this for customers or just the runtime (which has a unique problem)? Is this with things like nativeAOT in mind? I’m missing the big picture here

jkotas commented 2 years ago

How does that work with VS? Is this a command line only story?

VS test does not work for the inner loop if you work on the core runtime, for the reasons mentioned above. Command line only.

Is this for customers or just the runtime (which has a unique problem)? Is this with things like nativeAOT in mind?

It is primarily for the runtime, but the problems are not necessarily unique to the runtime.

For example, vstest does not have first class support for single-file testing (ie verifying that you library works in a single file deployment). We have custom solutions for number of these problems in the runtime.

ViktorHofer commented 2 years ago

VS test does not work for the inner loop if you work on the core runtime, for the reasons mentioned above. Command line only.

FWIW, VS Test Explorer which uses VSTest underneath does work well in the inner loop for libraries developers (working on source code in this repository under src/libraries).

danmoseley commented 2 years ago

As runtime has already shown a way forward,

I assume you mean https://github.com/dotnet/runtime/pull/71177 .. @MichalStrehovsky did that work?

We still need a way to get dumps from hangs in runtime unit tests. Reactivating while we discuss that and maybe when we have clarity we can move this to runtime.

@hoyosjs @mikem8361 @elinor-fung let us say that we are limiting the problem to getting a dump in this situation -- "long running test"

   System.Threading.Tasks.Dataflow.Tests: [Long Running Test] 'System.Threading.Tasks.Dataflow.Tests.TransformManyBlockTests.TestProducerConsumerAsyncEnumerable', Elapsed: 00:42:19
   System.Threading.Tasks.Dataflow.Tests: [Long Running Test] 'System.Threading.Tasks.Dataflow.Tests.TransformManyBlockTests.TestProducerConsumerAsyncEnumerable', Elapsed: 00:44:20
['System.Threading.Tasks.Dataflow.Tests' END OF WORK ITEM LOG: Command timed out, and was killed]

If we could make xunit send a SIGQUIT to the worker process, would that make a dump? Would we need the createdump variables set?

hoyosjs commented 2 years ago

We'd need the variables set. dotnet test used the diagnostics netcore client library to collect dumps on hang, but as they noted this is a VSTest feature. The library could be used in XUnit too or whatever we desired to request a point-in-time dump. Now sure mono implements that one. Also, that solution doesn't work for NativeAOT. Native AOT would require ulimit and rely on the OSs capability for dump collection.

danmoseley commented 2 years ago

OK so work to make this happen for our unit tests would be

and vstest does all this, so we know it works?

If so - I can open an issue in dotnet/runtime.

This wouldn't help coreclr tests, right? If I understand right, those are standalone exe's so there is no harness to figure out something hung. (cc @trylek )

jkotas commented 2 years ago

This wouldn't help coreclr tests, right?

Also, it wouldn't help libraries tests in case the hang is in the runtime (GC, etc.).

hoyosjs commented 2 years ago
hoyosjs commented 2 years ago

In runtime the one that launches the scripts acts as a watchdog of hangs and collects dumps on timeout.

danmoseley commented 2 years ago

@hoyosjs is it reasonable to move this to an issue in dotnet/runtime? unless and until it becomes clear that there's some way to handle it centrally

hoyosjs commented 2 years ago

CoreCLR already implements this for the runtime tests in here: https://github.com/dotnet/runtime/blob/543bcc5ee7d6a2b9471b016770227421c43a756e/src/tests/Common/Coreclr.TestWrapper/CoreclrTestWrapperLib.cs#L207-L254. The vstest solution is better for upstack repos. Libraries would either need to use vstest (which I am not sure I recommend given they use a live-built coreclr), or have an external monitor grab dumps. In-proc xunit running the patched runtime as we currently do is not resilient against runtime issues. CoreCLR will likely need this too with the wrapper based approach.

rzikm commented 4 months ago

I see this feature request has been opened for some time, is there an approximate timeline for this feature or some minimal subset? We (.NET Networking team) are mainly interested for hangs in .NET libraries test runs.

agocke commented 4 months ago

I would probably close this issue on the dnceng side. It doesn’t seem realistic to have shared dnceng test execution infrastructure any time soon. There are too many moving pieces and unique configurations in different repos.

If this gets done it probably needs to be done at the individual team level.