Skip to content

Reduce functional-test metric memory and fix matching lifetimes - #68

Draft
chaptersix wants to merge 2 commits into
mainfrom
fix/test-metric-capture-memory
Draft

chaptersix wants to merge 2 commits into
mainfrom
fix/test-metric-capture-memory

Conversation

@chaptersix

@chaptersix chaptersix commented Sep 13, 2026

Copy link
Copy Markdown
Owner

Summary

Fix functional-test metric capture memory, matching backlog-manager goroutine lifetimes during migration, and matching timeout defaults.

The first commit is the metric-capture fix. It bounds captured metric samples and changes the test harness to retain only the aggregate data needed by assertions, with focused capture-handler and harness regression tests.

The priority and fairness backlog managers now own child contexts and cancel them idempotently after final persistence. Drain completion serializes metadata update, final GC, and draining-manager shutdown while leaving the physical queue and active manager usable.

The common functional harness now uses 30s task-queue idle expiry and 5s task/user-data long polls. Tests that require longer queue lifetimes retain explicit overrides, including task-queue statistics. The opt-in memory probe pins all three values explicitly.

Validation

  • Focused lifecycle tests: 20 repetitions, including race mode
  • go test -tags test_dep ./service/matching -count=1
  • go test -tags test_dep ./tests/testcore -count=1
  • Selected SQLite functional coverage for worker shutdown, forwarding, Nexus, versioning, unload/reload, and queue statistics
  • NDC/XDC consumers of the shared harness
  • make GOLANGCI_LINT_FIX=false lint-code-fast

Lifecycle A/B at 120 seconds retained all 96 physical partitions: the unchanged control retained 96 fairness writer goroutines, while the fix retained zero. The coordinated timeout probe returned matching goroutines to baseline in 90 seconds.

@chaptersix

chaptersix commented Sep 13, 2026

Copy link
Copy Markdown
Owner Author

How these issues were found

This work started as a matching memory and goroutine-retention investigation. The opt-in probe creates a fresh SQLite test cluster, uses four read and four write partitions, starts 12 SDK workers on separate queues, completes one workflow and activity per queue, stops the workers, and samples goroutine profiles and process memory without issuing DescribeTaskQueue calls that could refresh queue liveness. Samples force Go GC and record live heap, stack memory, total and matching goroutine counts, and /proc/self/smaps_rollup RSS.

The metric-capture memory issue was identified while reviewing the same functional-test memory profile. Capturing every emitted sample kept high-cardinality metric data alive for the lifetime of the test environment, so the capture path was changed to retain only the aggregate data needed by assertions. The capture-handler and testcore changes, including their focused regression coverage, are the first commit in this PR (388216d6f).

Production lifecycle issue

With the production timeout values, stopping the workers left 1,001 matching goroutines. At 120 seconds, 96 physical partitions were still loaded. The goroutine profile showed 96 fairTaskWriter.taskWriterLoop goroutines and 96 priBacklogManager.periodicSync goroutines in the unchanged control. The drain-completion logs had already run, so these were work owned by a draining backlog manager that had been detached but was still waiting on the physical queue context.

The failure was reduced to a focused regression test covering both migration directions: priority active with fairness draining, and fairness active with priority draining. The test waits for both managers to initialize, completes draining, asserts that only the draining manager's context is canceled, verifies that the physical queue and active manager remain live, and dispatches a task through the active manager. It fails on unchanged production code because the draining context remains live. Additional cases cover uninitialized shutdown, skipped and failed final persistence, concurrent and repeated Stop(), and shutdown racing drain completion.

The fix gives each priority and fairness backlog manager an owned child context derived from the physical queue context. Stop() is guarded by sync.Once, performs the existing final persistence attempt first, and defers cancellation so every path—including uninitialized, skipped, failed, or lost-ownership paths—releases the manager's background work. Drain completion now serializes active-manager metadata update, draining-manager final GC, and draining-manager stop. The physical queue and active manager are not canceled by drain completion.

The corrected A/B used identical production timeouts and the same 120-second observation horizon. Both runs retained all 96 physical partitions. The unchanged control retained 905 matching goroutines, including 96 fairness writers; the fixed run retained 809 matching goroutines and zero fairness writers. This isolates the lifecycle cleanup from normal physical-queue eviction.

Functional-test retention issue

The same workload exposed a separate harness problem. Under the existing test defaults—five-minute task-queue idle expiry, 60-second task long polls, and a 290-second user-data long poll—matching goroutines remained elevated for roughly 14 minutes after worker shutdown. The probe returned to its baseline of five matching goroutines only after the queues became idle and evicted.

The timeout comparison coordinated the three settings: 30 seconds for task-queue idle expiry and five seconds for both long-poll limits. With the same 12 queues and worker lifecycle, matching goroutines fell from 1,001 after shutdown to 245 at 30 seconds, 101 at 60 seconds, and five at 90 seconds. The probe explicitly sets all three values in both control and short scenarios, so the control cannot silently inherit the new harness defaults.

The common harness now uses 30s/5s/5s. Tests that intentionally need longer queue lifetimes retain explicit overrides; for example, task-queue statistics uses a five-minute idle timeout with its existing 10-second long poll, and the versioning eviction test keeps its explicit five-second eviction setup and keepalive behavior. No production timeout defaults, partition counts, cluster-pool sizes, or forwarding/versioning coverage were changed.

Verification

The focused lifecycle tests passed 20 repetitions normally and 20 repetitions under the race detector. The full service/matching and tests/testcore unit packages passed. Selected SQLite coverage passed for worker shutdown and outstanding polls, workflow/activity forwarding and dispatch latency, queue statistics, Nexus polling, versioning updates, and idle unload/reload; NDC and XDC consumers were also exercised. make GOLANGCI_LINT_FIX=false lint-code-fast passed with the pinned lint and vet tools rebuilt under Go 1.27.1.

The probe records RSS separately from Go live heap and stack memory. SQLite allocation counters are unavailable in this build because SQLite memstatus is disabled, and the small empty-backlog heap difference should not be extrapolated to explain multi-gigabyte process memory.

@chaptersix chaptersix changed the title Fix matching backlog manager lifetimes and test timeouts Reduce functional-test metric memory and fix matching lifetimes Sep 13, 2026
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.

1 participant