Skip to content

fix: Fix shared trace dispatcher lifecycle and discovery - #1243

Merged
ShannonDing merged 17 commits into
apache:masterfrom
wenxuwan:fix_dispatcher_panic
Oct 8, 2026
Merged

ShannonDing merged 17 commits into
apache:masterfrom
wenxuwan:fix_dispatcher_panic

Conversation

@wenxuwan

Copy link
Copy Markdown
Member

What is the purpose of the change

Fix trace dispatcher failures when consumers are recreated in a long-running process after NameServer discovery changes.

Previously, trace initialization could return a typed nil dispatcher, causing the interceptor to panic before invoking the business callback.
Closing a dispatcher also left its underlying client behind.

This change makes unavailable tracing preserve business callback execution and gives shared trace resources an explicit lifecycle, while retaining
the existing WithTrace API and TraceConfig layout.

Brief changelog

  • Add optional WithSharedTrace support for sharing trace resources by stable logical destination, with additional isolation by unit, access
    channel and credentials. An independent resolver factory manages discovery ownership.
  • Reference-count shared NameServer and Broker transports. Make dispatcher startup and shutdown idempotent, drain pending records with bounded
    shutdown, and release resources after the final user closes.
  • Release trace ownership when consumer or producer construction/startup fails.
  • Handle nil and typed-nil dispatchers without preventing business operations or replacing their results/errors.
  • Share discovery state and periodically refresh trace topic routes, including cloud region topics.
  • Clone cached route snapshots before sorting publish queues to prevent races between dispatchers and background refresh.
  • Add lifecycle, failure-path and concurrent discovery tests, plus usage documentation in docs/trace.md.

The general producer/consumer client registry and its NameServer conflict checks remain unchanged.

Verifying this change

  • Trace regression tests passed with -race in internal, consumer and producer.
  • Added an 8-Broker concurrent route-refresh test that reproduces the race before the fix.
  • Cache immutability and concurrent multi-Broker refresh tests passed five consecutive runs with -race after the fix.
  • git diff --check passed.

Full-suite success is not claimed. The full run encountered an asynchronous mock expectation failure in consumer.TestStart and a timeout in
internal/remote.TestInvokeAsyncTimeout. Both were independently reproduced against the unmodified upstream baseline.

Checklist

  • File and link the GitHub issue addressed by this PR.
  • Format the title as [ISSUE #<number>] Fix shared trace dispatcher lifecycle and discovery.
  • Provide a description explaining the problem, implementation and compatibility.
  • Add necessary unit tests with over 80% coverage. Regression tests are included; the coverage threshold has not been confirmed for the final
    patch.
  • File an Apache Individual Contributor License Agreement if required.

@wenxuwan wenxuwan changed the title Fix shared trace dispatcher lifecycle and discovery fix: Fix shared trace dispatcher lifecycle and discovery Sep 28, 2026
Balance client ownership from construction through shutdown, preserve trace batching and NameServer failover, and safely replace trace interceptors.

Separate address discovery from route refresh, reuse concurrent route lookups, publish broker addresses before queues, and avoid mutating resolver snapshots. Add lifecycle, migration, concurrency, and timeout regression coverage.
@ShannonDing
ShannonDing merged commit fa64715 into apache:master Oct 8, 2026
7 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.

2 participants