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

Exclude FileSystemTests from parallelization #18153

Merged
merged 2 commits into from
Dec 17, 2024
Merged

Conversation

majocha
Copy link
Contributor

@majocha majocha commented Dec 17, 2024

FileSystem is a not thread-safe resource. These tests need to be excluded from parallelization.

@majocha majocha requested a review from a team as a code owner December 17, 2024 08:56
Copy link
Contributor

✅ No release notes required

@psfinaki
Copy link
Member

@majocha I am afraid we might want to do something on top of this since the current execution has crashed on Linux again :/

The active Test Run was aborted because the host process exited unexpectedly. Please inspect the call stack above, if available, to get more information about where the exception originated from.
The test running when the crash occurred: 
Signatures.SigGenerationRoundTripTests.Generate and compile
Language.ExtensionMethodTests.Recursive toplevel named module with Extension attribute and top level let binding with Extension attribute
Language.FixedBindings.Legacy.Pin generic with unmanaged - illegal

@majocha
Copy link
Contributor Author

majocha commented Dec 17, 2024

@psfinaki, I will add a small section about this to TESTGUIDE later, as you suggested.
Probably CI will catch a few more test cases needing exemption from parallelism. I'll keep an eye.

@majocha
Copy link
Contributor Author

majocha commented Dec 17, 2024

@majocha I am afraid we might want to do something on top of this since the current execution has crashed on Linux again :/

The active Test Run was aborted because the host process exited unexpectedly. Please inspect the call stack above, if available, to get more information about where the exception originated from.
The test running when the crash occurred: 
Signatures.SigGenerationRoundTripTests.Generate and compile
Language.ExtensionMethodTests.Recursive toplevel named module with Extension attribute and top level let binding with Extension attribute
Language.FixedBindings.Legacy.Pin generic with unmanaged - illegal

Yeah, currently we're at

args+=" -- xUnit.MaxParallelThreads=3"

and apparently CI can't handle it. It was low on memory even before, so maybe it's that?

@psfinaki
Copy link
Member

Yeah, I remember the low memory warnings... let's see how this plays out. Thanks for taking care :)

@psfinaki
Copy link
Member

Alrighty, so this works although the CI still reports low memory on Linux.

I've looked at a few runs, in all of them the memory gets tight during FsharpSuiteMigrated_TypeCheckTests.

Wonder if we should make them sequential to be an the safe side? We don't have to if it brings the times back again, I am just curious.

@majocha
Copy link
Contributor Author

majocha commented Dec 17, 2024

Alrighty, so this works although the CI still reports low memory on Linux.

I think that low memory was there even before parallel testing PR.

I've looked at a few runs, in all of them the memory gets tight during FsharpSuiteMigrated_TypeCheckTests.

I'm not sure it's that, given that basically all of them have [<FactForDESKTOP>] i.e. should not run at all on linux?

I wonder if this could be related to the Internal CLR error. (0x80131506) we've seen before. We have a minidump so maybe we should open an issue about it somewhere? Maybe in dotnet/runtime?

Copy link
Contributor

@psfinaki
Copy link
Member

Right, yeah it's skipping them.

I wonder if this could be related to the Internal CLR error. (0x80131506) we've #17872 (comment). We have a minidump so maybe we should open an issue about it somewhere? Maybe in dotnet/runtime?

Worth trying, at least maybe they can investigate it a bit :)

Copy link
Contributor

@psfinaki psfinaki merged commit 9d6d41d into dotnet:main Dec 17, 2024
33 checks passed
@majocha majocha deleted the fstests branch December 17, 2024 14:35
@majocha
Copy link
Contributor Author

majocha commented Dec 19, 2024

Right, yeah it's skipping them.

I wonder if this could be related to the Internal CLR error. (0x80131506) we've #17872 (comment). We have a minidump so maybe we should open an issue about it somewhere? Maybe in dotnet/runtime?

Worth trying, at least maybe they can investigate it a bit :)

@psfinaki, I opened an issue for this here: dotnet/runtime#110837 with the minidump we got 3 weeks ago.
This became quite common: https://dev.azure.com/dnceng-public/public/_build/results?buildId=899623&view=logs&j=170942dc-5349-5022-2275-77744f335216&t=20776a93-33c2-52f0-ee8f-b8c9809b2e23&l=14650
maybe it'd be helpful to provide them with additional more recent dumps from above? I can download logs but I don't think I have access to dumps.

Copy link
Contributor

Failed to run : https://github.com/dotnet/fsharp/actions/runs/12416108147

@psfinaki
Copy link
Member

@majocha so looks like you've discovered a real issue :)
Yeah I added another one, hopefully the right thing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants