Skip to content

Add a fresh-chain realtime seed to the directory writers - #16

Open
ktsaou wants to merge 12 commits into
sow-0071/runtime-purity-guardfrom
sow-0123/fresh-realtime-seed
Open

ktsaou wants to merge 12 commits into
sow-0071/runtime-purity-guardfrom
sow-0123/fresh-realtime-seed

Conversation

@ktsaou

@ktsaou ktsaou commented Nov 1, 2026 •

Copy link
Copy Markdown
Member

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

  • Agent-chosen: the Go pointer helper is named journal.FreshRealtimeSeed(uint64), following PublishEveryEntries and SyncOnArchive. The Rust names follow the plan reviewed in the SOW.
  • Needs your decision, not blocking this PR: with the option unset, a fresh Rust chain clamps a past realtime forward to now, while a fresh Go chain keeps it. This difference predates the PR; the PR keeps both defaults unchanged and documents the difference. It is tracked as SOW-0165, with options A (Rust follows Go), B (Go follows Rust) and C (keep it as an accepted exception).

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

  • Rust: Config::fresh_realtime_seed_usec: Option<u64> and Config::with_fresh_realtime_seed_usec. Config::new is unchanged.
  • Go: LogConfig.FreshRealtimeSeedUsec *uint64 and journal.FreshRealtimeSeed.
  • In both languages:
    • an existing chain tail always wins over the seed;
    • the maximum uint64 seed is rejected;
    • omitted realtime still uses the wall clock;
    • duplicate and out-of-order realtime still clamp to previous + 1.
  • Docs: Rust-API.md, Go-API.md, Options-Reference.md, and the spec, including the lazy-open requirement for event-time filenames.

Validation

  • New tests in both languages: seed 0 keeps past times; the seed acts as a floor; the tail wins; clamping of duplicate and out-of-order times; wall-clock for omitted realtime; maximum rejected. Rust also covers the default clamping.
  • cargo test --workspace (45 suites) and CGO_ENABLED=0 go test ./... pass.
  • tests/docs/check_wiki_docs.py and tests/docs/verify_examples.py pass.

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.

  • Rust Config::with_fresh_realtime_seed_usec and Go LogConfig.FreshRealtimeSeedUsec (via the FreshRealtimeSeed helper) set the realtime floor for a chain with no existing tail.
  • Behavior changes only when the option is set; an existing tail always wins, seeds at or above 2^55 - 1 are rejected so seed + 1 stays valid systemd realtime, and omitted realtime still uses the wall clock. Defaults are unchanged.
  • Event-time filenames require lazy open; eager open names the file before any event exists.

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 Log callers; this PR changes neither default.

Written for commit b24495d. Summary will update on new commits.

View guided diff Turn on auto-fix

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.
@coderabbitai

coderabbitai Bot commented Nov 1, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 4bc8ff4b-7ed0-42a3-aa28-68ab02050007

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

CodeRabbit Verify

  • Run CodeRabbit Verify to test the changes in this pull request.

@codacy-production

codacy-production Bot commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 43 complexity · 4 duplication

Metric Results
Complexity 43
Duplication 4

View in Codacy

AI Reviewer: first review requested successfully. AI can make mistakes. Always validate suggestions.

Run reviewer

TIP This summary will be updated as you push new changes.

@codacy-production codacy-production Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

⚠️ Resolve issues before merging

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

Comment thread go/journal/log.go Resolved

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Loading

Reply with feedback, questions, or to request a fix.

View guided diff | Turn on auto-fix | Re-trigger cubic

Comment thread .agents/sow/SOW-status.md Resolved
Comment thread docs/Options-Reference.md Resolved
Comment thread docs/Rust-API.md Resolved
Comment thread go/journal/log.go Resolved
Comment thread docs/Rust-API.md Resolved
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.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant