Skip invalid timeframes during ROOT input - #15576
Conversation
|
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. |
da4d742 to
404624e
Compare
bb69cda to
d369614
Compare
Handle corrupt reads as recoverable and discard the affected timeframe when DPL_AOD_READER_SKIP_INVALID is enabled.
2f0f392 to
39c7515
Compare
|
The part about the MessageContext can also be merged separately. Can you spawn a new PR? |
|
@ktf Do you have other comments apart from the splitting? |
|
I did not look into the details of the rest. I will do once I am back, tomorrow. |
|
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. |
| 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!"); |
There was a problem hiding this comment.
Does this actually trigger? I suspect this is old dead code.
| static uint64_t totalDFSent = 0; | ||
| static uint64_t totalInvalidReadSkipped = 0; | ||
| static bool skipInvalidReads = [] { | ||
| auto const* envValue = getenv("DPL_AOD_READER_SKIP_INVALID"); |
There was a problem hiding this comment.
How about doing the check only once in the initialization of the callback and capture the result in the lambda?
|
|
||
| if (!didir->readTree(outputs, dh, fcnt, ntf, totalSizeCompressed, totalSizeUncompressed, wasAOD)) { | ||
| if (first) { | ||
| enum class ReadState { |
There was a problem hiding this comment.
That's not what I meant, sorry for not explaining me. What I was thinking is that it would make sense to have the state tracked across all the loop iterations, not merely to collapse two variables into one. Does it make sense?
4 participants
Handle corrupt reads as recoverable, discarding the timeframe