You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
🟢 85% lib/voyager/services/tracer.ex:30 — the # ponytail: comment carries the tool's prefix and a note about future work ("per-key LRU if pairs go missing"). Cut it down to the reason only.
🟢 85% lib/voyager/services/tracer.ex:300 — the # ponytail: comment has the same prefix and a future-tense "prune per batch if that bites". Either drop it or keep one line stating the disk peak.
🟡 85% lib/voyager/services/tracer.ex:429: no test covers the new :storage_limit stop (stop_over_limit/1). Add a tracer test that sets a tiny :trace_storage_limit_mb and asserts the run stops with :storage_limit.
🟢 65% lib/voyager/actions/trace_sessions.ex:77: interrupt_running/0 sets no stopped_at and writes no :stopped entry. A loaded interrupted session has a log ending at :started and stopped_at: nil.
🦐 70% lib/voyager/schemas/trace_session.ex:40-41: delete: unused has_many :session_events / :events. The FK on_delete: :delete_all already cascades.
🟡 80% lib/voyager/services/tracer.ex:170: entries/2 still specs pos_integer(), but unsaved sessions now get negative ids. Change it to integer().
🦐 65% lib/voyager/schemas/trace_session.ex:46-57: yagni: the changeset casts ten fields, but create/1 only passes origin and status. Use cast(attrs, [:origin, :status]).
No findings above the confidence threshold. The earlier points (the entries/2 spec, the changeset cast, stopped_at on interrupt, the storage-limit test, the unused associations) are all fixed.
Row encoding was the Writer's CPU ceiling. Rows are now prepared on
the task supervisor while the single inserter runs, and a full queue is
flushed at once, so the pending-entries backpressure only trips when the
database really lags.
Flush the surviving Writer before interrupting sessions on a Tracer
restart, drain the Writer on shutdown, drop open-call stacks when events
were dropped, bound retained timestamps instead of keys, and make the
recursive duration test tolerant of equal microsecond timestamps.
Include serialized configuration size in storage estimates
lib/voyager/actions/trace_entries.ex:121
🟡 should-fix · 98%: Configured entries store their config as a map, so this filter excludes the entire serialized config from the storage estimate. Short saved sessions configured with many PIDs can consequently store kilobytes per configuration while counting only the fixed overhead and node string, substantially exceeding the byte-based stop/pruning limit. Include the encoded config's size in the total.
Apply Writer backpressure only to saved sessions
lib/voyager/services/tracer.ex:612
🟡 should-fix · 97%: The global Writer backlog also makes unsaved runs shed events. A saved run can stop with more than 10,000 rows still queued, and create/2 does not flush them, so an immediately started save: false session drops traces despite never using the Writer and having an empty mailbox. Pass the session's save flag into this check and apply Writer backpressure only to saved sessions.
A marker per shed batch still queued on the overloaded Writer, so
sustained overload kept growing its queue. Shed counts are now logged
as one entry once the Tracer stops shedding.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Non-transactional table reads can permanently skip trace entries, while write failures after a run stops can silently discard batches and leave stale saved data despite flush reporting success.
No open issues 🎉
1 previously missed issue
In code that hasn't changed since last review
Read trace tables in one transaction for consistent pagination
lib/voyager/queries/trace_entries.ex:21
🔴 blocking · 98%: The Writer can commit between these two table reads. An ascending page can then contain trace seq 2 but omit the configured/started entries at seq 0 and 1 committed in the same batch; advancing since skips those entries permanently. Read both tables in one transaction so the merged page uses a consistent snapshot.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
kraleppa
left a comment
srzeszut
left a comment
No description provided.