search

dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
14.9k stars 4.63k forks source link

The given key 'size' was not present in the dictionary in coreclr libraries outerloop tests #87359

Closed Maoni0 closed 1 year ago

Maoni0 commented 1 year ago

Build Information

Build: https://dev.azure.com/dnceng-public/cbb18261-c48f-4abb-8651-8cdcb5474649/_build/results?buildId=302240 Build error leg or test failing: System.Formats.Tar.Tests.TarWriter_WriteEntry_LongFile_Tests.WriteEntry_LongFileSize System.Formats.Tar.Tests.TarWriter_WriteEntryAsync_LongFile_Tests.WriteEntry_LongFileSizeAsync Pull request: https://github.com/dotnet/runtime/pull/87311

Error Message

Fill the error message using step by step known issues guidance.

{
  "ErrorMessage": "System.Collections.Generic.KeyNotFoundException : The given key 'size' was not present in the dictionary.",
  "ErrorPattern": "",
  "BuildRetry": false,
  "ExcludeConsoleLog": false
}

Report

Build Definition Test Pull Request
342381 dotnet/runtime System.Formats.Tar.Tests.TarWriter_WriteEntryAsync_LongFile_Tests.WriteEntry_LongFileSizeAsync dotnet/runtime#88853
342346 dotnet/runtime System.Formats.Tar.Tests.WorkItemExecution dotnet/runtime#88974
342148 dotnet/runtime System.Formats.Tar.Tests.WorkItemExecution dotnet/runtime#88853
341199 dotnet/runtime System.Formats.Tar.Tests.TarWriter_WriteEntryAsync_LongFile_Tests.WriteEntry_LongFileSizeAsync dotnet/runtime#88853
339323 dotnet/runtime System.Formats.Tar.Tests.TarWriter_WriteEntryAsync_LongFile_Tests.WriteEntry_LongFileSizeAsync dotnet/runtime#88893
338729 dotnet/runtime System.Formats.Tar.Tests.WorkItemExecution dotnet/runtime#88853
336280 dotnet/runtime System.Formats.Tar.Tests.TarWriter_WriteEntryAsync_LongFile_Tests.WriteEntry_LongFileSizeAsync dotnet/runtime#87319
336062 dotnet/runtime System.Formats.Tar.Tests.WorkItemExecution dotnet/runtime#87319
326554 dotnet/runtime System.Formats.Tar.Tests.WorkItemExecution dotnet/runtime#87335
324045 dotnet/runtime System.Formats.Tar.Tests.TarWriter_WriteEntryAsync_LongFile_Tests.WriteEntry_LongFileSizeAsync dotnet/runtime#87916
317695 dotnet/runtime System.Formats.Tar.Tests.TarWriter_WriteEntryAsync_LongFile_Tests.WriteEntry_LongFileSizeAsync dotnet/runtime#87943

Summary

24-Hour Hit Count 7-Day Hit Count 1-Month Count
3 8 11

Known issue validation

Build: :mag_right: https://dev.azure.com/dnceng-public/public/_build/results?buildId=324045 Error message validated: System.Collections.Generic.KeyNotFoundException : The given key 'size' was not present in the dictionary. Result validation: :white_check_mark: Known issue matched with the provided build. Validation performed at: 6/28/2023 9:23:56 PM UTC

ghost commented 1 year ago

Tagging subscribers to this area: @dotnet/area-infrastructure-libraries See info in area-owners.md if you want to be subscribed.

Issue Details
## Build Information Build: https://dev.azure.com/dnceng-public/cbb18261-c48f-4abb-8651-8cdcb5474649/_build/results?buildId=302240 Build error leg or test failing: System.Formats.Tar.Tests.TarWriter_WriteEntry_LongFile_Tests.WriteEntry_LongFileSize System.Formats.Tar.Tests.TarWriter_WriteEntryAsync_LongFile_Tests.WriteEntry_LongFileSizeAsync Pull request: https://github.com/dotnet/runtime/pull/87311 ## Error Message Fill the error message using [step by step known issues guidance](https://github.com/dotnet/arcade/blob/main/Documentation/Projects/Build%20Analysis/KnownIssueJsonStepByStep.md#how-to-create-a-known-issue-step-by-step). ```json { "ErrorMessage": "System.Collections.Generic.KeyNotFoundException : The given key 'size' was not present in the dictionary.", "ErrorPattern": "", "BuildRetry": false, "ExcludeConsoleLog": false } ```
Author: Maoni0
Assignees: -
Labels: `area-Infrastructure-libraries`, `blocking-clean-ci`, `Known Build Error`
Milestone: -
ghost commented 1 year ago

Tagging subscribers to this area: @dotnet/area-system-formats-tar See info in area-owners.md if you want to be subscribed.

Issue Details
## Build Information Build: https://dev.azure.com/dnceng-public/cbb18261-c48f-4abb-8651-8cdcb5474649/_build/results?buildId=302240 Build error leg or test failing: System.Formats.Tar.Tests.TarWriter_WriteEntry_LongFile_Tests.WriteEntry_LongFileSize System.Formats.Tar.Tests.TarWriter_WriteEntryAsync_LongFile_Tests.WriteEntry_LongFileSizeAsync Pull request: https://github.com/dotnet/runtime/pull/87311 ## Error Message Fill the error message using [step by step known issues guidance](https://github.com/dotnet/arcade/blob/main/Documentation/Projects/Build%20Analysis/KnownIssueJsonStepByStep.md#how-to-create-a-known-issue-step-by-step). ```json { "ErrorMessage": "System.Collections.Generic.KeyNotFoundException : The given key 'size' was not present in the dictionary.", "ErrorPattern": "", "BuildRetry": false, "ExcludeConsoleLog": false } ``` ### Report |Build|Definition|Test|Pull Request| |---|---|---|---| |[302240](https://dev.azure.com/dnceng-public/public/_build/results?buildId=302240)|dotnet/runtime|[System.Formats.Tar.Tests.WorkItemExecution](https://dev.azure.com/dnceng-public/public/_build/results?buildId=302240&view=ms.vss-test-web.build-test-results-tab&runId=6127826&resultId=104912)|dotnet/runtime#87311| #### Summary |24-Hour Hit Count|7-Day Hit Count|1-Month Count| |---|---|---| |1|1|1|
Author: Maoni0
Assignees: -
Labels: `blocking-clean-ci`, `untriaged`, `Known Build Error`, `area-System.Formats.Tar`
Milestone: -
danmoseley commented 1 year ago 

System.Formats.Tar.Tests.TarWriter_WriteEntry_LongFile_Tests.WriteEntry_LongFileSize(entryFormat: Pax, size: 8589934592, unseekableStream: False) [FAIL]
      System.Collections.Generic.KeyNotFoundException : The given key 'size' was not present in the dictionary.
      Stack Trace:
        /_/src/libraries/System.Private.CoreLib/src/System/Collections/Generic/Dictionary.cs(233,0): at System.Collections.Generic.Dictionary`2.get_Item(TKey key)
        /_/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.Write.cs(518,0): at System.Formats.Tar.TarHeader.WriteCommonFields(Span`1 buffer, TarEntryType actualEntryType)
        /_/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.Write.cs(366,0): at System.Formats.Tar.TarHeader.WritePaxFieldsToBuffer(Int64 size, Span`1 buffer)
        /_/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.Write.cs(385,0): at System.Formats.Tar.TarHeader.WriteFieldsToBuffer(TarEntryFormat format, Int64 bytesToWrite, Span`1 buffer)
        /_/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.Write.cs(40,0): at System.Formats.Tar.TarHeader.WriteAs(TarEntryFormat format, Stream archiveStream, Span`1 buffer)
        /_/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.Write.cs(228,0): at System.Formats.Tar.TarHeader.WriteAsPax(Stream archiveStream, Span`1 buffer)
        /_/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarWriter.cs(297,0): at System.Formats.Tar.TarWriter.WriteEntryInternal(TarEntry entry)
        /_/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarWriter.cs(226,0): at System.Formats.Tar.TarWriter.WriteEntry(TarEntry entry)
        /_/src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.WriteEntry.LongFile.Tests.cs(40,0): at System.Formats.Tar.Tests.TarWriter_WriteEntry_LongFile_Tests.WriteEntry_LongFileSize(TarEntryFormat entryFormat, Int64 size, Boolean unseekableStream)
           at InvokeStub_TarWriter_WriteEntry_LongFile_Tests.WriteEntry_LongFileSize(Object, Object, IntPtr*)
           at System.Reflection.MethodInvoker.Invoke(Object obj, IntPtr* args, BindingFlags invokeAttr)
   System.Formats.Tar.Tests: [Long Running Test] 'System.Formats.Tar.Tests.TarWriter_WriteEntry_LongFile_Tests.WriteEntry_LongFileSize', Elapsed: 00:08:01
   System.Formats.Tar.Tests: [Long Running Test] 'System.Formats.Tar.Tests.TarWriter_WriteEntry_LongFile_Tests.WriteEntry_LongFileSize', Elapsed: 00:10:01
   System.Formats.Tar.Tests: [Long Running Test] 'System.Formats.Tar.Tests.TarWriter_WriteEntry_LongFile_Tests.WriteEntry_LongFileSize', Elapsed: 00:12:01
   System.Formats.Tar.Tests: [Long Running Test] 'System.Formats.Tar.Tests.TarWriter_WriteEntry_LongFile_Tests.WriteEntry_LongFileSize', Elapsed: 00:14:02
['System.Formats.Tar.Tests' END OF WORK ITEM LOG: Command timed out, and was killed]
danmoseley commented 1 year ago

Note there's apparently two issues, the failure due to KeyNotFoundException , and the below. Might need the test to be looped.

After fixing those, this test should probably become manual only, as it's running up against the 15 min timeout, even on release builds.

   System.Formats.Tar.Tests: [Long Running Test] 'System.Formats.Tar.Tests.TarWriter_WriteEntry_LongFile_Tests.WriteEntry_LongFileSize', Elapsed: 00:04:11
   System.Formats.Tar.Tests: [Long Running Test] 'System.Formats.Tar.Tests.TarWriter_WriteEntry_LongFile_Tests.WriteEntry_LongFileSize', Elapsed: 00:06:23
   System.Formats.Tar.Tests: [Long Running Test] 'System.Formats.Tar.Tests.TarWriter_WriteEntry_LongFile_Tests.WriteEntry_LongFileSize', Elapsed: 00:08:35
   System.Formats.Tar.Tests: [Long Running Test] 'System.Formats.Tar.Tests.TarWriter_WriteEntry_LongFile_Tests.WriteEntry_LongFileSize', Elapsed: 00:10:47
   System.Formats.Tar.Tests: [Long Running Test] 'System.Formats.Tar.Tests.TarWriter_WriteEntry_LongFile_Tests.WriteEntry_LongFileSize', Elapsed: 00:12:59
    System.Formats.Tar.Tests.TarWriter_WriteEntry_LongFile_Tests.WriteEntry_LongFileSize(entryFormat: Pax, size: 8589934592, unseekableStream: False) [FAIL]
      Assert.Equal() Failure
      Expected: 8589934592
      Actual:   0
      Stack Trace:
        /_/src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.WriteEntry.LongFile.Tests.cs(48,0): at System.Formats.Tar.Tests.TarWriter_WriteEntry_LongFile_Tests.WriteEntry_LongFileSize(TarEntryFormat entryFormat, Int64 size, Boolean unseekableStream)
           at InvokeStub_TarWriter_WriteEntry_LongFile_Tests.WriteEntry_LongFileSize(Object, Object, IntPtr*)
           at System.Reflection.MethodInvoker.Invoke(Object obj, IntPtr* args, BindingFlags invokeAttr)
danmoseley commented 1 year ago

I had a look to see whether it's something simple.

this should be adding the size to the dictionary, but at this point _size is zero so it doesn't.

>   System.Formats.Tar.dll!System.Formats.Tar.TarHeader.CollectExtendedAttributesFromStandardFieldsIfNeeded() Line 799  C#
    System.Formats.Tar.dll!System.Formats.Tar.TarHeader.WriteAsPax(System.IO.Stream archiveStream, System.Span<byte> buffer) Line 223   C#
    System.Formats.Tar.dll!System.Formats.Tar.TarWriter.WriteEntryInternal(System.Formats.Tar.TarEntry entry) Line 297  C#
    System.Formats.Tar.dll!System.Formats.Tar.TarWriter.WriteEntry(System.Formats.Tar.TarEntry entry) Line 226  C#
    System.Formats.Tar.Tests.dll!System.Formats.Tar.Tests.TarWriter_WriteEntry_LongFile_Tests.WriteEntry_LongFileSize(System.Formats.Tar.TarEntryFormat entryFormat, long size, bool unseekableStream) Line 40  C#

that is here https://github.com/dotnet/runtime/blob/472091827b6a266f9d964564a77ba11160622a50/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.Write.cs#L223

the size is not calculated and added to the extended attributes dictionary until here https://github.com/dotnet/runtime/blob/472091827b6a266f9d964564a77ba11160622a50/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.Write.cs#L228

The 2nd failure mode is probably simply because its release mode and so it gets past the debug assert.

Probably introduced by https://github.com/dotnet/runtime/issues/76707 @Jozkee and wasn't spotted as it's outerloop?

Maoni0 commented 1 year ago

thanks for taking a look, @danmoseley! it seems like the libraries outerloop has many errors. I did another PR that just triggers more background GCs with no other changes and it's still hitting many libraries outerloop failures (hard to say if it's hitting the exactly same issues since there isn't a convenient way to compare them when there are so many).

danmoseley commented 1 year ago

Yeah, I'm not sure what the process currently is for keeping the outerloop clean (@ericstj ). Hopefully it's not too difficult to identify anything new you introduce. Most of these failures are pretty vanilla.

Maoni0 commented 1 year ago

it's actually not easy to identify new errors. as I mentioned, there isn't a convenient way to compare results - not that I know of anyway. and when Build Analysis does not categorize test failures it requires laboriously going through the tests marked as new to see if the failure on each test to see if it's actually a new error (not to mention Build Analysis does not necessarily give accurate results so that's already questionable). I've already spent a bunch of time on this so I think I will stop here.

ericstj commented 1 year ago

Unlike PR validation which will have a forcing function to stay clean, outerloop is only clean based on folks responding to and resolving bugs.

@Maoni0 - are you needing to look in one particular area or are you trying to look across all areas?

Maoni0 commented 1 year ago

no one area. I'm thinking of making a change in GC so I'm just looking for some test runs that might uncover problems.

carlossanlop commented 1 year ago

https://github.com/dotnet/runtime/pull/76707 tried to get it fixed but it seems it didn't work Did fix it. Unfortunately, adding the [Outerloop] attribute to a test class or a test method will cause it to get ignored when running locally, and if Outerloop tests aren't working in CI, they will not be executed. Which is why we couldn't catch it.

I'm working on a fix. _size is being set too late when writing a PAX entry. It should be calculated before writing the extended attributes entry, so that if the number is too large to fit in the regular header, it should get written as an extended attribute.

am11 commented 1 year ago

Passing -p:Outloop=true in -t:Test command runs the outerloop test. I think we can make a basic version of "are all attributes accounted for" test in the regular CI.

carlossanlop commented 1 year ago

Passing -p:Outloop=true in -t:Test command runs the outerloop test.

Thanks @am11, I'll make sure to run those locally.

I think we can make a basic version of "are all attributes accounted for" test in the regular CI.

Not for this scenario, at least not easily. The size extended attribute is only added when the data section size surpasses the max allowed number for the standard size field. This is the reason why we had to write a test that simulates having a very large stream, pass it to the entry data stream, force it to get it all written in memory, and the size should've been detected as being larger than allowed, hence forcing the creation of the extended attribute. But unfortunately, this didn't happen.

It seems I introduced this bug when I merged the PR that refactored our Tar APIs to allow handling of unseekable data streams. The outerloop tests didn't run so I never caught the bug 😢 . https://github.com/dotnet/runtime/commit/84a7be769fa9054dac7a973dda538cf61a488e56

I'm currently testing the fix, will submit a PR soon.

carlossanlop commented 1 year ago

Passing -p:Outloop=true in -t:Test command runs the outerloop test

Tiny typo BTW: it's Outerloop 😸 .