Skip invalid timeframes during ROOT input by autumn-mck · Pull Request #15576 · AliceO2Group/AliceO2

autumn-mck · 2026-07-02T07:56:21Z

Handle corrupt reads as recoverable, discarding the timeframe

ktf · 2026-07-06T17:44:30Z

thank you for your contribution. somehow i did not get notified about this. i will have a look at it tomorrow, time permitting. One comment i already have is that you can probably split the changes to LifetimeHolder as a separate PR.

ktf · 2026-08-03T14:41:25Z

The part about the MessageContext can also be merged separately. Can you spawn a new PR?

jgrosseo · 2026-08-04T07:06:48Z

@ktf Do you have other comments apart from the splitting?

ktf · 2026-08-04T07:16:09Z

I did not look into the details of the rest. I will do once I am back, tomorrow.

ktf · 2026-08-04T07:28:49Z

We should also think how to test this, e.g. making sure it actually does not get stuck or silently skips stuff.

jgrosseo · 2026-08-04T07:41:47Z

We should also think how to test this, e.g. making sure it actually does not get stuck or silently skips stuff.

Once we can enable it by environment variable, we plan to clone some trains which had large number of failures, enable the feature only for those, and do a detailed inspection of the logs and output.

alibuild · 2026-08-04T16:00:56Z

Error while checking build/O2/fullCI_slc9 for 39c7515 at 2026-08-05 03:44:

## sw/BUILD/O2-latest/log
CMake Error in Detectors/Base/CMakeLists.txt:

Full log here.

ktf · 2026-08-06T05:04:27Z

+        if (!treeRead) {
+          if (!first) {
+            LOGP(fatal, "Can not retrieve tree for table {}: fileCounter {}, timeFrame {}", concrete.origin.as<std::string>(), fcnt, ntf);
+            throw std::runtime_error("Processing is stopped!");


Does this actually trigger? I suspect this is old dead code.

My understanding is it can still trigger if a later requested table uses a separate DataInputDescriptor whose file has fewer timeframes than the first table’s file, but for the normal case this is effectively unreachable. Should this be removed?

Handle corrupt reads as recoverable and discard the affected timeframe when DPL_AOD_READER_SKIP_INVALID is enabled.

…iterations

ktf · 2026-08-17T10:18:38Z

+      };
+      auto readState = TFReaderState::READ_FIRST_TABLE;
+      size_t routeIndex = 0;
+      while (readState == TFReaderState::READ_FIRST_TABLE ||


Sorry, but I find this still hard to read / understand. I would have expected the state machine to work as follows:

while(!exitStates(state)) { case STATE: { // Perform operations for state state = NEXT_STATE; } break; case STATE2: { } break; // etc. }

while here it's all a bit intermixed. Am I missing any particular corner case which makes this complicated, hence the current approach?

I've pushed a change now to try clarify and separate this

ktf · 2026-08-17T10:19:44Z

        MetricSpec{.name = "dropped_computations", .metricId = static_cast<short>(ProcessingStatsId::DROPPED_COMPUTATIONS), .kind = Kind::UInt64, .minPublishInterval = quickUpdateInterval},
        MetricSpec{.name = "dropped_incoming_messages", .metricId = static_cast<short>(ProcessingStatsId::DROPPED_INCOMING_MESSAGES), .kind = Kind::UInt64, .minPublishInterval = quickUpdateInterval},
        MetricSpec{.name = "relayed_messages", .metricId = static_cast<short>(ProcessingStatsId::RELAYED_MESSAGES), .kind = Kind::UInt64, .minPublishInterval = quickUpdateInterval},
+        MetricSpec{.name = "aod-invalid-read-skipped-timeframes",


The extra metric can go in even without the rest of the code, if you want.

ktf · 2026-08-17T13:20:41Z

    int level = originLevelMapping.empty() ? -1 : 0;
    auto fileCounter = std::make_shared<int>(0);
    auto numTF = std::make_shared<int>(-1);
+    bool const skipInvalidReads = [] {


This can probably be a static function, no?

Also, didn't you have some more complete logic for it? what happened to it?

There wasn't a more complete logic than this, I've also checked force pushed I made previously to make sure. What way should this better be handled?

ktf · 2026-08-17T13:24:45Z

+  try {
+    f2b->fill(datasetSchema, format);
+  } catch (std::exception const& e) {
+    f2b.discard();


Maybe all the exception handling you are doing should be rewritten using std::experimental::scoped_exit (which our compiler requirements support) and std::throw_nested_exception .

alibuild · 2026-08-17T15:56:43Z

Error while checking build/O2/fullCI_slc9 for 0b77c70 at 2026-08-17 17:56:

## sw/BUILD/O2Physics-latest/log
c++: fatal error: Killed signal terminated program cc1plus
ninja: build stopped: subcommand failed.

Full log here.

jgrosseo · 2026-08-18T09:11:34Z

@ktf We just discussed and think it's best we do the parent file exception implementation later. In a first step we will test this feature only on demand on certain datasets on Hyperloop and we can start testing with non-parent file datasets.

autumn-mck force-pushed the fault-tolerence branch from da4d742 to 404624e Compare July 10, 2026 10:26

autumn-mck force-pushed the fault-tolerence branch from bb69cda to d369614 Compare July 22, 2026 09:09

ktf reviewed Jul 22, 2026

View reviewed changes

Comment thread Framework/Core/include/Framework/DataAllocator.h Outdated

Comment thread Framework/AnalysisSupport/src/TTreePlugin.cxx

autumn-mck force-pushed the fault-tolerence branch from 2f0f392 to 39c7515 Compare August 3, 2026 12:43

autumn-mck marked this pull request as ready for review August 3, 2026 13:30

autumn-mck requested a review from a team as a code owner August 3, 2026 13:30

ktf reviewed Aug 6, 2026

View reviewed changes

Comment thread Framework/AnalysisSupport/src/AODJAlienReaderHelpers.cxx Outdated

Comment thread Framework/AnalysisSupport/src/AODJAlienReaderHelpers.cxx Outdated

autumn-mck added 5 commits August 13, 2026 09:23

Skip invalid timeframes during ROOT input

14146a3

Handle corrupt reads as recoverable and discard the affected timeframe when DPL_AOD_READER_SKIP_INVALID is enabled.

Use DataProcessingStats for skipped metric

8d1e4b2

Better parse skipInvalidReads env var

9bad534

Switch reading booleans to state

f03ffeb

Only checkskipInvalidReads env var once, track state across all loop …

a9a11c7

…iterations

autumn-mck force-pushed the fault-tolerence branch from cac116c to a9a11c7 Compare August 13, 2026 07:25

Track AOD reader state per timeframe rather than table

4601255

ktf reviewed Aug 13, 2026

View reviewed changes

Comment thread Framework/AnalysisSupport/src/AODJAlienReaderHelpers.cxx Outdated

Track timeframe reading progress in reader state machine

4f77b72

ktf reviewed Aug 17, 2026

View reviewed changes

Clarify reader state machine

a76e55b

ktf reviewed Aug 17, 2026

View reviewed changes

Comment thread Framework/AnalysisSupport/src/AODJAlienReaderHelpers.cxx Outdated

ktf reviewed Aug 17, 2026

View reviewed changes

Comment thread Framework/AnalysisSupport/src/AODJAlienReaderHelpers.cxx Outdated

Explicit captures for reader lambdas

0b77c70

ktf reviewed Aug 17, 2026

View reviewed changes

autumn-mck and others added 3 commits August 18, 2026 10:32

Move shouldSkipInvalidReads to static function

ec405f1

Redo reading exception handling with scoped_exit and rethrow_if_nested

5388a43

Please consider the following formatting changes

97a4e24

alibuild mentioned this pull request Aug 18, 2026

Please consider the following formatting changes to #15576 autumn-mck/AliceO2#1

Merged

clang-format from alibuild/alibot-cleanup-15576

b69eeaa

Uh oh!

Conversation

autumn-mck commented Jul 2, 2026

Uh oh!

ktf commented Jul 6, 2026

Uh oh!

Uh oh!

Uh oh!

ktf commented Aug 3, 2026

Uh oh!

jgrosseo commented Aug 4, 2026

Uh oh!

ktf commented Aug 4, 2026

Uh oh!

ktf commented Aug 4, 2026

Uh oh!

jgrosseo commented Aug 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alibuild commented Aug 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ktf Aug 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alibuild commented Aug 17, 2026

Uh oh!

jgrosseo commented Aug 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

jgrosseo commented Aug 4, 2026 •

edited

Loading

alibuild commented Aug 4, 2026 •

edited

Loading

ktf Aug 17, 2026 •

edited

Loading