Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make the TPZipper latency < TAZipper latency to avoid tardy sets #101

Merged

Conversation

philiprodrigues
Copy link
Contributor

This PR reduces the latency timeout of the TPZipper to ensure it's less than the timeout for the TAZipper downstream. Reasoning given below. Along with DUNE-DAQ/trigger#133 , this PR solves the "Tardy sets" warnings we've seen with pre-v3.0.0.

The max_latency_ms of the TPZipper should be
kept smaller than the corresponding value in the
downstream TAZipper. The reason is to avoid tardy
sets at run stop, which are caused as follows:

  1. The TPZipper receives its last input TPSets from
    multiple links. In general, the last time received
    from each link will be different (because the
    upstream readout senders don't all stop
    simultaneously). So there will be sets on one link
    that don't have time-matched sets on the other
    links. TPZipper sends these unmatched sets out after
    TPZipper's max_latency_ms milliseconds have passed,
    so these sets are delayed by
    "tpzipper.max_latency_ms"

  2. Meanwhile, the TAZipper has also stopped
    receiving data from all but one of the readout units
    (which are stopped sequentially), and so is in a
    similar situation. Once tazipper.max_latency_ms has
    passed, it sends out the sets from the remaining
    live input, and "catches up" with the current time

So, if tpzipper.max_latency_ms >
tazipper.max_latency_ms, the TA inputs made from the
delayed TPSets will certainly arrive at the TAZipper
after it has caught up to the current time, and be
tardy. If the tpzipper.max_latency_ms ==
tazipper.max_latency_ms, then depending on scheduler
delays etc, the delayed TPSets's TAs may arrive at
the TAZipper tardily. With tpzipper.max_latency_ms <
tazipper.max_latency_ms, everything should be fine.

The max_latency_ms of the TPZipper should be
kept smaller than the corresponding value in the
downstream TAZipper. The reason is to avoid tardy
sets at run stop, which are caused as follows:

1. The TPZipper receives its last input TPSets from
multiple links. In general, the last time received
from each link will be different (because the
upstream readout senders don't all stop
simultaneously). So there will be sets on one link
that don't have time-matched sets on the other
links. TPZipper sends these unmatched sets out after
TPZipper's max_latency_ms milliseconds have passed,
so these sets are delayed by
"tpzipper.max_latency_ms"

2. Meanwhile, the TAZipper has also stopped
receiving data from all but one of the readout units
(which are stopped sequentially), and so is in a
similar situation. Once tazipper.max_latency_ms has
passed, it sends out the sets from the remaining
live input, and "catches up" with the current time

So, if tpzipper.max_latency_ms >
tazipper.max_latency_ms, the TA inputs made from the
delayed TPSets will certainly arrive at the TAZipper
after it has caught up to the current time, and be
tardy. If the tpzipper.max_latency_ms ==
tazipper.max_latency_ms, then depending on scheduler
delays etc, the delayed TPSets's TAs _may_ arrive at
the TAZipper tardily. With tpzipper.max_latency_ms <
tazipper.max_latency_ms, everything should be fine.
Copy link
Contributor

@dingp dingp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dingp dingp marked this pull request as draft June 11, 2022 05:36
@dingp
Copy link
Contributor

dingp commented Jun 11, 2022

Put the PR into draft until we are post-3.0 release cycle.

@philiprodrigues philiprodrigues marked this pull request as ready for review June 14, 2022 14:19
@philiprodrigues philiprodrigues merged commit f30f963 into develop Jun 14, 2022
@philiprodrigues philiprodrigues deleted the philiprodrigues/tpzipper-vs-tazipper-latency branch June 14, 2022 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants