Skip to content
Snippets Groups Projects

AthenaCommon+AthenaConfiguration, etc: Add EventWritten incident.

Merged Scott Snyder requested to merge ssnyder/athena:eventWritten.AthenaCommon-20210602 into master
1 unresolved thread

Fire an EventWritten incident after all event output has completed.

Update affected reference files.

Merge request reports

Pipeline #2675102 failed

Pipeline failed for 34864601 on ssnyder:eventWritten.AthenaCommon-20210602

Merged by Johannes ElmsheuserJohannes Elmsheuser 3 years ago (Jun 2, 2021 3:59pm UTC)

Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • :warning: WARNING: big files (>100K) are found in the changeset

    :pencil: 236K in file Control/DataModelTest/DataModelRunTests/share/xAODTestRead3MT.ref

  • This merge request affects 9 packages:

    • Calorimeter/CaloClusterCorrection
    • Calorimeter/CaloRec
    • Calorimeter/CaloUtils
    • Control/AthenaCommon
    • Control/AthenaConfiguration
    • Control/AthenaExamples/AthExHelloWorld
    • Control/AthenaServices
    • Control/DataModelTest/DataModelRunTests
    • TileCalorimeter/TileSvc/TileByteStream

    Affected files list will not be printed in this case

    Adding @pavol ,@harkusha ,@solodkov ,@ssnyder as watchers

  • :negative_squared_cross_mark: CI Result FAILURE (hash 34864601)

    Athena AthSimulation AthGeneration AnalysisBase AthAnalysis DetCommon
    externals :white_check_mark: :white_check_mark: :white_check_mark: :white_check_mark: :white_check_mark: :white_check_mark:
    cmake :white_check_mark: :white_check_mark: :white_check_mark: :white_check_mark: :white_check_mark: :white_check_mark:
    make :white_check_mark: :white_check_mark: :white_check_mark: :white_check_mark: :white_check_mark: :white_check_mark:
    required tests :o: :o: :white_check_mark: :white_check_mark: :white_check_mark: :white_check_mark:
    optional tests :white_check_mark: :white_check_mark: :white_check_mark: :white_check_mark: :white_check_mark: :white_check_mark:

    Full details available on this CI monitor view
    :white_check_mark: Athena: number of compilation errors 0, warnings 0
    :white_check_mark: AthSimulation: number of compilation errors 0, warnings 0
    :white_check_mark: AthGeneration: number of compilation errors 0, warnings 0
    :white_check_mark: AnalysisBase: number of compilation errors 0, warnings 0
    :white_check_mark: AthAnalysis: number of compilation errors 0, warnings 0
    :white_check_mark: DetCommon: number of compilation errors 0, warnings 0
    :pencil: For experts only: Jenkins output [CI-MERGE-REQUEST-CC7 34651]

  • Jenkins please retry a build

  • :warning: WARNING: big files (>100K) are found in the changeset

    :pencil: 236K in file Control/DataModelTest/DataModelRunTests/share/xAODTestRead3MT.ref

  • This merge request affects 9 packages:

    • Calorimeter/CaloClusterCorrection
    • Calorimeter/CaloRec
    • Calorimeter/CaloUtils
    • Control/AthenaCommon
    • Control/AthenaConfiguration
    • Control/AthenaExamples/AthExHelloWorld
    • Control/AthenaServices
    • Control/DataModelTest/DataModelRunTests
    • TileCalorimeter/TileSvc/TileByteStream

    Affected files list will not be printed in this case

    Adding @pavol ,@harkusha ,@solodkov ,@ssnyder as watchers

    • This is just unconditionally firing an "EventWritten" incident after AthOutSeq in all athena jobs, right? Even if AthOutSeq is empty and there is no output writing actually done.

      What do you plan to use this incident for? I assume it will be harmless for jobs not writing any output, but for jobs using HltEventLoopMgr the incident will be fired actually before the output writing which may be a problem.

      I'm not opposing this MR (reviewers, please proceed with the review) - just asking questions to understand where this is going.

    • Author Developer

      The goal is to allow moving clearing the event store, which has been observed to be a bottleneck in many-thread jobs, into the context of an algorithm, so it can be run in parallel.

      Doing it from an incident is attractive because that allows the event loop manager to retain control (and responsibility) for when this happens. But there was no existing incident appropriate for this. EndEvent happens before the event is written, and EndProcessing is fired from the event loop manager and runs in that thread (and changing that would be risky).

      The next step is to change the MT event loop manager to clear the store when this incident is fired. Other event loop managers will be unaffected.

    • Thanks for explaining. I can certainly see how this solution is attractive - it's likely to achieve what's needed in a fairly straightforward way. But this is also where online and offline diverge a bit more, so I'm just cautious about a possible interference between our solutions. As long as the offline loop manager remains the only listener of this new incident, I see no problem. I was worried you had some other services/components in mind for this, but with just AthenaHiveEventLoopMgr this is fine.

      cc @fwinkl, @wiedenma

    • Author Developer

      I guess we could consider calling it something else if you think it might be misleading. The current name was intended to connote something that runs after AthOutSeq is finished.

    • Perhaps [Algorithms|Sequences][Finished|Done] or something along these lines could be a bit more matching? If you prefer to keep the current name, we'll just need to keep an eye on people reusing this for another purpose with a wrong assumption. It would likely come up in CI anyway if someone uses the incident in an online-incompatible way.

    • Please register or sign in to reply
  • :negative_squared_cross_mark: CI Result FAILURE (hash 34864601)

    Athena AthSimulation AthGeneration AnalysisBase AthAnalysis DetCommon
    externals :white_check_mark: :white_check_mark: :white_check_mark: :white_check_mark: :white_check_mark: :white_check_mark:
    cmake :white_check_mark: :white_check_mark: :white_check_mark: :white_check_mark: :white_check_mark: :white_check_mark:
    make :white_check_mark: :white_check_mark: :white_check_mark: :white_check_mark: :white_check_mark: :white_check_mark:
    required tests :o: :white_check_mark: :white_check_mark: :white_check_mark: :white_check_mark: :white_check_mark:
    optional tests :white_check_mark: :white_check_mark: :white_check_mark: :white_check_mark: :white_check_mark: :white_check_mark:

    Full details available on this CI monitor view
    :white_check_mark: Athena: number of compilation errors 0, warnings 0
    :white_check_mark: AthSimulation: number of compilation errors 0, warnings 0
    :white_check_mark: AthGeneration: number of compilation errors 0, warnings 0
    :white_check_mark: AnalysisBase: number of compilation errors 0, warnings 0
    :white_check_mark: AthAnalysis: number of compilation errors 0, warnings 0
    :white_check_mark: DetCommon: number of compilation errors 0, warnings 0
    :pencil: For experts only: Jenkins output [CI-MERGE-REQUEST-CC7 34666]

  • Author Developer

    Test failures seen are not related to this change. This can be merged.

  • Scott Snyder mentioned in merge request !44070 (closed)

    mentioned in merge request !44070 (closed)

  • mentioned in commit 208de525

Please register or sign in to reply
Loading