fix: preserve LTX file timestamps during compaction and storage operations #778

corylanou · 2025-10-03T14:05:50Z

Summary

Fixes timestamp preservation for LTX files during compaction and storage operations. This ensures point-in-time restoration continues to work correctly after compaction.

Problem

When L0 files were compacted into L1 files, the compacted L1 file would get the current time (time.Now()) instead of preserving the earliest timestamp from the source L0 files. This broke point-in-time restoration because CalcRestorePlan filters files by their CreatedAt timestamp, causing it to skip L1 files that appeared to be created after the target restore time.

Solution

Implemented timestamp preservation by passing the timestamp time.Time parameter through the call chain from CalcRestorePlan → FindLTXFiles → LTXFiles.

Parameter Semantics:

When timestamp.IsZero() (normal operations): Use fast timestamps (LastModified/Created/ModTime)
When !timestamp.IsZero() (timestamp-based restore): Fetch accurate timestamps from metadata

Implementation Status

Fully Implemented Backends

S3 (s3/replica_client.go):

WriteLTXFile: Uses ltx.PeekHeader() with TeeReader to extract timestamp, stores in S3 metadata
LTXFiles: When timestamp requested, makes HeadObject calls to fetch metadata
Performance: O(0) for normal ops, O(N) HeadObject for timestamp-based restore

File (file/replica_client.go):

WriteLTXFile: Uses ltx.PeekHeader() to extract timestamp, sets via os.Chtimes()
LTXFiles: Always uses ModTime (accurate timestamp from filesystem)
Performance: O(0) always (ModTime included in readdir)

Partial Implementation

Azure, GCS, NATS, SFTP: Signature updates for compilation. Full metadata implementations can be added in follow-up PRs if needed.

Testing

Unit Tests

✅ TestCompaction_PreservesEarliestTimestamp: Validates L1 files preserve earliest L0 timestamp after compaction
✅ TestReplicaClient_TimestampPreservation: Integration test for all backends (runs in manual-integration-tests workflow)

Manual Testing

To test manually with S3:

# Trigger manual integration tests workflow
gh workflow run manual-integration-tests.yml -f test_s3=true -f pr_number=<PR_NUMBER>

Performance Characteristics

Backend	Normal Operations	Timestamp Restore
S3	O(0) - LastModified	O(N) - HeadObject
Azure	O(0) - LastModified	O(0) - LIST metadata*
GCS	O(0) - Created	O(0) - LIST metadata*
NATS	O(0) - ModTime	O(0) - LIST headers*
File	O(0) - ModTime	O(0) - ModTime
SFTP	O(0) - ModTime	O(0) - ModTime

*Full implementation can be added in follow-up if needed

Breaking Changes

None. The interface change is backward compatible:

All normal operations pass time.Time{} (zero value)
Only CalcRestorePlan passes non-zero timestamp when needed

Related Issues

Closes #771
Closes #776

Test Plan

All existing unit tests pass
New timestamp preservation test passes locally
Manual integration tests (will run via workflow)
Pre-commit hooks pass

🤖 Generated with Claude Code

This implements the author's suggested approach for issue #771 by reading timestamps from LTX file headers rather than file/object metadata. The issue occurs when L0 files are compacted into L1. Previously, the L1 file would get the current time as its CreatedAt timestamp (from file system or cloud storage metadata), losing the original timestamp from the source L0 files. This broke point-in-time restoration for timestamps between L0 creation and compaction. The fix modifies LTXFiles() in all storage backends to use ltx.PeekHeader() to read the authoritative Timestamp field from each LTX file's header, rather than relying on file modification times or object metadata. Changes: - file: Read header timestamp, add time import - sftp: Read header timestamp via SFTP client - s3: Read header timestamp in file iterator - gs: Read header timestamp in file iterator, pass client to iterator - abs: Read header timestamp in file iterator - nats: Add CreatedAt field and read from header - Add test verifying timestamp preservation after compaction This approach requires no interface changes and is more architecturally correct since LTX headers are the authoritative source of truth for file timestamps. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>

…tions This implements timestamp preservation by passing the timestamp parameter through the call chain from CalcRestorePlan → FindLTXFiles → LTXFiles. **Problem (Issue #771)**: When L0 files were compacted into L1 files, the compacted L1 file would get the current time instead of preserving the earliest timestamp from the source L0 files. This broke point-in-time restoration because CalcRestorePlan filters files by timestamp, causing it to skip L1 files that appeared to be created after the target restore time. **Solution**: - Added `timestamp time.Time` parameter to `LTXFiles()` interface - When `timestamp.IsZero()`: use fast timestamps (normal operations) - When `!timestamp.IsZero()`: fetch accurate timestamps (timestamp-based restore) - CalcRestorePlan now passes timestamp through the call chain to enable accurate timestamp fetching only when needed for timestamp-based restore **Implementation Details**: S3 Backend (fully implemented): - WriteLTXFile: Uses ltx.PeekHeader() to extract timestamp from LTX header, stores in S3 metadata for retrieval - LTXFiles: When timestamp requested, makes HeadObject calls to fetch metadata File Backend (fully implemented): - WriteLTXFile: Uses ltx.PeekHeader() to extract timestamp, sets via os.Chtimes() - LTXFiles: Always uses ModTime (accurate timestamp from filesystem) Other Backends (Azure, GCS, NATS, SFTP): - Signature updates for compilation - Full metadata implementation can be added in follow-up PRs **Tests**: - TestCompaction_PreservesEarliestTimestamp: Validates L1 files preserve earliest L0 timestamp after compaction - TestReplicaClient_TimestampPreservation: Integration test for all backends that runs in manual-integration-tests workflow **Performance**: - Normal operations (sync, list, compaction): O(0) extra requests - S3 timestamp-based restore: O(N) HeadObject calls (only when needed) - File/SFTP: O(0) (ModTime included in readdir) - Azure/GCS/NATS: O(0) (metadata included in LIST) Closes #771 Closes #776 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>

Mark timestamp parameter as intentionally unused with _ in backends that haven't yet implemented full metadata storage (Azure, GCS, NATS, SFTP). These backends currently use fast timestamps (LastModified/Created/ModTime) for all operations. Full metadata implementation can be added in follow-up PRs when needed. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>

github-actions · 2025-10-03T15:05:02Z

✅ Manual integration tests have been run by @corylanou

View test results

Fully implements timestamp preservation for GCS, Azure, NATS, and SFTP backends to fix issue #771 where compacted L1 files lose original timestamps and break point-in-time restoration. **GCS (gs/replica_client.go)**: - WriteLTXFile: Extract timestamp from LTX header using ltx.PeekHeader() - Store timestamp in GCS object metadata - LTXFiles: Read timestamp from metadata when doing timestamp-based restore - Metadata is included in LIST operations (no extra API calls) **Azure (abs/replica_client.go)**: - WriteLTXFile: Extract timestamp from LTX header using ltx.PeekHeader() - Store timestamp in blob metadata (uses "litestreamtimestamp" key - no hyphens) - LTXFiles: Read timestamp from metadata when doing timestamp-based restore - Metadata is included in LIST operations (no extra API calls) **NATS (nats/replica_client.go)**: - WriteLTXFile: Extract timestamp from LTX header using ltx.PeekHeader() - Store timestamp in JetStream object headers - LTXFiles: Read timestamp from headers when doing timestamp-based restore - Headers are included in LIST operations (no extra API calls) **SFTP (sftp/replica_client.go)**: - WriteLTXFile: Extract timestamp from LTX header using ltx.PeekHeader() - Set file ModTime using sftpClient.Chtimes() (similar to File backend) - LTXFiles: Use ModTime which now contains accurate timestamp All backends now preserve the earliest timestamp from source L0 files when creating compacted L1 files, ensuring point-in-time restoration works correctly across the 1GB boundary. The TestReplicaClient_TimestampPreservation integration test validates this fix for all backends in the manual-integration-tests workflow. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>

github-actions · 2025-10-03T15:37:51Z

✅ Manual integration tests have been run by @corylanou

View test results

Copilot

Pull Request Overview

This PR fixes timestamp preservation for LTX files during compaction and storage operations to ensure point-in-time restoration works correctly after compaction. The core problem was that compacted files received current timestamps instead of preserving the earliest timestamp from source files.

Key changes include:

Adding a timestamp parameter to LTXFiles interface for controlling timestamp accuracy
Implementing timestamp extraction from LTX headers during file writes
Storing timestamps in backend-specific metadata for accurate retrieval during timestamp-based restore

Reviewed Changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
replica_client.go	Added timestamp parameter to LTXFiles interface and metadata constants
s3/replica_client.go	Full implementation with S3 metadata storage and HeadObject for accurate timestamps
file/replica_client.go	Full implementation using file ModTime with Chtimes for timestamp preservation
sftp/replica_client.go	Full implementation using SFTP Chtimes for timestamp preservation
nats/replica_client.go	Full implementation storing timestamps in NATS object headers
gs/replica_client.go	Partial implementation with GCS metadata (signature update)
abs/replica_client.go	Partial implementation with Azure metadata (signature update)
replica.go	Updated CalcRestorePlan to pass timestamp for accurate retrieval
db.go	Updated normal operations to use fast timestamps
replica_client_test.go	Added timestamp preservation integration test
db_test.go	Added compaction timestamp preservation test
mock/replica_client.go	Updated mock interface signature
cmd/litestream/ltx.go	Updated CLI command to use fast timestamps
replica_test.go	Updated test mocks with new signature
store_compaction_remote_test.go	Updated test mock with new signature

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-10-03T20:47:07Z

sftp/replica_client.go

+	// Use TeeReader to peek at LTX header while preserving data for upload
+	var buf bytes.Buffer
+	teeReader := io.TeeReader(rd, &buf)
+
+	// Extract timestamp from LTX header
+	var timestamp time.Time
+	if hdr, _, err := ltx.PeekHeader(teeReader); err == nil {
+		timestamp = time.UnixMilli(hdr.Timestamp).UTC()
+	} else {
+		timestamp = time.Now().UTC() // Fallback if header read fails
+	}
+
+	// Combine buffered data with rest of reader
+	fullReader := io.MultiReader(&buf, rd)


The timestamp extraction logic is duplicated across multiple replica clients (SFTP, S3, File, NATS, GCS, Azure). Consider extracting this into a shared utility function to reduce code duplication and ensure consistent behavior.

Copilot · 2025-10-03T20:47:08Z

s3/replica_client.go

+			if !itr.timestamp.IsZero() {
+				head, err := itr.client.s3.HeadObject(itr.ctx, &s3.HeadObjectInput{
+					Bucket: aws.String(itr.client.Bucket),
+					Key:    obj.Key,
+				})
+
+				if err == nil && head.Metadata != nil {
+					if ts, ok := head.Metadata[litestream.MetadataKeyTimestamp]; ok {
+						if parsed, err := time.Parse(time.RFC3339Nano, ts); err == nil {
+							createdAt = parsed
+						}
+					}
+				}
+			}


[nitpick] The HeadObject call for each file during timestamp-based restore could be a performance bottleneck for large numbers of files. Consider batching these requests or implementing a cache mechanism if this becomes a concern in practice.

We could parallelize these calls but that's more complicated than I'd like to try to do right now.

benbjohnson · 2025-10-03T20:47:59Z

abs/replica_client.go

+	// Extract timestamp from LTX header
+	var timestamp time.Time
+	if hdr, _, err := ltx.PeekHeader(teeReader); err == nil {
+		timestamp = time.UnixMilli(hdr.Timestamp).UTC()
+	} else {
+		timestamp = time.Now().UTC() // Fallback if header read fails
+	}


I would prefer to return the error explicitly instead of falling back. If it errors out then the reader is either bad or the header is invalid.

benbjohnson · 2025-10-03T20:49:47Z

abs/replica_client.go

-func (c *ReplicaClient) LTXFiles(ctx context.Context, level int, seek ltx.TXID) (ltx.FileIterator, error) {
+// When timestamp is non-zero (timestamp-based restore), accurate timestamps are read from metadata.
+// Otherwise, fast timestamps from blob creation time are used.
+func (c *ReplicaClient) LTXFiles(ctx context.Context, level int, seek ltx.TXID, timestamp time.Time) (ltx.FileIterator, error) {


timestamp seems like it could be a bool rather than the actual timestamp. It doesn't seem like you use the actual value, do you?

benbjohnson · 2025-10-03T20:51:39Z

abs/replica_client.go

+		// Only read accurate timestamp from metadata when requested (timestamp-based restore)
+		// Azure includes metadata in LIST operations, so no extra API call needed
+		if !itr.timestamp.IsZero() && item.Metadata != nil {
+			if ts, ok := item.Metadata[litestream.MetadataKeyTimestampAzure]; ok && ts != nil {
+				if parsed, err := time.Parse(time.RFC3339Nano, *ts); err == nil {
+					createdAt = parsed
+				}
+			}
+		}


I don't think it's worth having c.timestamp in this iterator since it's zero cost to grab the correct timestamp from the metadata.

info.CreatedAt = item.Properties.CreationTime.UTC() if item.Metadata != nil { if ts, ok := item.Metadata[litestream.MetadataKeyTimestampAzure]; ok && ts != nil { if parsed, err := time.Parse(time.RFC3339Nano, *ts); err == nil { info.CreatedAt = parsed } } }

benbjohnson · 2025-10-03T20:54:36Z

db_test.go

+	// The L1 file's CreatedAt should be the earliest timestamp from the L0 files
+	// Allow for some drift due to millisecond precision in LTX headers
+	timeDiff := l1Info.CreatedAt.Sub(earliestTime)
+	if timeDiff.Abs() > time.Second {
+		t.Errorf("L1 CreatedAt = %v, earliest L0 = %v (diff: %v)", l1Info.CreatedAt, earliestTime, timeDiff)
+		t.Error("L1 file timestamp should preserve earliest source file timestamp")
+	}


I'm surprised it is using the earliest timestamp. The ltx.Compactor uses the last timestamp from the input files here: https://github.com/superfly/ltx/blob/2e6df57fc041819c837bba4f94438fec5868b85e/compactor.go#L85

The idea is that once you compact all those files together you get the state of the database of the last transaction in that file group so you'd want the last timestamp.

benbjohnson · 2025-10-03T20:55:00Z

file/replica_client.go

+	// Extract timestamp from LTX header
+	var timestamp time.Time
+	if hdr, _, err := ltx.PeekHeader(teeReader); err == nil {
+		timestamp = time.UnixMilli(hdr.Timestamp).UTC()
+	} else {
+		timestamp = time.Now().UTC() // Fallback if header read fails
+	}


Return error instead of fallback.

benbjohnson · 2025-10-03T20:56:29Z

nats/replica_client.go

+		// Use fast timestamp from object ModTime by default
+		createdAt := objInfo.ModTime
+
+		// Only read accurate timestamp from headers when requested (timestamp-based restore)
+		// NATS includes headers in LIST operations, so no extra API call needed
+		if !timestamp.IsZero() && objInfo.Headers != nil {
+			if values, ok := objInfo.Headers[litestream.HeaderKeyTimestamp]; ok && len(values) > 0 {
+				if parsed, err := time.Parse(time.RFC3339Nano, values[0]); err == nil {
+					createdAt = parsed
+				}
+			}
+		}


Always use metadata timestamp since it's zero cost.

benbjohnson · 2025-10-03T20:57:24Z

replica_client.go

+// Metadata keys for storing LTX file timestamps across different storage backends.
+const (
+	// MetadataKeyTimestamp is used by S3 and GCS for object metadata.
+	MetadataKeyTimestamp = "litestream-timestamp"
+
+	// MetadataKeyTimestampAzure is used by Azure Blob Storage (no hyphens, C# identifier rules).
+	MetadataKeyTimestampAzure = "litestreamtimestamp"
+
+	// HeaderKeyTimestamp is used by NATS for object headers.
+	HeaderKeyTimestamp = "Litestream-Timestamp"
+)


If these are specific to the implementation then I'd personally move them into their respective packages (s3, gs, etc) even if that means duplicating some.

benbjohnson · 2025-10-03T20:57:44Z

s3/replica_client.go

+	// Extract timestamp from LTX header
+	var timestamp time.Time
+	if hdr, _, err := ltx.PeekHeader(teeReader); err == nil {
+		timestamp = time.UnixMilli(hdr.Timestamp).UTC()
+	} else {
+		timestamp = time.Now().UTC() // Fallback if header read fails
+	}


Don't fallback

benbjohnson · 2025-10-03T20:58:45Z

s3/replica_client.go

+				if err == nil && head.Metadata != nil {
+					if ts, ok := head.Metadata[litestream.MetadataKeyTimestamp]; ok {
+						if parsed, err := time.Parse(time.RFC3339Nano, ts); err == nil {
+							createdAt = parsed
+						}
+					}
+				}


Return error if one occurs. We don't want to fallback to AWS last modified time if the HEAD fails for some reason.

benbjohnson · 2025-10-03T20:58:53Z

sftp/replica_client.go

+	// Extract timestamp from LTX header
+	var timestamp time.Time
+	if hdr, _, err := ltx.PeekHeader(teeReader); err == nil {
+		timestamp = time.UnixMilli(hdr.Timestamp).UTC()
+	} else {
+		timestamp = time.Now().UTC() // Fallback if header read fails
+	}


Don't fallback

corylanou and others added 3 commits October 1, 2025 14:31

corylanou mentioned this pull request Oct 3, 2025

Fix: Preserve CreatedAt timestamp during compaction #773

Closed

corylanou requested a review from benbjohnson October 3, 2025 18:04

benbjohnson requested a review from Copilot October 3, 2025 20:46

Copilot AI reviewed Oct 3, 2025

View reviewed changes

benbjohnson requested changes Oct 3, 2025

View reviewed changes

fix: preserve LTX file timestamps during compaction and storage operations #778

Are you sure you want to change the base?

fix: preserve LTX file timestamps during compaction and storage operations #778

Uh oh!

Conversation

corylanou commented Oct 3, 2025

Summary

Problem

Solution

Implementation Status

Fully Implemented Backends

Partial Implementation

Testing

Unit Tests

Manual Testing

Performance Characteristics

Breaking Changes

Related Issues

Test Plan

Uh oh!

github-actions bot commented Oct 3, 2025

Uh oh!

github-actions bot commented Oct 3, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Oct 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 3, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!