Skip to content

Commit aff253d

Browse files
Minipadaclaude
andcommitted
fix(bridge): send Record timestamps at nanosecond resolution
The Bridge read only `msg.header.stamp.sec` and dropped `.nanosec`, then packed it as the ingest protocol's plain integer time. Every Record's timestamp was rounded to the second, so a Measurement polling faster than 1 Hz produced Records that were indistinguishable in time — measured at 5 Hz: 8 Records, 3 distinct timestamps. The wire format already supported the fix. Fluent Forward has an EventTime extension (ext type 0x00, 4 bytes seconds + 4 nanoseconds), and the vendored Vector 0.57.0 parses it with all nine digits intact — verified by feeding its fluent source both frame shapes. Only the Bridge needed to change: Record now carries seconds and nanoseconds, and the Forwarder packs EventTime. Adds `epoch_nanos` as a new time_format, and makes it the default. `double` cannot represent nanoseconds — a float64 has ~15-16 significant digits and current epoch seconds spend 10 of them, so it tops out near microseconds. That is IEEE 754, not an implementation limit. `double` and `iso8601` stay available. Exactness also matters beyond precision: it is what makes the timestamp usable as part of a Record identity, which #309 needs. The schema follows: `date` becomes bigint in both init.sql files, `to_timestamp(date)` becomes `to_timestamp(date / 1e9)` in the dashboard, and params writing to those tables drop their explicit `time_format: "double"`. Console-only demos keep theirs. Regression cover: e2e_params.yaml's memory Measurement now polls at 5 Hz so several Records land inside one second, and verify_zero_loss.py asserts a nanosecond remainder exists, that some second holds 2+ Records (so it cannot pass vacuously), and that no two Records share an instant. At 1 Hz a return to whole-second stamps would not collide and would pass unnoticed. Closes #308 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A5JwZEZrxEtYJdQUfsZRVo Signed-off-by: David Bensoussan <d.bensoussan@proton.me>
1 parent 8c00136 commit aff253d

21 files changed

Lines changed: 378 additions & 40 deletions

File tree

dc_bridge/include/dc_bridge/forwarder.hpp

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,20 @@ namespace dc_bridge
1717
{
1818

1919
/// A DC Record ready to be forwarded: a shipper ingest protocol tag, a Unix timestamp
20-
/// (seconds since the epoch, matching the protocol's integer time), and the Record's
20+
/// split into whole seconds plus the nanoseconds within that second, and the Record's
2121
/// JSON payload (StringStamped.data, already parsed).
22+
///
23+
/// The split mirrors the ingest protocol's **EventTime** extension (4 bytes of seconds,
24+
/// 4 of nanoseconds), which is what the Forwarder emits. The protocol's older integer
25+
/// time carries whole seconds only; sending that rounded every Record's timestamp to the
26+
/// second, so a Measurement polling faster than 1 Hz produced Records that were
27+
/// indistinguishable in time (#308).
2228
struct Record
2329
{
2430
std::string tag;
2531
std::uint64_t timestamp_secs;
32+
/// Nanoseconds within `timestamp_secs`; always < 1'000'000'000.
33+
std::uint32_t timestamp_nanos{ 0 };
2634
nlohmann::json payload;
2735
};
2836

dc_bridge/include/dc_bridge/render.hpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,16 @@ inline constexpr std::uint64_t MIN_DISK_BUFFER_BYTES = 268435488ULL;
3636
/// route transform id, rest = the Tag verbatim).
3737
std::string route_output_for_tag(const std::string& tag);
3838

39+
/// How a Destination's normalized time field is written.
40+
///
41+
/// `EpochNanos` is the default: an exact integer count of nanoseconds since the epoch.
42+
/// `Double` divides that by 1e9 into a float64, which has ~15-16 significant digits and
43+
/// spends 10 of them on the seconds — so it cannot represent better than roughly
44+
/// microseconds, and rounds. It stays available for consumers that want a fractional
45+
/// seconds column, but it is lossy by construction, not by implementation (#308).
3946
enum class TimeFormat
4047
{
48+
EpochNanos,
4149
Double,
4250
Iso8601,
4351
};

dc_bridge/src/bridge_node.cpp

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,19 @@ std::string expand_with_env(const std::string& input)
3939
return expand_env(input, env_lookup);
4040
}
4141

42+
// Stamps a Bridge-generated Record (File status, retention audit) with the current time,
43+
// split into the seconds/nanoseconds pair the ingest protocol's EventTime carries. These
44+
// Records have no ROS header to take a stamp from, unlike the ones forwarded from a
45+
// Measurement's topic. Truncating this to whole seconds was part of #308.
46+
void stamp_now(Record& record)
47+
{
48+
const auto since_epoch = std::chrono::system_clock::now().time_since_epoch();
49+
const auto secs = std::chrono::duration_cast<std::chrono::seconds>(since_epoch);
50+
record.timestamp_secs = static_cast<std::uint64_t>(secs.count());
51+
record.timestamp_nanos =
52+
static_cast<std::uint32_t>(std::chrono::duration_cast<std::chrono::nanoseconds>(since_epoch - secs).count());
53+
}
54+
4255
// Declares `name` with dynamic typing and a PARAMETER_NOT_SET default, so a value the
4356
// user didn't provide reads back as nullopt (rather than forcing a sentinel/default).
4457
std::optional<std::string> declare_optional_string(rclcpp::Node* node, const std::string& name)
@@ -419,7 +432,11 @@ BridgeNode::BridgeNode(const rclcpp::NodeOptions& options) : rclcpp::Node("dc_br
419432
{
420433
Record record;
421434
record.tag = tag;
435+
// Both halves of the ROS stamp. The nanoseconds used to be dropped here,
436+
// which rounded every Record to the second and left a Measurement polling
437+
// faster than 1 Hz with Records indistinguishable in time (#308).
422438
record.timestamp_secs = static_cast<std::uint64_t>(std::max<std::int32_t>(0, msg.header.stamp.sec));
439+
record.timestamp_nanos = msg.header.stamp.nanosec;
423440
record.payload = std::move(payload);
424441
try
425442
{
@@ -465,8 +482,7 @@ void BridgeNode::run_uploader_worker(std::string forward_host, std::uint16_t for
465482
auto emit = [&forwarder](const nlohmann::json& row) {
466483
Record record;
467484
record.tag = uploader::FILE_STATUS_TAG;
468-
record.timestamp_secs = static_cast<std::uint64_t>(
469-
std::chrono::duration_cast<std::chrono::seconds>(std::chrono::system_clock::now().time_since_epoch()).count());
485+
stamp_now(record);
470486
record.payload = row;
471487
// Vector may be briefly down (restart, backpressure); keep trying for a while before
472488
// handing the whole Record back for an idempotent retry.
@@ -493,8 +509,7 @@ void BridgeNode::run_uploader_worker(std::string forward_host, std::uint16_t for
493509
auto retention_emit = [&forwarder](const nlohmann::json& row) {
494510
Record record;
495511
record.tag = uploader::FILE_STATUS_TAG;
496-
record.timestamp_secs = static_cast<std::uint64_t>(
497-
std::chrono::duration_cast<std::chrono::seconds>(std::chrono::system_clock::now().time_since_epoch()).count());
512+
stamp_now(record);
498513
record.payload = row;
499514
forwarder.send(record);
500515
};

dc_bridge/src/forwarder.cpp

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include <sys/types.h>
1010
#include <unistd.h>
1111

12+
#include <array>
1213
#include <cerrno>
1314
#include <cstring>
1415
#include <ctime>
@@ -87,6 +88,29 @@ void pack_record_map(Packer& pk, const nlohmann::json& payload)
8788
}
8889
}
8990

91+
// Packs a timestamp as the ingest protocol's **EventTime** extension: msgpack ext type
92+
// 0x00 with an 8-byte body — 4 bytes of seconds then 4 of nanoseconds, both big-endian
93+
// (network order), exactly as the protocol specifies.
94+
//
95+
// The alternative the protocol allows is a plain integer of whole seconds, which is what
96+
// DC sent until #308. That silently rounded every Record's timestamp down to the second,
97+
// so five Records from a 5 Hz Measurement all claimed the same instant. Seconds are
98+
// narrowed to 32 bits here because the extension's layout is fixed at 4 bytes; that field
99+
// overflows in 2106, long after the int32 ROS header stamp this is built from.
100+
template <typename Packer>
101+
void pack_event_time(Packer& pk, std::uint64_t seconds, std::uint32_t nanos)
102+
{
103+
std::array<char, 8> body{};
104+
const auto secs32 = static_cast<std::uint32_t>(seconds);
105+
for (int i = 0; i < 4; ++i)
106+
{
107+
body[i] = static_cast<char>((secs32 >> (8 * (3 - i))) & 0xFF);
108+
body[4 + i] = static_cast<char>((nanos >> (8 * (3 - i))) & 0xFF);
109+
}
110+
pk.pack_ext(body.size(), 0x00);
111+
pk.pack_ext_body(body.data(), body.size());
112+
}
113+
90114
} // namespace
91115

92116
Forwarder::Forwarder(ForwarderConfig config) : config_(std::move(config))
@@ -153,7 +177,7 @@ std::string Forwarder::frame(const Record& record, const std::string& chunk_id)
153177
pk.pack(record.tag);
154178
pk.pack_array(1); // one entry
155179
pk.pack_array(2); // [time, record]
156-
pk.pack(record.timestamp_secs);
180+
pack_event_time(pk, record.timestamp_secs, record.timestamp_nanos);
157181
pack_record_map(pk, record.payload);
158182
pk.pack_map(1);
159183
pk.pack(std::string("chunk"));

dc_bridge/src/render.cpp

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,14 @@ std::string render_normalize_source(const RenderConfig& config)
134134
for (const auto& dest : config.destinations)
135135
{
136136
out += "if " + tag_condition(destination_tags(dest)) + " {\n";
137-
if (dest.time_format == TimeFormat::Double)
137+
if (dest.time_format == TimeFormat::EpochNanos)
138+
{
139+
// Exact: an i64 of nanoseconds since the epoch, no float rounding anywhere. Good
140+
// until the year 2262, and the only format here that can round-trip the full
141+
// resolution the Forwarder now sends (#308).
142+
out += " ." + dest.time_key + " = to_unix_timestamp!(.timestamp, unit: \"nanoseconds\")\n";
143+
}
144+
else if (dest.time_format == TimeFormat::Double)
138145
{
139146
out +=
140147
" ." + dest.time_key + " = to_float(to_unix_timestamp!(.timestamp, unit: \"nanoseconds\")) / 1000000000.0\n";
@@ -430,9 +437,13 @@ Destination destination_from_raw(const std::string& name, const std::string& typ
430437
}
431438

432439
std::string time_key = optional_field(raw.time_key).value_or("date");
433-
std::string tf = raw.time_format && !raw.time_format->empty() ? *raw.time_format : "double";
440+
std::string tf = raw.time_format && !raw.time_format->empty() ? *raw.time_format : "epoch_nanos";
434441
TimeFormat time_format;
435-
if (tf == "double")
442+
if (tf == "epoch_nanos")
443+
{
444+
time_format = TimeFormat::EpochNanos;
445+
}
446+
else if (tf == "double")
436447
{
437448
time_format = TimeFormat::Double;
438449
}
@@ -442,9 +453,10 @@ Destination destination_from_raw(const std::string& name, const std::string& typ
442453
}
443454
else
444455
{
445-
throw RenderError(
446-
RenderErrorKind::InvalidTimeFormat,
447-
"destination '" + name + "': time_format '" + tf + "' is invalid (expected 'double' or 'iso8601')", name, tf);
456+
throw RenderError(RenderErrorKind::InvalidTimeFormat,
457+
"destination '" + name + "': time_format '" + tf +
458+
"' is invalid (expected 'epoch_nanos', 'double' or 'iso8601')",
459+
name, tf);
448460
}
449461

450462
DestinationKind kind;

dc_bridge/test/forwarder_test.cpp

Lines changed: 55 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,29 @@ using namespace dc_bridge;
2525
namespace
2626
{
2727

28-
Record make_record(const std::string& tag, const std::string& json)
28+
Record make_record(const std::string& tag, const std::string& json, std::uint32_t nanos = 0)
2929
{
30-
return Record{ tag, 1700000000ULL, nlohmann::json::parse(json) };
30+
return Record{ tag, 1700000000ULL, nanos, nlohmann::json::parse(json) };
31+
}
32+
33+
// The EventTime extension the frame carries the timestamp in: msgpack ext type 0x00,
34+
// 8-byte body of big-endian seconds then big-endian nanoseconds (#308). Built here
35+
// independently of the packing code so the test pins the wire bytes rather than
36+
// re-deriving them the same way the implementation does.
37+
std::string expected_event_time_bytes(std::uint32_t secs, std::uint32_t nanos)
38+
{
39+
std::string out;
40+
out.push_back(static_cast<char>(0xd7)); // fixext8
41+
out.push_back(static_cast<char>(0x00)); // ext type 0 = EventTime
42+
for (int i = 0; i < 4; ++i)
43+
{
44+
out.push_back(static_cast<char>((secs >> (8 * (3 - i))) & 0xFF));
45+
}
46+
for (int i = 0; i < 4; ++i)
47+
{
48+
out.push_back(static_cast<char>((nanos >> (8 * (3 - i))) & 0xFF));
49+
}
50+
return out;
3151
}
3252

3353
// A mock shipper ingest protocol server: binds :0, hands back the chosen port, and runs
@@ -147,7 +167,8 @@ TEST(Forwarder, FrameIsWellFormedWithCorrectTag)
147167
msgpack::object entries = top.via.array.ptr[1];
148168
ASSERT_EQ(entries.via.array.size, 1u);
149169
msgpack::object entry = entries.via.array.ptr[0]; // [time, record]
150-
EXPECT_EQ(entry.via.array.ptr[0].as<std::uint64_t>(), 1700000000ULL);
170+
// The time element is the EventTime extension, not a bare integer of seconds (#308).
171+
EXPECT_EQ(entry.via.array.ptr[0].type, msgpack::type::EXT);
151172

152173
msgpack::object rec = entry.via.array.ptr[1];
153174
ASSERT_EQ(rec.type, msgpack::type::MAP);
@@ -163,6 +184,36 @@ TEST(Forwarder, FrameIsWellFormedWithCorrectTag)
163184
EXPECT_TRUE(found);
164185
}
165186

187+
// #308: sub-second resolution must survive the wire. Before this, the frame carried a
188+
// plain integer of whole seconds, so a Measurement polling faster than 1 Hz produced
189+
// Records that all claimed the same instant.
190+
TEST(Forwarder, FrameCarriesSubSecondPrecisionAsEventTime)
191+
{
192+
const std::uint32_t nanos = 123456789u;
193+
const std::string frame = Forwarder::frame(make_record("dc.test", R"({"n": 1})", nanos), "chunk-ns");
194+
EXPECT_NE(frame.find(expected_event_time_bytes(1700000000u, nanos)), std::string::npos)
195+
<< "frame does not carry the EventTime extension for 1700000000.123456789";
196+
}
197+
198+
// Two Records one nanosecond apart must not collapse onto the same wire timestamp —
199+
// the property that makes a timestamp usable as part of an identity.
200+
TEST(Forwarder, FramesWithinTheSameSecondAreDistinguishable)
201+
{
202+
const std::string a = Forwarder::frame(make_record("dc.test", R"({"n": 1})", 1u), "c1");
203+
const std::string b = Forwarder::frame(make_record("dc.test", R"({"n": 1})", 2u), "c1");
204+
EXPECT_NE(a, b);
205+
EXPECT_NE(a.find(expected_event_time_bytes(1700000000u, 1u)), std::string::npos);
206+
EXPECT_NE(b.find(expected_event_time_bytes(1700000000u, 2u)), std::string::npos);
207+
}
208+
209+
// A whole-second Record still encodes as EventTime, with a zero nanosecond half, rather
210+
// than falling back to the integer form.
211+
TEST(Forwarder, WholeSecondTimestampStillUsesEventTime)
212+
{
213+
const std::string frame = Forwarder::frame(make_record("dc.test", R"({"n": 1})", 0u), "c0");
214+
EXPECT_NE(frame.find(expected_event_time_bytes(1700000000u, 0u)), std::string::npos);
215+
}
216+
166217
TEST(Forwarder, FrameCarriesTheChunkOption)
167218
{
168219
auto oh = unpack(Forwarder::frame(make_record("dc.test", R"({"n": 1})"), "my-chunk-id"));
@@ -258,7 +309,7 @@ TEST(Forwarder, ReturnsBackpressureWhenPeerStalls)
258309

259310
nlohmann::json big;
260311
big["blob"] = std::string(1000000, 'x');
261-
Record big_record{ "dc.test", 1700000000ULL, big };
312+
Record big_record{ "dc.test", 1700000000ULL, 0u, big };
262313

263314
bool saw_backpressure = false;
264315
for (int i = 0; i < 20; ++i)

dc_bridge/test/render_test.cpp

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -417,6 +417,33 @@ TEST(Render, DestinationFromRawRejectsInvalidTimeFormat)
417417
}
418418
}
419419

420+
TEST(Render, DestinationFromRawAcceptsEveryTimeFormat)
421+
{
422+
const std::vector<std::pair<std::string, TimeFormat>> cases = { { "epoch_nanos", TimeFormat::EpochNanos },
423+
{ "double", TimeFormat::Double },
424+
{ "iso8601", TimeFormat::Iso8601 } };
425+
for (const auto& c : cases)
426+
{
427+
RawDestinationParams raw;
428+
raw.time_format = c.first;
429+
auto dest = destination_from_raw("debug_console", "console", "records", { "/dc/measurement/map" }, raw);
430+
EXPECT_EQ(dest.time_format, c.second) << "time_format '" << c.first << "'";
431+
}
432+
}
433+
434+
// The normalize transform must emit the timestamp with no float arithmetic in it —
435+
// `to_float(...) / 1000000000.0` is precisely what loses resolution below ~1 us.
436+
TEST(Render, EpochNanosNormalizesWithoutFloatRounding)
437+
{
438+
auto config =
439+
config_with({ make_destination("pgsql", { "/dc/group/robot" }, TimeFormat::EpochNanos, postgres_kind()) });
440+
const std::string rendered = render(config);
441+
EXPECT_NE(rendered.find(".date = to_unix_timestamp!(.timestamp, unit: \"nanoseconds\")"), std::string::npos)
442+
<< rendered;
443+
EXPECT_EQ(rendered.find("to_float("), std::string::npos)
444+
<< "epoch_nanos must not round through a float: " << rendered;
445+
}
446+
420447
TEST(Render, PostgresFromRawRejectsMissingRequiredFields)
421448
{
422449
RawDestinationParams raw;
@@ -465,11 +492,13 @@ TEST(Render, PostgresFromRawAppliesDefaults)
465492
EXPECT_EQ(pg.port, 5432);
466493
}
467494

495+
// #308: the default is the exact integer format, not the lossy float one. A Destination
496+
// that says nothing about time must not silently round its timestamps.
468497
TEST(Render, DestinationFromRawAppliesTimeDefaults)
469498
{
470499
auto dest = destination_from_raw("debug_console", "console", "records", { "/dc/group/robot" }, {});
471500
EXPECT_EQ(dest.time_key, "date");
472-
EXPECT_EQ(dest.time_format, TimeFormat::Double);
501+
EXPECT_EQ(dest.time_format, TimeFormat::EpochNanos);
473502
}
474503

475504
TEST(Render, S3FromRawRejectsMissingBucket)

dc_bringup/params/dc_params.yaml

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,10 @@ dc_bridge:
2525
database: "dc"
2626
table: "dc"
2727
time_key: "date"
28-
time_format: "double"
28+
# epoch_nanos (default) = exact integer nanoseconds since the epoch; iso8601 =
29+
# "…T11:36:28.123456789" string; double = fractional seconds, which is a float64
30+
# and rounds below ~1 us. Match the column type: bigint for epoch_nanos.
31+
time_format: "epoch_nanos"
2932
# The other blessed destination types (add the name to `destinations` to enable):
3033
#
3134
# rustfs: # S3-compatible object storage (self-hosted: RustFS

dc_demos/params/qrcodes_minio_pgsql.yaml

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ dc_bridge:
2020
database: "dc"
2121
table: "dc"
2222
time_key: "date"
23-
time_format: "double"
2423
pgsql_files:
2524
type: postgres
2625
receives: records
@@ -31,7 +30,6 @@ dc_bridge:
3130
database: "dc"
3231
table: "dc_files"
3332
time_key: "date"
34-
time_format: "double"
3533
rustfs:
3634
type: s3
3735
receives: files

dc_demos/params/tb3_simulation_pgsql_minio.yaml

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ dc_bridge:
3131
database: "dc"
3232
table: "dc"
3333
time_key: "date"
34-
time_format: "double"
3534
pgsql_files:
3635
type: postgres
3736
receives: records
@@ -42,7 +41,6 @@ dc_bridge:
4241
database: "dc"
4342
table: "dc_files"
4443
time_key: "date"
45-
time_format: "double"
4644
rustfs:
4745
type: s3
4846
receives: files

0 commit comments

Comments
 (0)