Skip to content

Make sure Mergers always use Lifetime::Sporadic#12377

Merged
martenole merged 1 commit into
AliceO2Group:devfrom
knopers8:mergers-sporadic
Dec 4, 2023
Merged

Make sure Mergers always use Lifetime::Sporadic#12377
martenole merged 1 commit into
AliceO2Group:devfrom
knopers8:mergers-sporadic

Conversation

@knopers8
Copy link
Copy Markdown
Collaborator

@knopers8 knopers8 commented Dec 1, 2023

No description provided.

@knopers8 knopers8 requested a review from Barthelemy as a code owner December 1, 2023 07:21
@davidrohr
Copy link
Copy Markdown
Collaborator

just one comment: we need to be a bit carefull here. If you expect lifetime data, it should not be marker as sporadic.
Otherwise, if data arrives from different sources, DPL will not wait for all data to arrive, but once all timeframe data is there (i.e. immediately when everything is sporadic), start processing even if some of the sporadic inputs from other sources are missing. You will also not get any warning / error if expected sporadic inputs are missing.

This might be improved in the future, but right now, sporadic must not be used if inputs coming from different channels for the same tf are to be merged, as in that case only the input that arrives first is taken.

Anyway, it applies only to consumeWhenAll completion policy, for consumeWhenAny it doesn't matter anyway. Not sure what the mergers use.

@knopers8
Copy link
Copy Markdown
Collaborator Author

knopers8 commented Dec 1, 2023

Mergers is consumeWhenAny, so it does not wait for a complete slice. It cannot really count on it, since different QC tasks publish at different times and the timeslice id is the timestamp of the timer (AFAIK).

So I assume we are safe here.

@martenole martenole merged commit e0b472b into AliceO2Group:dev Dec 4, 2023
andreasmolander pushed a commit to andreasmolander/AliceO2 that referenced this pull request Apr 12, 2024
mwinn2 pushed a commit to mwinn2/AliceO2 that referenced this pull request Apr 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants