A writer refusing to overwrite an existing file was reported badly, and the
earlier fix only covered half of it. The commit message of a6a715703 states the
assumption that broke: "SendStartMessage precedes every std::async in the
receiver constructor". That is true of JFJochReceiverFPGA, where the start
message goes out from the constructor body, and false of JFJochReceiverLite,
where it goes out from MeasurementThread - launched by std::async from the
constructor, which waits only on data_analysis_started. On the DECTRIS path the
writer's refusal therefore cannot reach services.Start(); it arrives from
services.Stop(), i.e. the second try-block of the state machine's
MeasurementThread, which never touched start_exception. /wait_till_done answered
200 for a run that never wrote anything.
Two changes, in front of the problem and behind it.
In front: a PREFLIGHT frame (TCP protocol version 3 -> 4), sent to every writer
before the receiver is built and before the detector is armed. It carries the
same CBOR start message a START does and asks one question: could this run be
written? The writer runs FileWriter::Preflight - the first three statements of
the FileWriter constructor, stopping short of the only one that creates
anything - and answers with an ACK. Shared code, so a pre-flight that passes
cannot be contradicted by the start behind it. A refusal is not fatal: nothing
was started, the connection stays usable, and above all no series is left armed
waiting for a trigger that will not come. Both ends already reject a frame of
another protocol version, so the bump costs no compatibility.
The check now covers the data files, not only the master. A stale
_data_000001.h5 used to sail through and fail when the file was renamed into
place at the end of the run, after the whole dataset had been collected. The
writer holding the master file checks for all of them - they share a directory
by construction, the master linking them by relative name - so the siblings do
not race each other statting files the others are creating.
Behind: /wait_till_done maps an Idle state to 200 or 500 by the severity of the
end-of-run message, so a collection that lost packets, or whose writer could not
write, is no longer indistinguishable from a good one to a script. A cancelled
collection is not affected - MeasurementThread checks the cancel before it looks
at the completeness of the data, and reports it with Info severity - and a
warning is still a 200. start_exception is untouched.
The PREFLIGHT payload is built from FillMessage alone, which is what keeps it
small: the per-pixel arrays are added by SendStartMessage and are not in it.
Measured on a JUNGFRAU 9M, 1.4 kB against 543 kB for the start message. It is
deliberately the whole of FillMessage rather than the fields the check reads
today - a hand-picked subset would answer for a different set of file names the
day the check learns to read one more field, and would do it silently.
Three defects found on the way:
TCPImagePuller republished a PREFLIGHT as a real start message, announcing to
downstream consumers a run that was not going to happen.
TCPStreamPusher::run_number was a plain uint64_t written by the control thread
and read by every ACK thread; Preflight adds a second writer to it.
StreamWriter::CollectImages reprocessed the last frame whenever WaitForImage
returned false - run is only tested at the top of the loop - so an abort wrote
the last image a second time. Pre-existing; the new branch made it visible.
Verified against a running broker on the local configuration: a colliding master
file and a colliding data file are both refused by /start, /wait_until_running
and /wait_till_done with the writer's own message, and the log shows the receiver
was never started for either; a cancelled collection still answers 200.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016WmryXe8ASbNi632sUMfsa
209 lines
9.6 KiB
C++
209 lines
9.6 KiB
C++
// SPDX-FileCopyrightText: 2025 Filip Leonarski, Paul Scherrer Institute <filip.leonarski@psi.ch>
|
|
// SPDX-License-Identifier: GPL-3.0-only
|
|
|
|
#pragma once
|
|
|
|
#include <atomic>
|
|
#include <future>
|
|
#include <mutex>
|
|
#include <optional>
|
|
#include <string>
|
|
#include <vector>
|
|
#include <condition_variable>
|
|
|
|
#include "ImagePusher.h"
|
|
#include "ZMQWriterNotificationPuller.h"
|
|
#include "../common/ThreadSafeFIFO.h"
|
|
#include "../common/Logger.h"
|
|
#include "../common/JfjochTCP.h"
|
|
#include "../frame_serialize/CBORStream2Serializer.h"
|
|
|
|
/// TCP-based image stream pusher with persistent connection pool.
|
|
///
|
|
/// Threading model:
|
|
/// - AcceptorThread: accepts new TCP connections, holds connections_mutex briefly
|
|
/// - KeepaliveThread: sends periodic keepalive frames when idle (skipped during data collection)
|
|
/// - Per-connection WriterThread: drains the connection's queue, sends DATA frames
|
|
/// - Per-connection PersistentAckThread: reads ACKs and keepalive pongs from the peer
|
|
///
|
|
/// Lock ordering: connections_mutex → send_mutex → ack_mutex
|
|
/// IMPORTANT: Never call blocking queue operations while holding connections_mutex.
|
|
///
|
|
/// Concurrency contract:
|
|
/// - StartDataCollection, EndDataCollection, SendCalibration, and Finalize
|
|
/// are called from a single control thread in a serialized manner.
|
|
/// - SendImage may be called concurrently from multiple threads between
|
|
/// StartDataCollection and EndDataCollection.
|
|
/// - SendCalibration is called between StartDataCollection and SendImage calls.
|
|
/// - GetConnectedWriters, GetImagesWritten, and PrintSetup are safe to call at any time.
|
|
|
|
class TCPStreamPusher : public ImagePusher {
|
|
struct Connection {
|
|
explicit Connection(size_t queue_size) : queue(queue_size) {}
|
|
|
|
std::atomic<int> fd{-1};
|
|
uint32_t socket_number = 0;
|
|
std::atomic<bool> active{false}; // data-collection threads running
|
|
std::atomic<bool> broken{false};
|
|
std::atomic<bool> connected{false}; // persistent connection is alive
|
|
|
|
ThreadSafeFIFO<ImagePusherQueueElement> queue;
|
|
std::future<void> writer_future;
|
|
|
|
// Persistent ack/keepalive reader (runs as long as the connection is alive)
|
|
std::future<void> persistent_ack_future;
|
|
|
|
// Serialises tearing this connection down. Both futures below are joined from more than one
|
|
// path - the acceptor reaping a dead connection, and the control plane starting or ending a
|
|
// run - and calling get() on one future from two threads is undefined and invalidates it.
|
|
// Held only around the joins, never around a send, and no thread it joins takes it.
|
|
std::mutex teardown_mutex;
|
|
|
|
std::mutex send_mutex;
|
|
std::mutex ack_mutex;
|
|
std::condition_variable ack_cv;
|
|
|
|
bool preflight_ack_received = false;
|
|
bool preflight_ack_ok = false;
|
|
bool start_ack_received = false;
|
|
bool start_ack_ok = false;
|
|
bool end_ack_received = false;
|
|
bool end_ack_ok = false;
|
|
bool cancel_ack_received = false;
|
|
bool cancel_ack_ok = false;
|
|
|
|
std::string last_ack_error;
|
|
std::atomic<TCPAckCode> last_ack_code{TCPAckCode::None};
|
|
|
|
// Soft writer failure reported via DATA ACK (do not break stream on this alone)
|
|
std::atomic<bool> data_ack_error_reported{false};
|
|
std::string data_ack_error_text;
|
|
|
|
std::atomic<uint64_t> data_acked_ok{0};
|
|
std::atomic<uint64_t> data_acked_bad{0};
|
|
std::atomic<uint64_t> data_acked_total{0};
|
|
std::atomic<uint64_t> last_ack_fifo_occupancy{0};
|
|
|
|
std::chrono::steady_clock::time_point last_keepalive_sent{};
|
|
std::chrono::steady_clock::time_point last_keepalive_recv{};
|
|
|
|
// Last time ANY frame (ACK / keepalive pong / busy heartbeat) was received from
|
|
// the peer, as steady_clock nanoseconds. Used to keep a healthy-but-busy writer
|
|
// alive through long backpressure while still detecting a genuinely dead peer.
|
|
std::atomic<int64_t> last_peer_activity_ns{0};
|
|
};
|
|
|
|
std::string endpoint;
|
|
size_t max_connections;
|
|
std::optional<int32_t> send_buffer_size;
|
|
size_t send_queue_size = 128;
|
|
|
|
// Persistent connection pool, guarded by connections_mutex.
|
|
// IMPORTANT: never call PutBlocking/GetBlocking on a queue while holding this mutex.
|
|
mutable std::mutex connections_mutex;
|
|
std::vector<std::shared_ptr<Connection>> connections;
|
|
std::vector<std::shared_ptr<Connection>> session_connections;
|
|
std::shared_ptr<Connection> calibration_connection;
|
|
|
|
// Acceptor thread state
|
|
std::atomic<int> listen_fd{-1};
|
|
std::atomic<bool> acceptor_running{false};
|
|
std::future<void> acceptor_future;
|
|
std::future<void> keepalive_future;
|
|
|
|
std::chrono::milliseconds send_poll_timeout{250};
|
|
// Maximum time a send (or the post-END ACK wait) may block with NO sign of life from
|
|
// the peer before the connection is declared dead. A busy writer refreshes its liveness
|
|
// every ~250 ms via BUSY heartbeats (and via DATA ACKs), so genuine backpressure of any
|
|
// duration is tolerated; only a truly silent (frozen/dead) peer trips this.
|
|
std::chrono::milliseconds peer_liveness_timeout{15000};
|
|
// Hard upper bound on backpressure: if the socket accepts no bytes for this long the
|
|
// writer is wedged and is declared dead even if it keeps heartbeating, so a
|
|
// misbehaving writer cannot block the run (or its finalization) forever. Generous
|
|
// relative to peer_liveness_timeout, since a heartbeating peer is given more grace
|
|
// than a silent one — but still finite.
|
|
std::chrono::milliseconds max_backpressure_timeout{60000};
|
|
|
|
int64_t images_per_file = 1;
|
|
// Written by the control thread (Preflight/StartDataCollection), read by every
|
|
// PersistentAckThread to discard ACKs belonging to an earlier run.
|
|
std::atomic<uint64_t> run_number{0};
|
|
std::string run_name;
|
|
std::atomic<bool> transmission_error = false;
|
|
std::atomic<bool> data_collection_active{false};
|
|
|
|
std::atomic<uint64_t> total_data_acked_ok{0};
|
|
std::atomic<uint64_t> total_data_acked_bad{0};
|
|
std::atomic<uint64_t> total_data_acked_total{0};
|
|
|
|
Logger logger{"TCPStreamPusher"};
|
|
|
|
static std::pair<std::string, std::optional<uint16_t>> ParseTcpAddress(const std::string& addr);
|
|
static std::pair<int, std::string> OpenListenSocket(const std::string& addr);
|
|
static int AcceptOne(int listen_fd, std::chrono::milliseconds timeout);
|
|
|
|
static void CloseFd(std::atomic<int>& fd);
|
|
bool IsConnectionAlive(const Connection& c) const;
|
|
bool SendAll(Connection& c, const void* buf, size_t len);
|
|
bool ReadExact(Connection& c, void* buf, size_t len);
|
|
bool ReadExactPersistent(Connection& c, void* buf, size_t len);
|
|
bool SendFrame(Connection& c, const uint8_t* data, size_t size, TCPFrameType type, int64_t image_number);
|
|
|
|
void WriterThread(Connection* c);
|
|
void PersistentAckThread(Connection* c);
|
|
void AcceptorThread();
|
|
void KeepaliveThread();
|
|
|
|
void SetupNewConnection(int new_fd, uint32_t socket_number);
|
|
// Unlink dead connections from the pool (connections_mutex held) and close them (mutex released).
|
|
// Split because closing joins a writer thread that can be blocked in a send.
|
|
std::vector<std::shared_ptr<Connection>> DetachDeadConnections();
|
|
void CloseDeadConnections(const std::vector<std::shared_ptr<Connection>> &dead);
|
|
void TearDownConnection(Connection& c);
|
|
|
|
void StartDataCollectionThreads(Connection& c);
|
|
void StopDataCollectionThreads(Connection& c);
|
|
void JoinPersistentAck(Connection& c);
|
|
|
|
bool WaitForAck(Connection& c, TCPFrameType ack_for, std::chrono::milliseconds timeout, std::string* error_text);
|
|
bool WaitForEndAck(Connection& c, std::chrono::milliseconds liveness_timeout, std::string* error_text);
|
|
public:
|
|
explicit TCPStreamPusher(const std::string& addr,
|
|
size_t in_max_connections,
|
|
std::optional<int32_t> in_send_buffer_size = {});
|
|
|
|
~TCPStreamPusher() override;
|
|
|
|
/// Max time a send may block on backpressure with no sign of life from the peer
|
|
/// before the connection is declared dead. A busy-but-alive writer keeps it fresh
|
|
/// via BUSY heartbeats, so this only catches a genuinely silent peer. Must be set
|
|
/// before data collection starts.
|
|
void SetPeerLivenessTimeout(std::chrono::milliseconds t) { peer_liveness_timeout = t; }
|
|
|
|
/// Hard upper bound on backpressure. Even while the peer keeps heartbeating, if no
|
|
/// bytes can be sent for this long the writer is declared dead so a wedged writer
|
|
/// cannot block the run or its finalization forever. Must be set before data
|
|
/// collection starts.
|
|
void SetMaxBackpressureTimeout(std::chrono::milliseconds t) { max_backpressure_timeout = t; }
|
|
|
|
std::vector<std::string> GetAddress() const override { return {endpoint}; }
|
|
|
|
/// Returns the number of currently connected writers (can be called at any time)
|
|
size_t GetConnectedWriters() const override;
|
|
|
|
void Preflight(StartMessage& message) override;
|
|
void StartDataCollection(StartMessage& message) override;
|
|
bool EndDataCollection(const EndMessage& message) override;
|
|
bool SendImage(const uint8_t *image_data, size_t image_size, int64_t image_number) override;
|
|
bool SendImage(ZeroCopyReturnValue &z) override;
|
|
bool SendCalibration(const CompressedImage& message) override;
|
|
|
|
std::string Finalize() override;
|
|
std::string PrintSetup() const override;
|
|
|
|
std::optional<uint64_t> GetImagesWritten() const override;
|
|
std::optional<uint64_t> GetImagesWriteError() const override;
|
|
std::vector<int64_t> GetWriterFifoUtilization() const override;
|
|
ImagePusherType GetType() const override { return ImagePusherType::TCP; }
|
|
};
|