Basekick-Labs / Basekick-Labs/arc

fix(compaction): daily tier flattens source keys by basename, so a collision duplicates one file's rows and silently drops another's

Open
#826 0 comments 0 reactions 0 assignees View on GitHub
bug
Dominant language
Go
Stars
677
Forks
53
Avg merge
9h 14m
Merged PRs (30d)
164

Description

## Summary

Daily compaction flattens every source key into one temp directory by basename, with no collision check. Two source files that share a basename collide: one file's rows are duplicated into the output, the other's are dropped, and **both originals are marked compacted and deleted**.

Split out of #750 (item 1) so it can be milestoned and closed on its own. Same shape as the already-closed #357.

## The code

`internal/compaction/job.go:591`:

```go
localPath := filepath.Join(tempDir, filepath.Base(fileKey))

// Create local file for streaming
file, err := os.Create(localPath)
```

`os.Create` truncates an existing file. There is no check that `localPath` was not already written by another source key in the same job.

## Why basenames collide

The daily tier's candidate set spans all 24 hour subdirectories (`internal/compaction/daily.go:150-218`), so one job downloads files from 24 different prefixes into one flat temp dir.

Basenames are built from wall-clock only — `internal/ingest/arrow_writer.go:3753-3760`:

```go
now := time.Now().UTC()
timestamp := now.Format("20060102_150405")
nanos := now.UnixNano() % 1_000_000_000

return fmt.Sprintf("%s/%s/%s/%s/%s/%s/%s_%s_%09d.parquet",
database, measurement, year, month, day, hour, measurement, timestamp, nanos)
```

The basename is `{measurement}_{YYYYMMDD_HHMMSS}_{nanos}.parquet`. It carries **no node ID, no PID, and no random component** — only the flush wall-clock. The partition hour that distinguishes the two keys is in the *directory* portion, which `filepath.Base` discards.

Note `nanos` is `UnixNano() % 1_000_000_000`, the sub-second remainder, so it repeats every second rather than being globally unique.

## Impact

Silent, and it destroys data: the dropped file's rows are gone, the duplicated file's rows are double-counted, and both source files are deleted as successfully compacted. Nothing errors.

## Suggested fix

Do not derive the temp filename from `filepath.Base`. The full key is already unambiguous — use a name derived from the whole `fileKey` (hash it, or mirror the key's directory structure under `tempDir`), or add an index to the local name.

Whichever is chosen, add a collision assertion: if the target local path already exists, fail the job rather than truncating.

## Test plan

- [ ] Unit test: two source keys differing only in the hour segment, asserting both land at distinct local paths
- [ ] Regression test must FAIL pre-fix (revert-run-restore)
- [ ] Integration: daily compaction over two hours holding same-basename files, asserting output row count equals the sum of inputs

Split from #750. Related: #357 (same shape, closed), #749 (temp-dir cleanup, same package).

Contributor guide

Open the contributing guide

Research direction

Start at internal/compaction/job.go:591 and trace the daily candidate set in internal/compaction/daily.go:150-218, using the basename construction in internal/ingest/arrow_writer.go:3753-3760 as context. Add tests for two source keys with the same basename, including a collision assertion and the daily integration case. Done means both inputs use distinct local paths and the output row count equals their combined rows without silently deleting either file.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
databases
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
74/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.