Conversation
* real fix needs to happen in TraceEvent
|
Tagging subscribers to this area: @tarekgh, @tommcdon, @pjanotti Issue Details
As noted in #44072, the intermittent failure is caused by the reader ( The bandaid fix is to put another provider on the test that guarantees there will be more data to read and the I'm hacking up a patch for the reader code in TraceEvent that will prevent this corner case, but that will take time to flow into the runtime repo, so this bandaid will do for now. I'll open the PR in draft mode to run the CI a few times through and then mark it for review. I want these tests turned back on before we complete #46079. CC @tommcdon @noahfalk @sywhang @lateralusX @brianrob
|
|
/azp run runtime |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run runtime |
|
2 successful runs and I'm running a 3rd for more confidence. |
|
Azure Pipelines successfully started running 1 pipeline(s). |
sywhang
left a comment
There was a problem hiding this comment.
LGTM as long as you remove this when we have the new TraceEvent with the actual fix.
As noted in #44072, the intermittent failure is caused by the reader (
EventPipeEventSource) indefinitely pausing while trying to read bytes to reach 8-byte alignment.EventBlocksare not required to be aligned lengths. There are only 2 events associated with the keywordEEStartupStartbelongs to, so once those events have been received there won't be more data sent. If theEventBlockcontaining those events is not an 8-byte aligned size, the reader will read theEventBlock, but not dispatch the events until it receives enough data to fill the alignment requirement. That data won't arrive, so the test fails with the reader never reporting that theEEStartupStartevent fired.The bandaid fix is to put another provider on the test that guarantees there will be more data to read and the
EventBlockcontainingEEStartupStartwill get dispatched byEventPipeEventSource. In this case, I'm addingMicrosoft-DotNETCore-SampleProfiler, but theoretically any other provider that generates an event would be sufficient.I'm hacking up a patch for the reader code in TraceEvent that will prevent this corner case, but that will take time to flow into the runtime repo, so this bandaid will do for now.
I'll open the PR in draft mode to run the CI a few times through and then mark it for review. I want these tests turned back on before we complete #46079.
CC @tommcdon @noahfalk @sywhang @lateralusX @brianrob