fix(backend): give each report file a name of its own (closes #61) #66

Open
clawbot wants to merge 1 commits from fix/report-file-names into next
Collaborator

Report files in DATA_DIR were named by a millisecond timestamp and created with exclusive create, so two flushes in the same millisecond, such as a flush for size and the final flush at shutdown, got the same name. The second create failed, and since #58 that fails the stop and loses the batch.

Names now carry a number after the timestamp, for example reports-2026-09-29T12-00-00.000Z-7.jsonl.zst. The number goes up by one for each file the server starts to write. A failed write uses up its number, leaving a gap if the file could not be created and otherwise a file under that number that may be incomplete. The number is taken atomically, so no two files of one run share a name. The timestamp still comes first at a fixed width, so names sort by time as before.

What the diff does not show:

  • The number restarts at 1 on every start. A later run can reuse a name only if the clock is set back to the exact millisecond of an earlier file.
  • The number is not zero-padded, so within one millisecond -10 sorts before -9. Across milliseconds the order is by time.
  • Files already on disk under the old names still count toward DATA_DIR_MAX_BYTES; the test that places an old-style name is unchanged.
  • The buffer reads the time for file names through a clock held in the buffer, time.Now outside tests. The new test stops it through StopClock in export_test.go, so its two flushes share one timestamp on every run; with the old naming it fails every time.
  • The test reads the files through os.DirFS because gosec flags os.ReadFile on a variable path; no rule is suppressed.

Model: opus-5-5

Report files in `DATA_DIR` were named by a millisecond timestamp and created with exclusive create, so two flushes in the same millisecond, such as a flush for size and the final flush at shutdown, got the same name. The second create failed, and since https://git.eeqj.de/sneak/netwatch/pulls/58 that fails the stop and loses the batch. Names now carry a number after the timestamp, for example `reports-2026-09-29T12-00-00.000Z-7.jsonl.zst`. The number goes up by one for each file the server starts to write. A failed write uses up its number, leaving a gap if the file could not be created and otherwise a file under that number that may be incomplete. The number is taken atomically, so no two files of one run share a name. The timestamp still comes first at a fixed width, so names sort by time as before. What the diff does not show: - The number restarts at 1 on every start. A later run can reuse a name only if the clock is set back to the exact millisecond of an earlier file. - The number is not zero-padded, so within one millisecond `-10` sorts before `-9`. Across milliseconds the order is by time. - Files already on disk under the old names still count toward `DATA_DIR_MAX_BYTES`; the test that places an old-style name is unchanged. - The buffer reads the time for file names through a clock held in the buffer, `time.Now` outside tests. The new test stops it through `StopClock` in `export_test.go`, so its two flushes share one timestamp on every run; with the old naming it fails every time. - The test reads the files through `os.DirFS` because gosec flags `os.ReadFile` on a variable path; no rule is suppressed. Model: opus-5-5
clawbot added the needs-review label 2026-09-29 04:32:35 +02:00
clawbot self-assigned this 2026-09-29 04:32:35 +02:00
clawbot force-pushed fix/report-file-names from c88c5da063 to 0987817aae 2026-09-29 05:31:55 +02:00 Compare
clawbot added needs-rework and removed needs-review labels 2026-09-29 05:39:20 +02:00
Author
Collaborator

Before review: the new test depends on two real flushes landing in the same millisecond, and it failed one of three image builds after the rebase. Rework it so it does not depend on timing (a fixed time source for the test, or an equivalent that forces two flushes onto the same timestamp every run), with no retry loop, and confirm it still fails with the old naming.

Model: opus-5-5

Before review: the new test depends on two real flushes landing in the same millisecond, and it failed one of three image builds after the rebase. Rework it so it does not depend on timing (a fixed time source for the test, or an equivalent that forces two flushes onto the same timestamp every run), with no retry loop, and confirm it still fails with the old naming. Model: opus-5-5
clawbot force-pushed fix/report-file-names from 0987817aae to 481423e6ec 2026-09-29 05:45:50 +02:00 Compare
clawbot added needs-review and removed needs-rework labels 2026-09-29 05:46:08 +02:00
Author
Collaborator

Reworked: the new test now stops the buffer's clock so its two flushes share one timestamp on every run, with no retry loop; it fails every time with the old naming.

Model: opus-5-5

Reworked: the new test now stops the buffer's clock so its two flushes share one timestamp on every run, with no retry loop; it fails every time with the old naming. Model: opus-5-5
Author
Collaborator
  1. backend/README.md (Report storage) and the new entry in TODO.md say the number counts the files the server has written since it started. It does not: writeFile in backend/internal/reportbuf/reportbuf.go takes the number before it creates the file, so a write that fails still uses a number up and the next file skips it. An operator reading the names after a failed flush would take the gap for a file that was written and later deleted. Acceptable: both say the number goes up by one for each file the server starts to write, so a failed write leaves a gap in the numbers.

Model: opus-5-5

1. `backend/README.md` (Report storage) and the new entry in `TODO.md` say the number counts the files the server has written since it started. It does not: `writeFile` in `backend/internal/reportbuf/reportbuf.go` takes the number before it creates the file, so a write that fails still uses a number up and the next file skips it. An operator reading the names after a failed flush would take the gap for a file that was written and later deleted. Acceptable: both say the number goes up by one for each file the server starts to write, so a failed write leaves a gap in the numbers. Model: opus-5-5
clawbot added needs-rework and removed needs-review labels 2026-09-29 06:22:11 +02:00
clawbot force-pushed fix/report-file-names from 481423e6ec to f665ab17bc 2026-09-29 06:24:17 +02:00 Compare
clawbot added needs-review and removed needs-rework labels 2026-09-29 06:24:36 +02:00
Author
Collaborator
  1. Fixed: backend/README.md, TODO.md, the commit message and the PR body now say the number goes up by one for each file the server starts to write, so a failed write leaves a gap in the numbers.

Model: opus-5-5

1. Fixed: `backend/README.md`, `TODO.md`, the commit message and the PR body now say the number goes up by one for each file the server starts to write, so a failed write leaves a gap in the numbers. Model: opus-5-5
clawbot force-pushed fix/report-file-names from f665ab17bc to 304cf79826 2026-09-29 07:11:04 +02:00 Compare
Author
Collaborator
  1. backend/README.md (Report storage), the new entry in TODO.md, the commit message and the PR body say a failed write leaves a gap in the numbers. That is true only when the file cannot be created. writeFile in backend/internal/reportbuf/reportbuf.go creates the file before it compresses the reports into it and never removes it after a later error, so a write that fails partway, for example when the disk fills up, leaves an empty or cut-off file under its number and no gap. An operator told that failed writes show as gaps would take that file for a complete one. Acceptable: all four say a failed write uses up its number, leaving a gap if the file could not be created and otherwise an incomplete file under that number.

Model: opus-5-5

1. `backend/README.md` (Report storage), the new entry in `TODO.md`, the commit message and the PR body say a failed write leaves a gap in the numbers. That is true only when the file cannot be created. `writeFile` in `backend/internal/reportbuf/reportbuf.go` creates the file before it compresses the reports into it and never removes it after a later error, so a write that fails partway, for example when the disk fills up, leaves an empty or cut-off file under its number and no gap. An operator told that failed writes show as gaps would take that file for a complete one. Acceptable: all four say a failed write uses up its number, leaving a gap if the file could not be created and otherwise an incomplete file under that number. Model: opus-5-5
clawbot added needs-rework and removed needs-review labels 2026-09-29 07:39:23 +02:00
clawbot added 1 commit 2026-09-29 07:42:02 +02:00
Report files were named by a millisecond timestamp and created with
O_EXCL, so two flushes in the same millisecond, such as a flush for
size and the final flush at shutdown, got the same name and the second
failed, losing its reports. Each name now carries a number after the
timestamp that goes up by one for each file the server starts to
write, so names still sort by time and never repeat within a run. A
failed write uses up its number, leaving a gap if the file could not
be created and otherwise a file under that number that may be
incomplete. The buffer reads the time through a clock the new test
stops, so its two flushes share one timestamp on every run; the 1 ms
pauses earlier tests used to dodge the collision are gone.

Model: opus-5-5
clawbot force-pushed fix/report-file-names from 304cf79826 to 93d4062794 2026-09-29 07:42:02 +02:00 Compare
clawbot added needs-review and removed needs-rework labels 2026-09-29 07:42:14 +02:00
Author
Collaborator
  1. Fixed: backend/README.md, TODO.md, the commit message and the PR body now say a failed write uses up its number, leaving a gap if the file could not be created and otherwise a file under that number that may be incomplete; "may be" because a failure in writeFile after the reports are fully written, when reading the file's size or closing it, can leave the file complete.

Model: opus-5-5

1. Fixed: `backend/README.md`, `TODO.md`, the commit message and the PR body now say a failed write uses up its number, leaving a gap if the file could not be created and otherwise a file under that number that may be incomplete; "may be" because a failure in `writeFile` after the reports are fully written, when reading the file's size or closing it, can leave the file complete. Model: opus-5-5
Author
Collaborator
  1. Commit message of 93d40627: the body runs to about 140 words, over the limit of about 120 for a commit body. Acceptable: about 120 words or fewer, for example by dropping the closing sentence on the test clock and the removed 1 ms pauses, which the diff already shows. The sentences about the file names stay as they are.
  2. PR description: it runs to about 285 words, over the limit of about 250 for a PR body. Acceptable: about 250 words or fewer, for example by dropping the bullet on the clock held in the buffer, which the diff already shows. The sentences about the file names stay as they are.

Model: opus-5-5

1. Commit message of `93d40627`: the body runs to about 140 words, over the limit of about 120 for a commit body. Acceptable: about 120 words or fewer, for example by dropping the closing sentence on the test clock and the removed 1 ms pauses, which the diff already shows. The sentences about the file names stay as they are. 2. PR description: it runs to about 285 words, over the limit of about 250 for a PR body. Acceptable: about 250 words or fewer, for example by dropping the bullet on the clock held in the buffer, which the diff already shows. The sentences about the file names stay as they are. Model: opus-5-5
All checks were successful
check / check (push) Successful in 48s
You are not authorized to merge this pull request.
This pull request can be merged automatically.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin fix/report-file-names:fix/report-file-names
git checkout fix/report-file-names
Sign in to join this conversation.
No Reviewers
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/netwatch#66