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

Quick infrastructure fix-ups #25545

Merged
merged 2 commits into from
Sep 3, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
2 changes: 1 addition & 1 deletion .azure/pipelines/quarantined-pr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ jobs:
jobName: MacOS_Quarantined_Test
jobDisplayName: "Tests: macOS 10.14"
agentOs: macOS
timeoutInMinutes: 60
timeoutInMinutes: 120
Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Ah, crap, I forgot to fix this in the regular job. Will go do that now

Copy link
Member Author

Choose a reason for hiding this comment

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

That job hasn't been timing out has it i.e. weren't the timeouts specific to the quarantined pipeline❔

Copy link
Member Author

Choose a reason for hiding this comment

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

And, oops, forgot you were doing this part

Copy link
Member

Choose a reason for hiding this comment

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

Not sure, I thought you meant it was both - I can close #25546 if it's just this one

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, ci.yml doesn't specify timeouts (except for the Helix job) which means its macOS test job gets the default-build.yml default of 180 minutes. Since that's so close to when https://dev.azure.com/dnceng/public/_build/results?buildId=795980&view=results passed, I'll use 240 minutes there.

Copy link
Contributor

Choose a reason for hiding this comment

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

Let me clarify, the aspnetcore-ci run completed after 90 minutes so it still passed without timing out. I think a default timeout of 180 minutes is still fine. I just wanted to mention that build because 90 minutes is more than 3x the usual time of about 25 mins. It's slow, but I don't think we need to adjust our timeout for it.

Copy link
Member Author

Choose a reason for hiding this comment

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

I pushed a change to err on the safe side. The extra half hour timeout is less likely to cause problems elsewhere than having to retry a 1.5 hour job.

Copy link
Member Author

Choose a reason for hiding this comment

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

(Note I didn't double the timeout there😃)

Copy link
Contributor

Choose a reason for hiding this comment

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

I haven't paid too much attention to it after my ops rotation so my sample size is limited, but I've seen slowness in macos builds both on aspnetcore-ci and aspnetcore-quarantined-pr.

About the same for me as well. Seen occasional slowness for macos specifically.

isTestingJob: true
steps:
- bash: ./build.sh --all --pack --ci --nobl --no-build-java
Expand Down
4 changes: 2 additions & 2 deletions eng/Version.Details.xml
Original file line number Diff line number Diff line change
Expand Up @@ -329,9 +329,9 @@
<Uri>https://github.com/dotnet/arcade</Uri>
<Sha>4be47e467013f8a07a1ed7b6e49e39c8150bde54</Sha>
</Dependency>
<Dependency Name="Microsoft.Net.Compilers.Toolset" Version="3.8.0-2.20407.3">
<Dependency Name="Microsoft.Net.Compilers.Toolset" Version="3.8.0-2.20403.2" Pinned="true">
Copy link
Member Author

Choose a reason for hiding this comment

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

@pranavkm the previous version came in through adea663 aka #24738. Do you know if we took a dependency on anything changed after 3.8.0-2.20403.2❔ It's a small window compared to the upgrade in that PR but I'm confirming❕

Copy link
Member

Choose a reason for hiding this comment

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

Why pin this?

Copy link
Member Author

Choose a reason for hiding this comment

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

@TanayParikh this won't protect us from downgrades in the 'master' branch 'til it's merged forward and also won't impact the aspnetcore ➡️ aspnetcore-tooling dependency updates 'til then. Suggest we trigger our Roslyn subscription to get back to being ahead of Arcade in this version on our 'master' branch. Do you know how to do that❔ If not, ping me in Teams.

Copy link
Member Author

Choose a reason for hiding this comment

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

Why pin this?

This is for safety. We've seen runtime changes break roslyn and then roslyn break runtime and extensions too late in the game. Had an offline conversation about this w/ @Anipik @ericstj @jaredpar @mmitche @safern and we landed on this version. (Might move forward slightly later in RC2 but that'll be deliberate and coordinated.)

Copy link
Member Author

Choose a reason for hiding this comment

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

/fyi all. This version has been discussed in a few different places, the latest being dotnet/roslyn#46575. In RC2, runtime, aspnetcore, and extensions will use 3.8.0-2.20403.2. (aspnetcore-tooling doesn't have release/5.0* branches.)

The plan is different in our master branches. At least arcade, aspnetcore and aspnetcore-tooling will follow the 'VS 16.8' channel to get newer Microsoft.Net.Compilers.Toolset versions. We will likely move to use the 'VS Master' channel later in 6.0.

<Uri>https://github.com/dotnet/roslyn</Uri>
<Sha>dba2fa57432b4bd9cc7880e2c6fe3acdd01bba3c</Sha>
<Sha>b6a07e61473ed8a804de465f7c1cb5c17f80732d</Sha>
</Dependency>
</ToolsetDependencies>
</Dependencies>
2 changes: 1 addition & 1 deletion eng/Versions.props
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@
-->
<PropertyGroup Label="Automated">
<!-- Packages from dotnet/roslyn -->
<MicrosoftNetCompilersToolsetPackageVersion>3.8.0-2.20407.3</MicrosoftNetCompilersToolsetPackageVersion>
<MicrosoftNetCompilersToolsetPackageVersion>3.8.0-2.20403.2</MicrosoftNetCompilersToolsetPackageVersion>
<!-- Packages from dotnet/runtime -->
<MicrosoftExtensionsDependencyModelPackageVersion>5.0.0-rc.2.20451.27</MicrosoftExtensionsDependencyModelPackageVersion>
<MicrosoftNETCoreAppInternalPackageVersion>5.0.0-rc.2.20451.27</MicrosoftNETCoreAppInternalPackageVersion>
Expand Down