Repository navigation
Conversation
Event-time writers append past, increasing timestamps. A fresh Rust chain started its realtime floor at wall-clock time, so those timestamps were clamped forward to now. Rust Config::with_fresh_realtime_seed_usec and Go LogConfig.FreshRealtimeSeedUsec (with the FreshRealtimeSeed helper) set the floor for a chain with no existing tail. An existing tail always wins, the maximum value is rejected, and omitted realtime still uses the wall clock. Defaults are unchanged. Documents the option and the lazy-open requirement for event-time filenames, and tracks the remaining unset-default difference between Rust and Go as SOW-0165.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the
|
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 43 |
| Duplication | 4 |
AI Reviewer: first review requested successfully. AI can make mistakes. Always validate suggestions.
TIP This summary will be updated as you push new changes.
There was a problem hiding this comment.
Pull Request Overview
A high-severity overflow path can violate strictly increasing realtime ordering after a near-maximum seed and a subsequent duplicate or out-of-order append. This protection must be consistent across Go and Rust.
The Rust configuration change may break downstream users that construct the public configuration struct with exhaustive literals. The documented lazy-open behavior also lacks an explicit regression test.
Test suggestions
- Rust seed zero preserves past increasing realtime values
- Go seed zero preserves past increasing realtime values
- A configured seed acts as a realtime floor in both languages
- An existing chain tail overrides the configured seed in both languages
- Duplicate and out-of-order timestamps clamp to the previous timestamp plus one
- Omitted realtime uses wall-clock time, including with seed zero
- Maximum uint64 seed is rejected in both languages
- Unset Rust behavior retains wall-clock clamping
- Lazy-open event-time filename behavior is explicitly verified
Prompt proposal for missing tests
Consider implementing these tests if applicable:
1. Lazy-open event-time filename behavior is explicitly verified
TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback
There was a problem hiding this comment.
All reported issues were addressed across 15 files
Architecture diagram
sequenceDiagram
participant User as Netdata Event-Time Writer
participant GoAPI as Go Log/LogConfig
participant RustAPI as Rust Log/Config
participant Chain as Journal Chain (Disk)
participant Clock as RealtimeClock
Note over User,Clock: Fresh Realtime Seed Flow (SOW-0123)
alt Go writer
User->>GoAPI: NewLog(dir, LogConfig{FreshRealtimeSeedUsec})
GoAPI->>GoAPI: validateNewLogConfig()
alt Seed == MaxUint64
GoAPI-->>User: errInvalidJournal
else Seed valid
GoAPI->>GoAPI: lastRealtime = *seed
GoAPI->>Chain: openExistingChain()
alt Existing tail found
Chain-->>GoAPI: tail realtime
GoAPI->>GoAPI: applyChainTailState() overwrites seed
else No tail
Note over GoAPI: seed remains as floor
end
end
else Rust writer
User->>RustAPI: Log::new(path, Config::with_fresh_realtime_seed_usec(seed))
RustAPI->>RustAPI: validate_config()
alt Seed == u64::MAX
RustAPI-->>User: WriterError::InvalidConfig
else Seed valid
RustAPI->>RustAPI: initial_log_clock(chain, config)
RustAPI->>Chain: chain.tail_realtime()
alt Existing tail found
Chain-->>RustAPI: tail realtime
RustAPI->>Clock: RealtimeClock::with_initial(tail)
else No tail, seed set
RustAPI->>Clock: RealtimeClock::with_initial(seed)
else No tail, seed unset
RustAPI->>Clock: RealtimeClock::new() (wall clock)
end
end
end
Note over GoAPI,RustAPI: Append Path - Strictly Increasing Realtime
User->>GoAPI: Append(entry, RealtimeUsec: past)
GoAPI->>GoAPI: entryOptionsForAppend()
alt Realtime <= previous
GoAPI->>GoAPI: Clamp to previous + 1
else Realtime > previous
GoAPI->>GoAPI: Keep realtime
end
GoAPI->>Chain: Write entry with realtime
User->>RustAPI: write_entry_with_timestamps(realtime: past)
RustAPI->>Clock: observe(realtime)
alt Realtime <= clock floor
Clock->>Clock: Clamp to floor + 1
Clock-->>RustAPI: clamped realtime
else Realtime > clock floor
Clock-->>RustAPI: original realtime
end
RustAPI->>Chain: Write entry with realtime
Note over GoAPI,RustAPI: Default Unset Behavior (SOW-0165)
alt Go default (no seed)
Note over GoAPI: Floor = 0 (zero value)
Note over GoAPI: Past realtime kept verbatim
else Rust default (no seed)
Note over RustAPI: Floor = wall clock at startup
Note over RustAPI: Past realtime clamped to now
end
Note over Chain: Lazy open required for event-time filenames
Note over Chain: Eager open names file before first event exists
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
systemd accepts realtime only in (0, 2^55). A seed just below the uint64 limit let a later clamp wrap to zero in Go or saturate in Rust. Seeds at or above 2^55 - 1 are now rejected in both languages, so seed + 1 is always a valid systemd realtime. Docs say the seed must be strictly below the first event time to keep it, and that the wall-clock floor applies only without a seed. SOW-0165 gains a conditional acceptance criterion and a backlog slot.
…time-seed # Conflicts: # .agents/sow/SOW-status.md
…time-seed # Conflicts: # .agents/sow/SOW-status.md
…time-seed # Conflicts: # .agents/sow/SOW-status.md
Stacked on #15. Merge order: #13, #14, #15, then this. Completes SOW-0123 (
.agents/sow/done/SOW-0123-20260624-event-time-lane-fresh-clock-seed.md).Decisions for review
journal.FreshRealtimeSeed(uint64), followingPublishEveryEntriesandSyncOnArchive. The Rust names follow the plan reviewed in the SOW.Problem
Event-time writers, such as the Netdata netflow VPC lane, append past timestamps in increasing order. A fresh Rust chain started its realtime floor at wall-clock time, so those timestamps were clamped forward to now.
Changes
Config::fresh_realtime_seed_usec: Option<u64>andConfig::with_fresh_realtime_seed_usec.Config::newis unchanged.LogConfig.FreshRealtimeSeedUsec *uint64andjournal.FreshRealtimeSeed.uint64seed is rejected;+ 1.Rust-API.md,Go-API.md,Options-Reference.md, and the spec, including the lazy-open requirement for event-time filenames.Validation
cargo test --workspace(45 suites) andCGO_ENABLED=0 go test ./...pass.tests/docs/check_wiki_docs.pyandtests/docs/verify_examples.pypass.Summary by cubic
Adds a fresh-chain realtime seed to the directory writers so event-time lanes can append past, increasing timestamps without Rust clamping them forward to the wall clock.
Config::with_fresh_realtime_seed_usecand GoLogConfig.FreshRealtimeSeedUsec(via theFreshRealtimeSeedhelper) set the realtime floor for a chain with no existing tail.2^55 - 1are rejected soseed + 1stays valid systemd realtime, and omitted realtime still uses the wall clock. Defaults are unchanged.SOW-0165 follow-up — with the option unset, fresh Rust chains clamp past realtime forward to now while fresh Go chains keep it. The user decided Rust will adopt Go's zero floor (option A), pending a check of Netdata's Rust
Logcallers; this PR changes neither default.Written for commit b24495d. Summary will update on new commits.