[FIXED] Preserve stream created time after recovery

Project: nats-io/nats-server | Language: Go | Concepts: consistency-models, error-handling, testing-strategy | Difficulty: mid
Link: https://github.com/nats-io/nats-server/pull/8471

The problem

When recovering a file-backed JetStream stream from disk, the stream object's Created timestamp was restored correctly, but the underlying file store's configuration (fs.cfg.Created) was not. This led to a mismatch: if the stream configuration was later updated and persisted, the file store would overwrite the correct creation time with a stale value (typically the server startup time). After another restart, the incorrect creation time would be reported via the JetStream API, breaking consistency guarantees.

Why it's non-trivial

A naive fix would only update the stream's in-memory Created field, missing that the file store's configuration (which is what gets serialized to disk) also needs to be kept in sync. Failing to synchronize both can cause subtle, persistent bugs that only surface after a sequence of restarts and updates. Thread safety is also a concern: concurrent updates could leave the two timestamps out of sync if not properly locked.

The approach

1. Add a method to update the file store's creation time:

func (fs *fileStore) setCreatedTime(created time.Time) {
	fs.mu.Lock()
	fs.cfg.Created = created
	fs.mu.Unlock()
}

This method safely updates the in-memory creation time of the file store. It does not write to disk directly, but ensures that the next time the config is persisted, the correct value is used.

2. Update the stream's setCreatedTime to also update the file store, holding the lock throughout:

func (mset *stream) setCreatedTime(created time.Time) {
	mset.mu.Lock()
	mset.created = created
	store := mset.store
	if fs, ok := store.(*fileStore); ok {
		fs.setCreatedTime(created)
	}
	mset.mu.Unlock()
}

This ensures that both the stream and its backing file store have their creation times updated in a single, thread-safe operation. The lock is held for the entire duration, preventing race conditions.

3. Add a regression test to verify the fix:

func TestJetStreamFileStreamCreatedTimePreservedAfterRestartAndUpdate(t *testing.T) {
	s := RunBasicJetStreamServer(t)
	defer func() { s.Shutdown() }()

	nc, js := jsClientConnect(t, s)
	defer func() { nc.Close() }()
	si, err := js.AddStream(&nats.StreamConfig{Name: "TEST", Storage: nats.FileStorage})
	require_NoError(t, err)
	created := si.Created

	sd := s.JetStreamConfig().StoreDir
	nc.Close()
	s.Shutdown()
	time.Sleep(10 * time.Millisecond)

	s = RunJetStreamServerOnPort(-1, sd)
	nc, js = jsClientConnect(t, s)
	si, err = js.StreamInfo("TEST")
	require_NoError(t, err)
	if !si.Created.Equal(created) {
		t.Fatalf("Expected created time %v after restart, got %v", created, si.Created)
	}

	cfg := si.Config
	cfg.MaxMsgs = 1
	_, err = js.UpdateStream(&cfg)
	require_NoError(t, err)
	nc.Close()
	s.Shutdown()

	s = RunJetStreamServerOnPort(-1, sd)
	nc, js = jsClientConnect(t, s)
	si, err = js.StreamInfo("TEST")
	require_NoError(t, err)
	if !si.Created.Equal(created) {
		t.Fatalf("Expected created time %v after restart and update, got %v", created, si.Created)
	}
}

This test creates a file-backed stream, restarts the server, updates the stream config, restarts again, and checks that the creation time remains unchanged throughout.

From the review thread

MauriceVanVeen: Could mset.mu.Unlock() be moved down to the end of this method, such that mset.created will always equal fs.cfg.Created? Without it concurrent calls to mset.setCreatedTime could result in these two drifting still, although I don't think that should be possible today.

Green7: Updated. mset.mu now remains held until the backing file store timestamp has been synchronized, preventing concurrent calls from leaving the two values out of sync.

Takeaways

• When synchronizing related state across multiple components (e.g., in-memory and persisted config), always update both under a single lock to prevent inconsistencies.
• Regression tests should reproduce the full sequence of events that trigger a bug, including restarts and configuration updates, to ensure correctness across the entire lifecycle.
• Avoid assuming that fixing an in-memory value is sufficient when there are persistent stores involved—always trace how and when data is written to disk.