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

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

Closed
Maoni0 opened this issue Jun 10, 2023 · 14 comments · Fixed by #88280
Closed
Assignees
Labels
area-System.Formats.Tar blocking-clean-ci Blocking PR or rolling runs of 'runtime' or 'runtime-extra-platforms'
Milestone

Comments

@Maoni0
Copy link
Member

Maoni0 commented Jun 10, 2023

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: #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 #88853
342346 dotnet/runtime System.Formats.Tar.Tests.WorkItemExecution #88974
342148 dotnet/runtime System.Formats.Tar.Tests.WorkItemExecution #88853
341199 dotnet/runtime System.Formats.Tar.Tests.TarWriter_WriteEntryAsync_LongFile_Tests.WriteEntry_LongFileSizeAsync #88853
339323 dotnet/runtime System.Formats.Tar.Tests.TarWriter_WriteEntryAsync_LongFile_Tests.WriteEntry_LongFileSizeAsync #88893
338729 dotnet/runtime System.Formats.Tar.Tests.WorkItemExecution #88853
336280 dotnet/runtime System.Formats.Tar.Tests.TarWriter_WriteEntryAsync_LongFile_Tests.WriteEntry_LongFileSizeAsync #87319
336062 dotnet/runtime System.Formats.Tar.Tests.WorkItemExecution #87319
326554 dotnet/runtime System.Formats.Tar.Tests.WorkItemExecution #87335
324045 dotnet/runtime System.Formats.Tar.Tests.TarWriter_WriteEntryAsync_LongFile_Tests.WriteEntry_LongFileSizeAsync #87916
317695 dotnet/runtime System.Formats.Tar.Tests.TarWriter_WriteEntryAsync_LongFile_Tests.WriteEntry_LongFileSizeAsync #87943

Summary

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

Known issue validation

Build: 🔎 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: ✅ Known issue matched with the provided build.
Validation performed at: 6/28/2023 9:23:56 PM UTC

@Maoni0 Maoni0 added blocking-clean-ci Blocking PR or rolling runs of 'runtime' or 'runtime-extra-platforms' Known Build Error Use this to report build issues in the .NET Helix tab labels Jun 10, 2023
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Jun 10, 2023
@ghost
Copy link

ghost commented Jun 10, 2023

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: #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
}
Author: Maoni0
Assignees: -
Labels:

area-Infrastructure-libraries, blocking-clean-ci, Known Build Error

Milestone: -

@ghost
Copy link

ghost commented Jun 10, 2023

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: #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
302240 dotnet/runtime System.Formats.Tar.Tests.WorkItemExecution #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
Copy link
Member


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 danmoseley removed the Known Build Error Use this to report build issues in the .NET Helix tab label Jun 10, 2023
@danmoseley
Copy link
Member

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
Copy link
Member

danmoseley commented Jun 12, 2023

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

CollectExtendedAttributesFromStandardFieldsIfNeeded();

the size is not calculated and added to the extended attributes dictionary until here

WriteAs(TarEntryFormat.Pax, archiveStream, buffer);

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

Probably introduced by #76707 @jozkee and wasn't spotted as it's outerloop?

@Maoni0
Copy link
Member Author

Maoni0 commented Jun 12, 2023

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
Copy link
Member

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
Copy link
Member Author

Maoni0 commented Jun 12, 2023

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
Copy link
Member

ericstj commented Jun 12, 2023

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
Copy link
Member Author

Maoni0 commented Jun 12, 2023

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.

@ViktorHofer ViktorHofer added this to the 8.0.0 milestone Jun 21, 2023
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Jun 21, 2023
@carlossanlop carlossanlop self-assigned this Jun 30, 2023
@carlossanlop
Copy link
Member

carlossanlop commented Jun 30, 2023

#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
Copy link
Member

am11 commented Jun 30, 2023

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
Copy link
Member

carlossanlop commented Jun 30, 2023

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 😢 . 84a7be7

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

@carlossanlop
Copy link
Member

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

Tiny typo BTW: it's Outerloop 😸 .

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jun 30, 2023
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jul 17, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Aug 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Formats.Tar blocking-clean-ci Blocking PR or rolling runs of 'runtime' or 'runtime-extra-platforms'
Projects
None yet
6 participants