Add HLT timeout handling and improve handling of other errors

Implemented handling of soft timeout in the online HLT framework (ATR-16897), improved a few aspects of general error handling (ATR-19248), and fixed a few small things.

Changes:

• Avoid using ATH_CHECK for calls which can return Athena::Status::TIMEOUT, because it prints out a FATAL message in this case.
• Return Athena::Status::TIMEOUT on timeout in the MTCalibPeb test tool/alg.
• Define hltonl::PSCErrorCode::TIMEOUT in TrigKernel
• Fix a few uninitialised members in HltEventLoopMgr (mainly related to timeout-handling).
• HltEventLoopMgr: on non-success EventStatus, check if timeout happened and, if yes, send event to a dedicated timeout debug stream.
• HltEventLoopMgr: use EventID from context instead of the old EventInfo in error print-outs.
• HLTResultMTByteStreamCnv: make sure failed events go only to the debug streams (remove stream tags of other type), but still save all the HLT results to this stream, if they're available.
• TrigOutputHandling tools: check validity of HLTSummary handle before using it.
• Add a test of the timeout handling in TrigP1Test (job options and ART test shell script).

Questions already resolved in the discussion below:

1. Shall we adapt ATH_CHECK to not print FATAL on timeout status?
   Answer: The error policy will be reviewed with software coordinators later, for now this MR can be accepted as it is.
2. I realised it doesn't make much sense to try constructing an HLT result if DecisionSummaryMaker didn't run, and this currently is the case for any timeout/failure. If we want to save as much information as possible to the debug stream, maybe we should run it explicitly in the online framework, and not as an algorithm?
   Answer: The partial summary information obtained by running summary maker on aborted events would not be useful for debugging offline, so there is no point running this.

Tagging @fwinkl, @tamartin, @tbold, @wiedenma

Could ATH_CHECK not also propagate timeout? (Sure, we can't decide this just ourselves, but we could ask?)

I am looking at ATH_CHECK. It shouldn't behave like this...

It does propagate the StatusCode correctly, but it prints a FATAL message because apparently TIMEOUT is not "recoverable":
https://gitlab.cern.ch/atlas/athena/blob/master/Control/AthenaKernel/AthenaKernel/errorcheck.h#L365-374

Yes, I am going to change that logic.

Thanks @fwinkl, then I will remove the ATH_CHECK workaround from this MR.

@tamartin, @tbold, any opinion on the second question?

Thinking about this some more, printing a FATAL is at least consistent. It's the same message level that's printed if you return StatusCode::FAILURE from an algorithm/tool. And the event does get send to the debug stream just like for any other failures during execute. I would suggest we keep it like that for the moment.

Thinking more from the online perspective, shouldn't FATAL mean that the application cannot continue running and will exit? I guess this is what happens in offline athena when an algorithm returns StatusCode::FAILURE, but that's not the case online. We don't want to propagate a FATAL message from HLT to ERS for issues which don't stop the application.

changed the description

Yes, I agree. I think only the EventLoopMgr should have the "right" to print a FATAL message when deciding on the fate of the job. Let me start a discussion on the core sw mailing list next week...

I'm not sure we gain anything from DecisionSummaryMaker running in aborted events.

So long as we read out whatever DesicionContainers were already produced, then offline we can reconstruct how far the processing managed to get.

StreamTagMakerTool and TriggerBitsMakerTool rely on the HLTSummary information, so without it we cannot get the PEB information (including data scouting) or HLT bits in the debug stream result. The serialiser doesn't depend on HLTSummary, so you are right that the individual DecisionContainers will be serialised. Since debug streams do full event building, I add the "main" HLT result to them by default, but if StreamTagMakerTool fails, no other HLT result will be produced. That's because we create only HLT result ROBFragments which are explicitly requested to be written out in at least one stream tag.

Different chains will accept the event after different numbers of steps.

If we run the DecisionSummaryMaker after soft timing out in (say) step 5, then chains which accepted the event already somewhere in step 1-4 would be able to properly pass through the DecisionSummaryMaker and set bits, stream tags.

Not sure we'd want this, really.

Right, thinking about it again, this information probably won't be useful in any way for debug recovery offline. In both cases, either a timeout or an algo failure, we would just rerun the HLT offline without even looking at the online HLT result.

Let's give this one round of CI in the current state and follow up on the ATH_CHECK next week.

Jenkins please retry a build

This merge request affects 7 packages:

• HLT/Trigger/TrigControl/TrigExamples/TrigExPartialEB
• HLT/Trigger/TrigControl/TrigKernel
• HLT/Trigger/TrigControl/TrigServices
• Trigger/TrigEvent/TrigSteeringEvent
• Trigger/TrigSteer/TrigHLTResultByteStream
• Trigger/TrigSteer/TrigOutputHandling
• Trigger/TrigValidation/TrigP1Test

Adding @fwinkl ,@wiedenma ,@rbielski as watchers

added Trigger master labels

CI Result FAILURE

	Athena	AthSimulation
externals
cmake
make
required tests
optional tests

Full details available at NICOS MR-21931-2019-03-16-02-26
Athena: number of compilation errors 0, warnings 0
AthSimulation: number of compilation errors 0, warnings 0
For experts only: Jenkins output [CI-MERGE-REQUEST 35533]

As discussed with @fwinkl, the ATH_CHECK behaviour and the general error policy will need to be re-discussed with a wider audience probably next week. This should not hold this MR and it should be fine to go in the current form. I will un-WIP now.

unmarked as a Work In Progress

changed the description

Jenkins please retry a build

This merge request affects 7 packages:

• HLT/Trigger/TrigControl/TrigExamples/TrigExPartialEB
• HLT/Trigger/TrigControl/TrigKernel
• HLT/Trigger/TrigControl/TrigServices
• Trigger/TrigEvent/TrigSteeringEvent
• Trigger/TrigSteer/TrigHLTResultByteStream
• Trigger/TrigSteer/TrigOutputHandling
• Trigger/TrigValidation/TrigP1Test

Adding @fwinkl ,@wiedenma ,@rbielski as watchers

added review-pending-level-1 label

CI Result FAILURE

	Athena	AthSimulation
externals
cmake
make
required tests
optional tests

Full details available at NICOS MR-21931-2019-03-19-02-15
Athena: number of compilation errors 1, warnings 1
AthSimulation: number of compilation errors 0, warnings 2
For experts only: Jenkins output [CI-MERGE-REQUEST 35610]

Jenkins please retry a build

This merge request affects 7 packages:

• HLT/Trigger/TrigControl/TrigExamples/TrigExPartialEB
• HLT/Trigger/TrigControl/TrigKernel
• HLT/Trigger/TrigControl/TrigServices
• Trigger/TrigEvent/TrigSteeringEvent
• Trigger/TrigSteer/TrigHLTResultByteStream
• Trigger/TrigSteer/TrigOutputHandling
• Trigger/TrigValidation/TrigP1Test

Adding @fwinkl ,@wiedenma ,@rbielski as watchers

CI Result SUCCESS

	Athena	AthSimulation
externals
cmake
make
required tests
optional tests

Full details available at NICOS MR-21931-2019-03-19-16-14
Athena: number of compilation errors 0, warnings 1
AthSimulation: number of compilation errors 0, warnings 0
For experts only: Jenkins output [CI-MERGE-REQUEST 35657]

Hi,

this MR looks very nice from the L1 side. Passing to L2 for further review.

Best, Jannik (L1 shifter)

added review-pending-level-2 label and removed review-pending-level-1 label

Changes look fine to me, @rbielski please resolve the open discussion to get this MR approved.

added review-user-action-required label and removed review-pending-level-2 label

added 1 commit

• 6b82d8e0 - Fix aux collection name to serialise

Compare with previous version

This merge request affects 7 packages:

• HLT/Trigger/TrigControl/TrigExamples/TrigExPartialEB
• HLT/Trigger/TrigControl/TrigKernel
• HLT/Trigger/TrigControl/TrigServices
• Trigger/TrigEvent/TrigSteeringEvent
• Trigger/TrigSteer/TrigHLTResultByteStream
• Trigger/TrigSteer/TrigOutputHandling
• Trigger/TrigValidation/TrigP1Test

Adding @fwinkl ,@wiedenma ,@rbielski as watchers

added review-pending-level-1 label and removed review-user-action-required label

resolved all discussions

added review-pending-level-2 label and removed review-pending-level-1 label

Back to @grancagn for final approval

Waiting for the Robot to complete his task before approving, or labels will be reverted back into review.

Thanks Rafal - must have missed that other instance

CI Result SUCCESS

	Athena	AthSimulation
externals
cmake
make
required tests
optional tests

Full details available at NICOS MR-21931-2019-03-21-18-53
Athena: number of compilation errors 0, warnings 1
AthSimulation: number of compilation errors 0, warnings 0
For experts only: Jenkins output [CI-MERGE-REQUEST 35788]

mentioned in merge request !22094 (merged)

Warning seems to be related to !21787 (comment 2458512) and not from this MR.

Approving as agreed.

added review-approved label and removed review-pending-level-2 label

merged

mentioned in commit 1e961818

added sweep:ignore label