Skip to content

Refactor appsink new buffer

Related to #36 Related to !154 (merged)

I've run 3 tests. One usual injection test, one with short snapshots, and one on gaps. They all crashed at the very end (when gstlal_postcoh_spiir_online dumps stats), as I'd left half of a variable rename in. Should be fixed now and I've relaunched the tests.

Pinged all devs as reviewers since I'm not sure who's the goto for python. Would appreciate any feedback on the style and such @daniel.tang, but no pressure to do the full review.

This is an attempt to refactor appsink_new_buffer, pulling out all its independent functions into ... functions.

Each function has gone through a few refactors, some more than others.

I wanted fapupdater to deal with its own interval rather than tracking it in appsink new buffer, and trying to read through finalsink Generally lead to more changes.

There's a some opportunistic changes which could Technically go in their own MRs, but I'm not sure if it's worth it:

  1. Finalsink now checks clustering & snapshots even in a gap.
    1. This change is somewhat significant, it Could prevent some of the large latencies we were seeing, since large gaps would effectively pause clustering & snapshots. Once the gap ends, we'd be launching all snapshots at once and would potentially have some really old events in memory.
    2. I still think with the refactors it's easier to verify this change. The refactors themselves kinda depend on it too. (it's very tied in with add_segments)
  2. A few logs are altered/touched up.
  3. I've just realised I left some invalid changes in on update_fap_stats. I'll fix it tomorrow. Fixed
  4. IFOMap is now an immutable tuple. (it was a source of paranoia for a recent crash)
Edited by Timothy Davies

Merge request reports