diff --git a/cmd/server/obs_parsed_time_test.go b/cmd/server/obs_parsed_time_test.go new file mode 100644 index 00000000..65aabb82 --- /dev/null +++ b/cmd/server/obs_parsed_time_test.go @@ -0,0 +1,72 @@ +package main + +import ( + "sync" + "testing" + "time" + "unsafe" +) + +// StoreObs is the store's most numerous struct (400k on a week of a busy +// network). Its lazy timestamp cache used to be a time.Time, a sync.Once and +// a bool, 40 bytes that put each observation in the 192-byte size class. It +// is 8 bytes now, and each observation fits in 160. +func TestStoreObsFitsIn160Bytes(t *testing.T) { + if n := unsafe.Sizeof(StoreObs{}); n > 160 { + t.Fatalf("StoreObs is %d bytes, want at most 160 (one size class down from 192)", n) + } +} + +func parseObsTimestampReference(s string) (time.Time, bool) { + for _, layout := range []string{time.RFC3339Nano, time.RFC3339, "2006-01-02 15:04:05"} { + if t, err := time.Parse(layout, s); err == nil { + return t, true + } + } + return time.Time{}, false +} + +// ParsedTime gives the same time, location included, as parsing the string. +func TestStoreObsParsedTimeMatchesParse(t *testing.T) { + for _, ts := range []string{ + "2026-10-10T12:34:56.789Z", + "2026-10-10T12:34:56Z", + "2026-10-10T13:34:56+01:00", + "2026-10-10 12:34:56", + "", + "not a time", + "1970-01-01T00:00:00Z", + } { + o := &StoreObs{Timestamp: ts} + want, wantOK := parseObsTimestampReference(ts) + for i := 0; i < 2; i++ { // first call parses, second reads the cache + got, ok := o.ParsedTime() + if ok != wantOK || !got.Equal(want) || got.Location().String() != want.Location().String() { + t.Fatalf("%q call %d: got %v (%v) ok=%v, want %v (%v) ok=%v", ts, i, got, got.Location(), ok, want, want.Location(), wantOK) + } + if wantOK && got.Hour() != want.Hour() { + t.Fatalf("%q: hour %d, want %d", ts, got.Hour(), want.Hour()) + } + } + } +} + +// Concurrent readers under the store's read lock may race to fill the cache. +func TestStoreObsParsedTimeConcurrentFill(t *testing.T) { + o := &StoreObs{Timestamp: "2026-10-10T12:34:56.789Z"} + want, _ := parseObsTimestampReference(o.Timestamp) + var wg sync.WaitGroup + for i := 0; i < 16; i++ { + wg.Add(1) + go func() { + defer wg.Done() + for j := 0; j < 100; j++ { + if got, ok := o.ParsedTime(); !ok || !got.Equal(want) { + t.Errorf("got %v ok=%v", got, ok) + return + } + } + }() + } + wg.Wait() +} diff --git a/cmd/server/retransmission_pressure_test.go b/cmd/server/retransmission_pressure_test.go index 9226ba27..0456f141 100644 --- a/cmd/server/retransmission_pressure_test.go +++ b/cmd/server/retransmission_pressure_test.go @@ -675,8 +675,7 @@ func BenchmarkComputeRetransmissionPressureColdTimestamps(b *testing.B) { b.StopTimer() for _, tx := range s.packets { for _, o := range tx.Observations { - o.tsParseOnce = sync.Once{} - o.tsParsed, o.tsParsedOK = time.Time{}, false + o.tsUnixNano.Store(0) } } b.StartTimer() diff --git a/cmd/server/store.go b/cmd/server/store.go index 04bb7d6d..8374e787 100644 --- a/cmd/server/store.go +++ b/cmd/server/store.go @@ -93,36 +93,61 @@ type StoreObs struct { // #1481 P0-2: cached parsed timestamp. The legacy handlers parse // Timestamp as RFC3339Nano/RFC3339/"2006-01-02 15:04:05" with a // fallback chain on every read; for /api/observers/{id}/analytics - // that fires 60k+ times per request under RLock. ParsedTime returns - // the parsed value once, caching for the lifetime of the StoreObs. - tsParseOnce sync.Once - tsParsed time.Time - tsParsedOK bool + // that fires 60k+ times per request under RLock. ParsedTime parses + // once and keeps the result here in 8 bytes (see tsUnparsed): a + // time.Time, sync.Once and bool took 40 and put each observation in + // the 192-byte size class instead of 160. + tsUnixNano atomic.Int64 } // ParsedTime returns the parsed Timestamp, caching the result. // Tries RFC3339Nano → RFC3339 → "2006-01-02 15:04:05" in order. // Returns (zero, false) if Timestamp is empty or unparseable. -// Thread-safe via sync.Once. +// Safe for concurrent use: callers racing on the first call each parse and +// store the same value. func (o *StoreObs) ParsedTime() (time.Time, bool) { - o.tsParseOnce.Do(func() { - if o.Timestamp == "" { - return + switch n := o.tsUnixNano.Load(); n { + case 0: + case tsUnparsed: + return time.Time{}, false + case tsReparse: + return parseObsTimestamp(o.Timestamp) + default: + return time.Unix(0, n).UTC(), true + } + t, ok := parseObsTimestamp(o.Timestamp) + switch { + case !ok: + o.tsUnixNano.Store(tsUnparsed) + case t.Location() != time.UTC || t.Year() < 1678 || t.Year() > 2261 || t.UnixNano() == 0: + // Kept only as UTC nanoseconds; a time with another offset (or one + // UnixNano cannot hold) is parsed again on each call so its + // location is kept. + o.tsUnixNano.Store(tsReparse) + default: + o.tsUnixNano.Store(t.UnixNano()) + return time.Unix(0, t.UnixNano()).UTC(), true + } + return t, ok +} + +// tsUnixNano holds 0 before the first ParsedTime, the UTC time in +// nanoseconds after it, or one of these. +const ( + tsUnparsed = math.MinInt64 // empty or not a time + tsReparse = math.MinInt64 + 1 // parsed, but not held as UTC nanoseconds +) + +func parseObsTimestamp(s string) (time.Time, bool) { + if s == "" { + return time.Time{}, false + } + for _, layout := range []string{time.RFC3339Nano, time.RFC3339, "2006-01-02 15:04:05"} { + if t, err := time.Parse(layout, s); err == nil { + return t, true } - if t, err := time.Parse(time.RFC3339Nano, o.Timestamp); err == nil { - o.tsParsed, o.tsParsedOK = t, true - return - } - if t, err := time.Parse(time.RFC3339, o.Timestamp); err == nil { - o.tsParsed, o.tsParsedOK = t, true - return - } - if t, err := time.Parse("2006-01-02 15:04:05", o.Timestamp); err == nil { - o.tsParsed, o.tsParsedOK = t, true - return - } - }) - return o.tsParsed, o.tsParsedOK + } + return time.Time{}, false } // ParsedDecoded returns the parsed DecodedJSON map, caching the result.