Skip to content

fix(tune): add configurable timeout - #30

Merged
alexshapalov merged 2 commits into
pgrundev:mainfrom
YIKUAIBANZI:fix/tune-timeout
Sep 5, 2026
Merged

fix(tune): add configurable timeout#30
alexshapalov merged 2 commits into
pgrundev:mainfrom
YIKUAIBANZI:fix/tune-timeout

Conversation

@YIKUAIBANZI

Copy link
Copy Markdown
Contributor

Summary

  • add tune --timeout with the same 30-second default and Go duration syntax used by the other collection commands
  • apply the selected budget to both the command context and collect.Options.Deadline
  • cover the CLI behavior with a real stalled TCP connection and protect the gather-to-collector deadline mapping

Why

tune hard-coded a 30-second command context and did not register a timeout flag. In addition, gather did not forward inspectFlags.timeout, so the collector could still apply its shorter fallback deadline even if the outer command budget was increased.

The first commit in #28 independently contains the shared gather deadline forwarding. This focused fix includes that necessary mapping because tune --timeout values above the collector fallback would otherwise remain ineffective; if #28 lands first, the overlapping line can be dropped during rebase.

Verification

  • go test ./... -count=1
  • go test -race ./... -count=1
  • go vet ./...
  • golangci-lint v2.12.2 run ./...
  • scripts/gate.sh (isolated committed HEAD; Linux/macOS on amd64/arm64)

govulncheck ./... also ran, but the local Go 1.26.3 standard library reports advisories fixed in later Go patch releases; this change adds no dependencies.

Fixes #26

YIKUAIBANZI and others added 2 commits September 3, 2026 17:10
…apping

Follow-up to the tune --timeout fix (pgrundev#26): the deadline-only check would still
pass if gatherOptions dropped interval/ASH fields, so compare the whole
collect.Options; assert every inspectFlags-based collection command exposes
--timeout with the same 30s default; cover duration parsing. Changelog entry
under Unreleased, and refresh the two comments the review found stale (gather's
caller list, Options.Deadline's "5s" default that has been 20s since the
remote-database change).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013qGZKWgfGTBCoHsDjy1SuB
@alexshapalov

Copy link
Copy Markdown
Contributor

Code review

No issues found. Checked for bugs, git history of the touched code, prior PRs/issues (#26, #28), and code-comment guidance; this repo has no CLAUDE.md.

Notes for the record:

  • Forwarding Deadline: f.timeout from gather is the right call and matches what inspect already does: the collector nests its own context.WithTimeout inside the command context, so the effective budget is the smaller of the two, never additive. It also makes --timeout above the collector's 20s+interval fallback take effect on indexes, queries, tables, and vacuum, which it silently did not before.
  • feat(conn): --ssh-tunnel — reach a database through an SSH jump host #28 edits the same line inline; whichever lands second will need a trivial conflict resolution, no logic difference.
  • Pushed e1b0f63 to this branch (maintainer edits): tests pinning the full gatherOptions field mapping and the shared 30s --timeout default across collection commands, a CHANGELOG.md entry under Unreleased, and refreshed two stale comments (gather caller list, Options.Deadline default).

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

@alexshapalov
alexshapalov merged commit 3203e6d into pgrundev:main Sep 5, 2026
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Provide --timeout for tune

2 participants