Pre-flight: always answer, and record the invariant the check rests on
ProcessPreflight caught JFJochException only. The pusher is blocked on this ACK and reads its absence as a dead writer, so anything else escaping to Run()'s generic handler cost the caller the real reason and handed it a five-second timeout instead - for a check whose entire purpose is to report the problem before the detector is armed. std::filesystem::exists is the concrete leak: it throws filesystem_error, not JFJochException, when the output path cannot be walked at all (a directory component with no search permission, a symlink loop), which is exactly the misconfiguration worth catching early. Now catches std::exception, as JFJochReceiverService::Start does. Also corrects the comment above the write_master_file assignment in TCPStreamPusher::Preflight, which claimed index 0 had to fall on the same connection here as in StartDataCollection. It does not, and cannot be made to: Preflight filters the pool on connected && !broken, while START re-derives its index 0 after DetachDeadConnections, which additionally drops anything failing IsConnectionAlive - so a writer lost in between shifts it. That is harmless, because every writer of a run works on the same filesystem and so answers the same question about the same files. The comment now says that instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016L1qig74oYQzfUJJZbbxFh
This commit is contained in:
@@ -866,9 +866,11 @@ void TCPStreamPusher::Preflight(StartMessage& message) {
|
||||
for (size_t i = 0; i < local_connections.size(); i++) {
|
||||
auto& c = *local_connections[i];
|
||||
|
||||
// Same assignment StartDataCollection makes, and it has to be the same one: only the
|
||||
// writer holding the master file checks the output files, so a different index 0 here
|
||||
// than there would check the run on the wrong writer.
|
||||
// The same assignment StartDataCollection makes, so exactly one writer runs the check. It
|
||||
// does NOT have to fall on the same connection there as here - the pool can lose a writer in
|
||||
// between, and START re-derives index 0 after detaching the dead ones. Every writer of a run
|
||||
// works on the same filesystem, so any of them answers the same question about the same
|
||||
// files; which one is asked does not change the answer.
|
||||
message.socket_number = static_cast<int64_t>(c.socket_number);
|
||||
message.write_master_file = (i == 0);
|
||||
|
||||
|
||||
@@ -99,10 +99,16 @@ void StreamWriter::ProcessPreflight() {
|
||||
run_number = msg.run_number;
|
||||
socket_number = msg.socket_number.value_or(0);
|
||||
|
||||
// std::exception, not JFJochException: the answer matters more than the diagnosis. The pusher is
|
||||
// blocked on this ACK and reads its absence as a dead writer, so anything that escapes here costs
|
||||
// the caller the real reason and hands it a five-second timeout instead. std::filesystem::exists
|
||||
// is the concrete leak - it throws filesystem_error, not JFJochException, when the output path
|
||||
// cannot be walked at all (a directory component with no search permission, a symlink loop),
|
||||
// which is exactly the kind of misconfiguration the pre-flight exists to report before arming.
|
||||
try {
|
||||
FileWriter::Preflight(msg);
|
||||
NotifyTcpAck(TCPFrameType::PREFLIGHT, true, false, TCPAckCode::None);
|
||||
} catch (const JFJochException &e) {
|
||||
} catch (const std::exception &e) {
|
||||
logger.Warning("Pre-flight check failed: {}", e.what());
|
||||
// Not fatal: nothing was started, so the connection stays usable for the next attempt.
|
||||
NotifyTcpAck(TCPFrameType::PREFLIGHT, false, false, TCPAckCode::StartFailed, e.what());
|
||||
|
||||
Reference in New Issue
Block a user