transport+visor: retire on-disk CSV transport-log store; serve history from stats bbolt#2427
Merged
0pcom merged 1 commit intoskycoin:developfrom May 4, 2026
Merged
Conversation
…y from stats bbolt The per-transport CSV log store wrote a row to a daily file on every packet via gocsv reflection-based marshalling. On a public visor that turned into the dominant alloc-rate symbol — pprof showed transport.fileTransportLogStore.writeToCSV at ~78% cumulative bytes allocated, dwarfing every other hot path once the CXO publisher's own allocation churn was fixed (skycoin#2420). The CSV files were also narrower than the bbolt-backed stats store that already exists (no latency, less precise timestamps), and the stats store was already plumbed into the CXO publisher feeding TPD's aggregator. Drop the file-backed log stores entirely: - pkg/transport/log.go — remove fileTransportLogStore and the parallel fileLatencyLogStore (the latter was wired into ManagerConfig but never read by anything — pure dead weight). CsvEntry / LatencyCsvEntry gocsv structs go with them. The in-memory LogStore stays — ManagedTransport uses it to preserve cumulative byte counters across same-session reconnects of the same transport ID. - pkg/transport/manager.go — drop the unused LatencyLogStore field on ManagerConfig. - pkg/visor/init_transport.go — collapse the file/memory branch to always-memory; drop the latency-store init. LogStore.Type=file in existing visor configs is accepted but logged as deprecated. - pkg/visor/api_transport.go — re-implement GetTransportLogs to walk stats.Store.AllTransportRecords() and emit one entry per (transport, day) within the requested window. The CSV path's RecvBytes/SentBytes was the within-day delta (CSVs were zero-anchored at each day rollover), which matches DailyRollup semantics exactly — `skywire-cli tp -b N` aggregation is unchanged. Drop the local csvEntry parser and readTransportLogFile. - pkg/visor/logserver/api.go — drop the /transport_logs/:file HTTP route and its landing-page listing. Same data is served by /stats/transports/history (live snapshot at /stats/transports). The unused tpLogPath parameter on logserver.New stays in the signature for call-site compat (init_dmsg.go passes it). - pkg/skyenv/skyenv.go — drop the unused LatencyLogStore directory constant; leave TpLogStore in place since gen.go still emits it as the LogStore.Location default for backward-compatible configs (the runtime ignores the path — nothing is written there). Cumulative bytes counters on per-transport status (skywire-cli tp ls, hypervisor UI) still reflect "bytes since transport became live in this visor session" — same in-memory semantics that file-mode users also got within a single session. Across visor restarts everyone starts fresh, which is what memory-mode users were already experiencing. Tests: package tests pass (transport, visor, visor/logserver, visor/stats, visor/visorconfig). The only remaining file-mode test (TestFileTransportLogStore) is removed along with the implementation.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Retire the per-transport CSV log store. On a public visor it was the dominant alloc-rate symbol after #2420 landed — pprof showed
transport.fileTransportLogStore.writeToCSVat ~78% of cumulative bytes allocated, going throughgocsvreflection-based marshalling for every packet on every ManagedTransport. The CSV files were also strictly narrower than data we already publish: thepkg/visor/statsbbolt store carries the same bandwidth + latency snapshots, with daily rollups already feeding the CXO publisher subscribed by the TPD aggregator.This PR removes the file-backed store entirely and re-points
GetTransportLogs()(the only consumer of the CSVs — used byskywire-cli tp -b Nfor bandwidth history) at the bbolt stats store.Changes
Removed code
pkg/transport/log.go— dropfileTransportLogStore(writeToCSV / readFromCSV / recoverCSV / repairCSVFile / cleanLogs). Drop the parallelfileLatencyLogStorealong with its interfaceLatencyLogStore,inMemoryLatencyLogStore,LatencyCsvEntry— the manager held aLatencyLogStorefield but nothing in the codebase ever calledRecordon it, pure dead weight.CsvEntryand thegocsv/encoding/csv/bufioimports go with them.pkg/transport/manager.go— drop the unusedLatencyLogStorefield onManagerConfig.pkg/transport/log_test.go— dropTestFileTransportLogStore.pkg/visor/init_transport.go— collapse the file/memory branch to always-memory; drop the latency-store init.LogStore.Type=\"file\"in existing visor configs is accepted but logged as deprecated.pkg/visor/logserver/api.go— drop the/transport_logs/:fileHTTP route and its landing-page listing. The same data is at/stats/transports/history(live snapshot at/stats/transports). UnusedtpLogPathparameter onlogserver.Newstays in the signature for call-site compat (init_dmsg.gopasses it).pkg/skyenv/skyenv.go— drop the unusedLatencyLogStoredirectory constant.TpLogStorestays sincecmd/skywire-cli/commands/config/gen.gostill emits it as the defaultLogStore.Locationfor backward-compatible configs.Re-pointed read path
pkg/visor/api_transport.go—GetTransportLogs(days int)now walksstats.Store.AllTransportRecords()and emits one entry per (transport, day) within the requested window. The CSV store'sRecvBytes/SentByteswas the within-day delta (each daily file was zero-anchored at day rollover via the explicitReset()inwriteToCSV), which matchesDailyRollup's delta semantics exactly.skywire-cli tp -b Naggregation is unchanged. Drop the localcsvEntryparser andreadTransportLogFile.Diff
Behavioural notes
skywire-cli tp ls, hypervisor UI per-transport bytes): same in-memory semantics as today's memory-mode users — "bytes since transport became live in this visor session." File-mode users used to keep counters across visor restarts via the daily CSV; that no longer happens. Daily totals viatp -b Nare unaffected because bbolt is the source of truth./transport_logs/<date>.csvHTTP endpoint is removed. Functionally replaced by/stats/transports/history?since=&until=&id=already wired into the same logserver.Test plan
go test ./pkg/transport/...(transport, addrresolver, handshake, tpdclient — all green)go test ./pkg/visor/...(visor, dmsgtracker, logserver, stats, usermanager, visorconfig — all green)go build ./...— whole-repo build cleanskywire -bruns on the resulting binarywriteToCSVis gone from the alloc profile; verifyskywire-cli tp -b 7still returns sensible per-day bandwidth aggregated from the bbolt store