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
- 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
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