mx/Jungfraujoch
2
1
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 72 Wiki Activity

Broker: refuse a run the writer cannot write before arming the detector

Browse Source 
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
This commit is contained in:
leonarski_f
2026-08-27 22:09:50 +02:00
committed by leonarski_f
co-authored by Claude Opus 5
parent 749db470ca
commit 41983ca589
24 changed files with 723 additions and 45 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/JFJochStateMachineTest.cpp
+195
View File
@@ -355,3 +355,198 @@ TEST_CASE("JFJochStateMachine_CalibrationFailure") {
REQUIRE(calibrated.GetStatus().state == JFJochState::Idle);
REQUIRE(calibrated.GetStatus().message_severity == BrokerStatus::MessageSeverity::Success);
}
namespace {
// Holds the run open until the test lets go, so a cancel lands while the collection is running
// instead of racing its end.
class GatedPusher : public ImagePusher {
std::atomic<bool> released{true};
public:
void Hold() { released = false; }
void Release() { released = true; }
void StartDataCollection(StartMessage &) override {}
bool EndDataCollection(const EndMessage &) override { return true; }
bool SendImage(const uint8_t *, size_t, int64_t) override {
while (!released)
std::this_thread::sleep_for(1ms);
return true;
}
bool SendCalibration(const CompressedImage &) override { return true; }
std::string PrintSetup() const override { return "GatedPusher"; }
ImagePusherType GetType() const override { return ImagePusherType::Test; }
};
// A writer that cannot finish the run - a rename that collides, a disk that filled up. It
// reaches the state machine from Stop(), not from Start(), which is the case that used to be
// reported as a run without problems.
class FinalizeFailingPusher : public ImagePusher {
public:
void StartDataCollection(StartMessage &) override {}
bool EndDataCollection(const EndMessage &) override { return true; }
bool SendImage(const uint8_t *, size_t, int64_t) override { return true; }
bool SendCalibration(const CompressedImage &) override { return true; }
std::string Finalize() override {
throw JFJochException(JFJochExceptionCategory::FileWriteError, "writer_finalize_failed_9876");
}
std::string PrintSetup() const override { return "FinalizeFailingPusher"; }
ImagePusherType GetType() const override { return ImagePusherType::Test; }
};
}
// /wait_till_done reads the severity of the end-of-run message to decide between 200 and 500, so
// what the state machine puts there is what a script sees. A cancelled collection must not be an
// error: it is incomplete by definition - the packets it never received would otherwise report
// themselves - and calling every cancel a failure would make the endpoint useless.
TEST_CASE("JFJochStateMachine_CancelledRunIsNotAnError") {
Logger logger("JFJochStateMachine_CancelledRunIsNotAnError");
DiffractionExperiment experiment(DetJF(2));
experiment.Conversion().PedestalG0Frames(0).NumTriggers(1).UseInternalPacketGenerator(true)
.ImagesPerTrigger(8).IncidentEnergy_keV(12.4);
AcquisitionDeviceGroup aq_devices;
for (int i = 0; i < experiment.GetDataStreamsNum(); i++)
aq_devices.Add(std::make_unique<HLSSimulatedDevice>(i, 64));
GatedPusher pusher;
JFJochReceiverService receiver_service(aq_devices, logger, pusher);
JFJochServices services(logger);
services.Receiver(&receiver_service);
JFJochStateMachine state_machine(experiment, services, logger);
state_machine.AddDetectorSetup(DetJF(2));
state_machine.DebugOnly_SetState(JFJochState::Idle);
DatasetSettings setup;
setup.ImagesPerTrigger(8).NumTriggers(1);
// A run that finished properly is a Success, so /wait_till_done answers 200.
REQUIRE_NOTHROW(state_machine.Start(setup));
REQUIRE_NOTHROW(state_machine.WaitTillMeasurementDone());
REQUIRE(state_machine.GetStatus().state == JFJochState::Idle);
REQUIRE(state_machine.GetStatus().message_severity == BrokerStatus::MessageSeverity::Success);
// The same run cancelled halfway. The gate keeps it running until the cancel is in.
pusher.Hold();
REQUIRE_NOTHROW(state_machine.Start(setup, true));
for (int i = 0; i < 400 && state_machine.GetStatus().state != JFJochState::Measuring; i++)
std::this_thread::sleep_for(25ms);
REQUIRE(state_machine.GetStatus().state == JFJochState::Measuring);
state_machine.Cancel();
pusher.Release();
REQUIRE_NOTHROW(state_machine.WaitTillMeasurementDone());
auto status = state_machine.GetStatus();
REQUIRE(status.state == JFJochState::Idle);
// Info, not Error and not Warning: the collection did what it was told to do.
REQUIRE(status.message_severity == BrokerStatus::MessageSeverity::Info);
REQUIRE_THAT(status.message.value_or(""), Catch::Matchers::ContainsSubstring("cancelled"));
// The run really was short - what makes this test worth having is that the incompleteness did
// not get reported ahead of the cancel.
REQUIRE(state_machine.GetMeasurementStatistics()->cancelled);
}
// A writer that failed while the run was being finalized arrives from Stop(), which leaves the
// state Idle - the detector is still usable - and says so only in the severity. /wait_till_done
// turns that into a 500 with the writer's message rather than reporting a good collection.
TEST_CASE("JFJochStateMachine_WriterFailureAtEndIsAnError") {
Logger logger("JFJochStateMachine_WriterFailureAtEndIsAnError");
DiffractionExperiment experiment(DetJF(2));
experiment.Conversion().PedestalG0Frames(0).NumTriggers(1).UseInternalPacketGenerator(true)
.ImagesPerTrigger(4).IncidentEnergy_keV(12.4);
AcquisitionDeviceGroup aq_devices;
for (int i = 0; i < experiment.GetDataStreamsNum(); i++)
aq_devices.Add(std::make_unique<HLSSimulatedDevice>(i, 64));
FinalizeFailingPusher pusher;
JFJochReceiverService receiver_service(aq_devices, logger, pusher);
JFJochServices services(logger);
services.Receiver(&receiver_service);
JFJochStateMachine state_machine(experiment, services, logger);
state_machine.AddDetectorSetup(DetJF(2));
state_machine.DebugOnly_SetState(JFJochState::Idle);
DatasetSettings setup;
setup.ImagesPerTrigger(4).NumTriggers(1);
REQUIRE_NOTHROW(state_machine.Start(setup, true));
REQUIRE_NOTHROW(state_machine.WaitTillMeasurementDone());
auto status = state_machine.GetStatus();
REQUIRE(status.state == JFJochState::Idle);
REQUIRE(status.message_severity == BrokerStatus::MessageSeverity::Error);
REQUIRE_THAT(status.message.value_or(""),
Catch::Matchers::ContainsSubstring("writer_finalize_failed_9876"));
}
namespace {
// Refuses the run before it starts, the way a writer refuses to overwrite a file that is
// already there. Records whether the collection was started anyway.
class PreflightRefusingPusher : public ImagePusher {
public:
std::atomic<bool> start_called{false};
void Preflight(StartMessage &) override {
throw JFJochException(JFJochExceptionCategory::FileWriteError, "preflight_refused_2468");
}
void StartDataCollection(StartMessage &) override { start_called = true; }
bool EndDataCollection(const EndMessage &) override { return true; }
bool SendImage(const uint8_t *, size_t, int64_t) override { return true; }
bool SendCalibration(const CompressedImage &) override { return true; }
std::string PrintSetup() const override { return "PreflightRefusingPusher"; }
ImagePusherType GetType() const override { return ImagePusherType::Test; }
};
}
// The pre-flight is the first thing a measurement does, so a run the writer will not accept is
// refused before the receiver is built and before the detector is armed - which is what makes the
// refusal free: there is no series left armed and waiting for a trigger to tear down.
TEST_CASE("JFJochStateMachine_PreflightRefusesBeforeStarting") {
Logger logger("JFJochStateMachine_PreflightRefusesBeforeStarting");
DiffractionExperiment experiment(DetJF(2));
experiment.Conversion().PedestalG0Frames(0).NumTriggers(1).UseInternalPacketGenerator(true)
.ImagesPerTrigger(4).IncidentEnergy_keV(12.4).FilePrefix("preflight_refused");
AcquisitionDeviceGroup aq_devices;
for (int i = 0; i < experiment.GetDataStreamsNum(); i++)
aq_devices.Add(std::make_unique<HLSSimulatedDevice>(i, 64));
PreflightRefusingPusher pusher;
JFJochReceiverService receiver_service(aq_devices, logger, pusher);
JFJochServices services(logger);
services.Receiver(&receiver_service);
JFJochStateMachine state_machine(experiment, services, logger);
state_machine.AddDetectorSetup(DetJF(2));
state_machine.DebugOnly_SetState(JFJochState::Idle);
DatasetSettings setup;
setup.ImagesPerTrigger(4).NumTriggers(1).FilePrefix("preflight_refused");
// A synchronous start reports the writer's own message.
REQUIRE_THROWS_WITH(state_machine.Start(setup),
Catch::Matchers::ContainsSubstring("preflight_refused_2468"));
REQUIRE_FALSE(pusher.start_called);
// An ordinary failure, so the detector stays usable and the run can be retried from Idle.
auto status = state_machine.GetStatus();
REQUIRE(status.state == JFJochState::Idle);
REQUIRE(status.message_severity == BrokerStatus::MessageSeverity::Error);
// The asynchronous start returns, and the wait functions report it.
REQUIRE_NOTHROW(state_machine.Start(setup, true));
REQUIRE_THROWS_WITH(state_machine.WaitTillNotBusy(10s),
Catch::Matchers::ContainsSubstring("preflight_refused_2468"));
REQUIRE_THROWS_WITH(state_machine.WaitTillMeasurementDone(10s),
Catch::Matchers::ContainsSubstring("preflight_refused_2468"));
REQUIRE_FALSE(pusher.start_called);
}