fix(udps): stop packets being dated from an earlier time base
Reported as samples sporadically carrying a previous packet's timestamp: holes on one side of the stream and collisions on the other, in both the Go and the MARTe2 receiver. That it appeared in both is what located it -- the shared cause is upstream of either client. Four independent defects, all of which end in a packet's values being placed at a time that is not theirs. Reassembly slot exhaustion (the "Reassembly slots full; evicting oldest" flood). Chunk size was learnt only from fragment 0, so an out-of-order burst destroyed a packet whose bytes had all arrived and left the slot occupied until the 2 s GC. Slots were keyed on the counter alone, but DATA and CONFIG number independently, so equal counters merged the two streams. The 32-byte received-mask covered 256 of the 512 fragments the client accepts, so a duplicate above 255 was counted as new and the packet was delivered with a hole of stale bytes in it. And one datagram was read per Execute(), which cannot drain a fast producer. Fixed with a pendingTail deferral, (counter, type) keying, a 64-byte mask, a 256-datagram drain, counter-age slot reclamation, and a 1 Hz aggregated warning in place of the per-eviction flood. UDPStreamer dropping whole Accumulate batches. EventSem::ResetWait is Reset-then-Wait, so a Post() landing while the sender thread was inside ServiceClients()/SendData() was destroyed by the next Reset. The batch was then skipped with dataReady false, readyFill was never cleared, and the following flush overwrote it: an entire run of RT cycles never reached the wire. The record of pending work now lives in the buffers rather than in the semaphore edge, which also removes up to UDPS_DATA_WAIT_MS of latency; genuine backpressure overwrites are counted and reported. Against the unfixed code the new test sees 2999/3000 batches never consumed. Period inflation after loss. Accumulated scalars carry no SamplingRate, so the receiver derives dt from the sender-clock gap -- but dividing it by the previous packet's sample count is only right while nothing is lost. One loss doubles the reported period, which spreads a batch a full batch past its own end and into the range the next packet claims. That is the hole and the collision, exactly. Inferring the cycle count from the estimate's own period is not a way out: it has a stable fixed point wherever gap/dt is an integer, so a real rate change locks it at the old one for good (AccumDtGTest.FollowsSustainedRateChange). The packet counter removes the ambiguity, so all three receivers now order on it: a DATA packet that does not advance the counter is dropped rather than delivered, because its values are older than data already handed over. Ordering is on the signed difference so it survives the uint32 wrap, and the sequence resets on reconnect, where the producer's counter restarts independently of ours. The loss count that falls out of the same delta feeds the period estimate as cycles = prevN * (1 + lost), which reduces exactly to gap/prevN when nothing is lost and therefore still tracks a genuine rate change. UDPSClient::AcceptDataCounter (C++), udpsprotocol.SequenceGate (Go), decode_data (C). The C client's existing gap counter was wrap-unsafe and let a stale packet rewind last_counter, which made every subsequent gap wrong; it uses the same code now. Docs/Protocol.md gains an Ordering DATA section stating the requirement for any receiver, including ones outside this repository. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.6
parent
f334995865
commit
fbae7d712c
@@ -107,6 +107,9 @@ UDPStreamer::UDPStreamer()
|
||||
readyTimestamps = NULL_PTR(uint64 *);
|
||||
scratchTimestamps = NULL_PTR(uint64 *);
|
||||
readyFill = 0u;
|
||||
readySnapshotPending = false;
|
||||
droppedPublications = 0u;
|
||||
lastDropReportTicks = 0u;
|
||||
decimateRatio = 1u;
|
||||
decimateCounter = 0u;
|
||||
|
||||
@@ -871,6 +874,11 @@ bool UDPStreamer::Synchronise() {
|
||||
/* HI-3: if accumFill reached maxBatchCount, force-flush before writing */
|
||||
if (accumFill >= maxBatchCount) {
|
||||
uint32 filled = accumFill;
|
||||
if (readyFill > 0u) {
|
||||
/* The sender has not taken the previous batch: it is about to be
|
||||
* overwritten and its cycles will never reach any receiver. */
|
||||
droppedPublications++;
|
||||
}
|
||||
(void)MemoryOperationsHelper::Copy(readyBuffer, accumBuffer,
|
||||
filled * totalSrcBytes);
|
||||
(void)MemoryOperationsHelper::Copy(
|
||||
@@ -901,6 +909,9 @@ bool UDPStreamer::Synchronise() {
|
||||
|
||||
if (sizeCondition || timeCondition) {
|
||||
bufMutex.FastLock(TTInfiniteWait);
|
||||
if (readyFill > 0u) {
|
||||
droppedPublications++;
|
||||
}
|
||||
(void)MemoryOperationsHelper::Copy(readyBuffer, accumBuffer,
|
||||
filled * totalSrcBytes);
|
||||
(void)MemoryOperationsHelper::Copy(
|
||||
@@ -922,16 +933,24 @@ bool UDPStreamer::Synchronise() {
|
||||
if (decimateCounter >= decimateRatio) {
|
||||
decimateCounter = 0u;
|
||||
bufMutex.FastLock(TTInfiniteWait);
|
||||
if (readySnapshotPending) {
|
||||
droppedPublications++;
|
||||
}
|
||||
(void)MemoryOperationsHelper::Copy(readyBuffer, memory, totalSrcBytes);
|
||||
syncTimestamp = ts;
|
||||
readySnapshotPending = true;
|
||||
bufMutex.FastUnLock();
|
||||
(void)dataSem.Post();
|
||||
}
|
||||
} else {
|
||||
/* --- Strict path: post every call --- */
|
||||
bufMutex.FastLock(TTInfiniteWait);
|
||||
if (readySnapshotPending) {
|
||||
droppedPublications++;
|
||||
}
|
||||
(void)MemoryOperationsHelper::Copy(readyBuffer, memory, totalSrcBytes);
|
||||
syncTimestamp = ts;
|
||||
readySnapshotPending = true;
|
||||
bufMutex.FastUnLock();
|
||||
(void)dataSem.Post();
|
||||
}
|
||||
@@ -955,60 +974,71 @@ ErrorManagement::ErrorType UDPStreamer::Execute(ExecutionInfo &info) {
|
||||
}
|
||||
|
||||
if (info.GetStage() == ExecutionInfo::MainStage) {
|
||||
/* --- Wait for RT thread to post new data ---
|
||||
* ResetWait sleeps the background thread until the RT thread calls
|
||||
* Synchronise() and posts dataSem, or until the timeout expires.
|
||||
* Doing this FIRST means the thread spends nearly all its time here
|
||||
* instead of spinning on the non-blocking select() below.
|
||||
* Command latency is bounded by UDPS_DATA_WAIT_MS (acceptable for
|
||||
* CONNECT / DISCONNECT). */
|
||||
ErrorManagement::ErrorType waitErr =
|
||||
dataSem.ResetWait(TimeoutType(UDPS_DATA_WAIT_MS));
|
||||
bool dataReady = (waitErr == ErrorManagement::NoError);
|
||||
/* --- Wait for the RT thread to publish new data ---
|
||||
* dataSem is only a wake-up hint, never the record of pending work:
|
||||
* EventSem::ResetWait resets the semaphore before waiting, so a Post that
|
||||
* landed while this thread was inside ServiceClients()/SendData() is
|
||||
* destroyed by the next Reset. Deciding what to send from the wait result
|
||||
* would then skip that publication entirely, and the next flush would
|
||||
* overwrite it — the receiver sees the batch's whole time span missing.
|
||||
* The buffers therefore carry the state, and are only waited on when they
|
||||
* are empty (which also avoids paying the wait when work is already
|
||||
* queued). */
|
||||
if (!HasPendingPublication()) {
|
||||
(void)dataSem.ResetWait(TimeoutType(UDPS_DATA_WAIT_MS));
|
||||
}
|
||||
|
||||
/* --- Poll for incoming control commands (CONNECT / DISCONNECT / ACK) ---
|
||||
*/
|
||||
server.ServiceClients();
|
||||
|
||||
if (dataReady && server.HasClients()) {
|
||||
/* Synchronise() already gates posting dataSem to the correct rate
|
||||
* (size/time for Accumulate, every-Nth for Decimate, every call for
|
||||
* Strict). Execute() just sends whatever is in the ready buffers. */
|
||||
if (publishMode == UDPStreamerPublishAccumulate) {
|
||||
/* --- Accumulate batch send --- */
|
||||
uint32 fill = 0u;
|
||||
bufMutex.FastLock(TTInfiniteWait);
|
||||
fill = readyFill;
|
||||
if (fill > 0u) {
|
||||
(void)MemoryOperationsHelper::Copy(scratchBuffer, readyBuffer,
|
||||
fill * totalSrcBytes);
|
||||
(void)MemoryOperationsHelper::Copy(
|
||||
reinterpret_cast<uint8 *>(scratchTimestamps),
|
||||
reinterpret_cast<const uint8 *>(readyTimestamps),
|
||||
fill * static_cast<uint32>(sizeof(uint64)));
|
||||
}
|
||||
bufMutex.FastUnLock();
|
||||
/* Synchronise() already gates publication to the correct rate (size/time
|
||||
* for Accumulate, every-Nth for Decimate, every call for Strict). The
|
||||
* pending publication is consumed whether or not anyone is listening, so
|
||||
* that a client-less streamer neither spins here nor delivers a stale
|
||||
* snapshot to the next client that connects. */
|
||||
if (publishMode == UDPStreamerPublishAccumulate) {
|
||||
/* --- Accumulate batch send --- */
|
||||
uint32 fill = 0u;
|
||||
bufMutex.FastLock(TTInfiniteWait);
|
||||
fill = readyFill;
|
||||
if (fill > 0u) {
|
||||
(void)MemoryOperationsHelper::Copy(scratchBuffer, readyBuffer,
|
||||
fill * totalSrcBytes);
|
||||
(void)MemoryOperationsHelper::Copy(
|
||||
reinterpret_cast<uint8 *>(scratchTimestamps),
|
||||
reinterpret_cast<const uint8 *>(readyTimestamps),
|
||||
fill * static_cast<uint32>(sizeof(uint64)));
|
||||
readyFill = 0u;
|
||||
}
|
||||
bufMutex.FastUnLock();
|
||||
|
||||
if (fill > 0u) {
|
||||
SerializeAccumulated(scratchBuffer, scratchTimestamps, fill);
|
||||
uint32 sendBytes =
|
||||
UDPS_TIMESTAMP_BYTES + 4u + fill * singleCycleWireBytes;
|
||||
packetCounter++;
|
||||
if (!server.SendData(packetCounter, wireBuffer, sendBytes)) {
|
||||
REPORT_ERROR(ErrorManagement::Warning,
|
||||
"Failed to send Accumulate DATA packet (counter=%u).",
|
||||
packetCounter);
|
||||
}
|
||||
if ((fill > 0u) && server.HasClients()) {
|
||||
SerializeAccumulated(scratchBuffer, scratchTimestamps, fill);
|
||||
uint32 sendBytes =
|
||||
UDPS_TIMESTAMP_BYTES + 4u + fill * singleCycleWireBytes;
|
||||
packetCounter++;
|
||||
if (!server.SendData(packetCounter, wireBuffer, sendBytes)) {
|
||||
REPORT_ERROR(ErrorManagement::Warning,
|
||||
"Failed to send Accumulate DATA packet (counter=%u).",
|
||||
packetCounter);
|
||||
}
|
||||
} else {
|
||||
/* --- Single-snapshot send (Strict or Decimate) --- */
|
||||
uint64 ts = 0u;
|
||||
bufMutex.FastLock(TTInfiniteWait);
|
||||
}
|
||||
} else {
|
||||
/* --- Single-snapshot send (Strict or Decimate) --- */
|
||||
uint64 ts = 0u;
|
||||
bool pending = false;
|
||||
bufMutex.FastLock(TTInfiniteWait);
|
||||
pending = readySnapshotPending;
|
||||
if (pending) {
|
||||
(void)MemoryOperationsHelper::Copy(scratchBuffer, readyBuffer,
|
||||
totalSrcBytes);
|
||||
ts = syncTimestamp;
|
||||
bufMutex.FastUnLock();
|
||||
readySnapshotPending = false;
|
||||
}
|
||||
bufMutex.FastUnLock();
|
||||
|
||||
if (pending && server.HasClients()) {
|
||||
QuantizeAndSerialize(scratchBuffer, ts);
|
||||
|
||||
packetCounter++;
|
||||
@@ -1019,6 +1049,8 @@ ErrorManagement::ErrorType UDPStreamer::Execute(ExecutionInfo &info) {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
ReportDroppedPublications();
|
||||
}
|
||||
|
||||
if (info.GetStage() == ExecutionInfo::TerminationStage) {
|
||||
@@ -1327,6 +1359,36 @@ bool UDPStreamer::IsClientConnected() const { return server.HasClients(); }
|
||||
|
||||
bool UDPStreamer::IsMulticast() const { return server.IsMulticast(); }
|
||||
|
||||
uint32 UDPStreamer::GetDroppedPublications() const { return droppedPublications; }
|
||||
|
||||
bool UDPStreamer::HasPendingPublication() {
|
||||
bool pending = false;
|
||||
bufMutex.FastLock(TTInfiniteWait);
|
||||
pending = (readyFill > 0u) || readySnapshotPending;
|
||||
bufMutex.FastUnLock();
|
||||
return pending;
|
||||
}
|
||||
|
||||
void UDPStreamer::ReportDroppedPublications() {
|
||||
uint64 now = HighResolutionTimer::Counter();
|
||||
if ((now - lastDropReportTicks) < HighResolutionTimer::Frequency()) {
|
||||
return;
|
||||
}
|
||||
lastDropReportTicks = now;
|
||||
|
||||
uint32 dropped = 0u;
|
||||
bufMutex.FastLock(TTInfiniteWait);
|
||||
dropped = droppedPublications;
|
||||
bufMutex.FastUnLock();
|
||||
|
||||
if (dropped > 0u) {
|
||||
REPORT_ERROR(ErrorManagement::Warning,
|
||||
"Dropped %u unsent publication(s) so far: the sender thread is "
|
||||
"not keeping up with the RT cycle.",
|
||||
dropped);
|
||||
}
|
||||
}
|
||||
|
||||
CLASS_REGISTER(UDPStreamer, "1.0")
|
||||
|
||||
} /* namespace MARTe */
|
||||
|
||||
@@ -322,6 +322,17 @@ public:
|
||||
*/
|
||||
bool IsMulticast() const;
|
||||
|
||||
/**
|
||||
* @brief Number of publications the sender thread never put on the wire.
|
||||
* @details Synchronise() promotes a snapshot (Strict/Decimate) or a batch
|
||||
* (Accumulate) to the ready buffer for the sender thread. If the next
|
||||
* promotion arrives before the sender has taken the previous one, that
|
||||
* publication is overwritten and its cycles never reach any receiver —
|
||||
* which a consumer sees as a hole in the time series. Counts those, so the
|
||||
* loss is measurable rather than inferred from the plot.
|
||||
*/
|
||||
uint32 GetDroppedPublications() const;
|
||||
|
||||
private:
|
||||
/**
|
||||
* @brief Serializes the CONFIG payload into buf and sets payloadSize.
|
||||
@@ -349,6 +360,18 @@ private:
|
||||
*/
|
||||
static uint8 TypeDescriptorToCode(TypeDescriptor td);
|
||||
|
||||
/**
|
||||
* @brief True when the ready buffer holds data the sender has not taken yet.
|
||||
* @details Read under bufMutex. The sender must consult this rather than
|
||||
* rely on the dataSem edge, which ResetWait can destroy.
|
||||
*/
|
||||
bool HasPendingPublication();
|
||||
|
||||
/**
|
||||
* @brief Emits at most one warning per second about overwritten publications.
|
||||
*/
|
||||
void ReportDroppedPublications();
|
||||
|
||||
/* Configuration parameters */
|
||||
uint16 port; /**< UDP server port */
|
||||
uint32 maxPayloadSize; /**< Max payload bytes per UDP packet (excluding header) */
|
||||
@@ -367,6 +390,13 @@ private:
|
||||
uint64 *readyTimestamps; /**< Heap: [maxBatchCount] HRT for completed ready batch */
|
||||
uint64 *scratchTimestamps; /**< Heap: [maxBatchCount] background-thread local copy */
|
||||
uint32 readyFill; /**< Snapshot count in the ready batch */
|
||||
/** Strict/Decimate: readyBuffer holds a snapshot the sender has not taken
|
||||
* yet. Publication state must live here rather than in dataSem, because
|
||||
* EventSem::ResetWait resets before waiting and so destroys any Post that
|
||||
* landed while the sender was busy. */
|
||||
bool readySnapshotPending;
|
||||
uint32 droppedPublications; /**< Publications overwritten before being sent */
|
||||
uint64 lastDropReportTicks; /**< Sender-thread rate limit for the drop warning */
|
||||
/* Decimate mode */
|
||||
uint32 decimateRatio; /**< Send 1 packet every decimateRatio Synchronise() calls */
|
||||
uint32 decimateCounter; /**< Current decimate cycle counter */
|
||||
|
||||
Reference in New Issue
Block a user