chadtmiller.com/knotless

Switch repository

Loading…

What changes

A failed write keeps its lines. Logs::flush took the held lines and then wrote them, so a write that failed lost the batch, although wrote() said they would be written with the next one. Now a failed write puts the batch back, ahead of the lines logged since, still capped at PENDING_MAX with the oldest dropped first. The write itself moved to Logs::write, one transaction, all or none.

A trace reads whole. A read by trace now defaults to 500 lines, the most a read answers, instead of the last 100, so a long request's first lines aren't cut off. Other reads still default to 100, and limit still sets the number. getLogs' limit drops its declared default: 100 and its description gives both defaults instead. The api and cli reference pages say the same.

Why

These are the two points the review left open on An operator reads the appview's logs without SSH, which merged without them. A failed write is likely during a deploy, when two serves share the file and one can wait past the 5 s busy timeout. That's when an operator most wants the lines.

How it was checked

There are two new unit tests in logs.rs:

• a_failed_write_keeps_its_lines makes a write fail by renaming the table from a second connection. The lines held come back ahead of a later one once the table is back, and the cap still holds after a failed write.
• a_trace_reads_whole reads 150 lines of one trace whole, 10 when asked, and 100 without a trace.

Left to CI.

Shortcomings

A batch that keeps failing is still capped at PENDING_MAX. Past that, the oldest lines go, as before.

🤖 Generated with Claude Code

This pull puts a failed Logs::flush batch back ahead of the lines logged since, then caps it at PENDING_MAX and drops the oldest first. A read by trace now defaults to LIMIT_MAX (500), and other reads stay at 100. I read the diff closely and found nothing that must change.

• Failed write: write runs in a single transaction. A failed write drops the transaction, which rolls it back, so a retry can't double-insert. The requeue order and the drain cap are correct. The rename trick in the test is a sound way to force a failure.
• Trace default: the read change matches the doc comments, the api and cli references, and the changeset. The lexicon text now states both defaults.
• Not checked: test and lexicons hadn't finished when I looked. Please confirm that the lexicons check accepts dropping default: 100 from getLogs. As I read harness/lexicons.mjs, only removed fields, enum values and similar are flagged, so it should pass.