perf(app): read the version list and the running version in one round trip per host - #164
Merged
Merged
Conversation
… trip per host `dash app stale_containers` asked each (host, role) two questions in two SSH round trips: every version with a container, then the version running now. The deploy runs this with --stop before every boot, so a 4-host fleet paid 8 round trips to produce 4 small lists. Dash::Commands::App#stale_state chains the two builders around the same separator #boot_state uses; the CLI captures it once and splits. The split now lives beside the separator as Dash::Commands::App.split_state, shared with Boot#capture_boot_state, so the escape and anchor logic exists once. Which containers count as stale, the stop arguments, the lock behaviour and the printed lines are unchanged. Refs #162 Claude-Session: https://claude.ai/code/session_015NuGuQJZn6d7wYvWizV1Lw
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
A fold, not a behaviour change.
dash app stale_containersasked each (host, role) two questions in two SSH round trips — every version with a container, then the version running now. The deploy runs it with--stopbefore every boot, so every deploy paid twice per host.lib/dash/commands/app.rb—Dash::Commands::App#stale_statechainslist_versions, anechoofBOOT_STATE_SEPARATOR, andcurrent_running_version, the same shape asboot_state. NewDash::Commands::App.split_state(output)returns the two raw halves either side of the separator line.lib/dash/cli/app.rb—stale_containerscapturesstale_stateonce per (host, role) and splits. Stopping stays oneexecuteper stale version; on a healthy fleet that list is empty.lib/dash/cli/app/boot.rb—capture_boot_stateuses the samesplit_state, so the separator's escape/anchor logic lives once, beside the constant that defines it.Which containers count as stale, the stop arguments,
raise_on_non_zero_exit: false, the lock behaviour, and the printed lines are unchanged. The cost-guard sequence intest/cli/main_test.rbis untouched (app:stale_containersis stubbed there).Closes #162
Measured
Integration harness,
app_with_roles(web on vm1+vm2, workers on vm3 = 3 host/role pairs), same machine,origin/main(90c6663) vs this branch.Detect stale containers 0.1s 6 ssh 0.4sDetect stale containers 0.1s 3 ssh 0.3sDetect stale containers 0.2s 6 ssh 0.5sDetect stale containers 0.1s 3 ssh 0.2sOne round trip per (host, role), as the issue asked. The issue's own 4-host baseline (
8 ssh) becomes 4 on that topology by the same arithmetic; that fleet was not re-run here. Wall-clock on the harness is noise-level either way — the count is the measured quantity.Test plan
test/commands/app_test.rb— exactstale_statestring;split_statehalves, including that only a whole separator line splitstest/cli/app_test.rb— stale detected and stopped exactly as before; empty output on both halves means nothing stale;--quietdrops the host header; one capture per (host, role) ondeploy_with_roles.yml(4 pairs → 4 captures, was 8)bundle exec rubocop --parallel— no offensesbin/test— 1897 runs, 0 failures (Docker + published proxy image)Deviations & judgment calls
--quietnever silenced the stale lines.puts_by_host(quiet:)(lib/dash/sshkit_with_ext.rb:43) only drops theApp Host: <host>header; the finding always prints. The issue's gate said "--quietstill silent" — it was never silent. The test pins the real contract instead (header gone, "Detected stale container …" still printed). Behaviour is unchanged by this PR either way.Dash::Commands::App.split_staterather than repeating thepartition(/^#{Regexp.escape(SEPARATOR)}$/)line in a second file. The issue allowed either ("extract a tiny shared helper if both call sites end up identical, otherwise three lines is fine"); the split line is identical, and keeping it beside the constant means the escape and anchor logic exists once. Post-processing stays per-caller: boot wants twopresences, stale wants a list minus one. This toucheslib/dash/cli/app/boot.rb, which the issue did not list.https://claude.ai/code/session_015NuGuQJZn6d7wYvWizV1Lw
Summary by cubic
Halves the SSH round trips
dash app stale_containersmakes per (host, role) by reading the version list and the running version in a single capture instead of two. Stale-detection behavior, stop arguments, lock handling, and printed lines are unchanged, so every deploy that runs with--stopnow pays half the SSH cost.Refactors
Dash::Commands::App.split_stateand reuses it in boot-state capture so the separator parsing lives in one place.Closes #162.
Written for commit 8688f5d. Summary will update on new commits.