elastic / elastic/apm-server

TBS: persist the sampling-decision publisher UUID across restarts to avoid redundant resync

Open
#20,945 4 comments 2 reactions 0 assignees View on GitHub
enhancement
Dominant language
Go
Stars
1.3k
Forks
543
Avg merge
1d 18h
Merged PRs (30d)
109

Description

When tail-based sampling (TBS) is enabled, APM Server publishes finalized sampling decisions to the `traces-apm.sampled-*` data stream and subscribes to the same stream to consume decisions from peer APM Servers. To skip its own decisions during subscription, the query uses a `must_not` filter on `agent.ephemeral_id`, which is set to `samplerUUID`.

`samplerUUID` is a package-level `var` generated fresh on every process start:

https://github.com/elastic/apm-server/blob/e899b884877ea87605b24663efff8ffa39acddb1/x-pack/apm-server/main.go#L45-L47

Because the UUID rotates on every restart, the self-filter for a restarted process no longer matches decisions the same server published in previous incarnations. Those decisions are then re-fetched from Elasticsearch even though they are already present in the local decision DB.

Nothing is functionally incorrect here (decisions are idempotent on the consumer side, keyed by trace ID). The cost is wasted bandwidth, CPU, and disk writes on every restart.

/cc @carsonip which helped uncover this

## Behaviour

**Single-instance deployments.** Every document in the data stream is published by this instance, so every subscribe poll matches only self-docs and returns zero hits. `maxObservedSeqno` stays at -1 in `searchIndexTraceIDs`, the `if maxSeqno > observedSeqno` gate in `searchTraceIDs` never fires, and `subscriber_position.json` is never advanced past its initial state. On restart the resumed subscriber issues `_seq_no > -1 AND agent.ephemeral_id != newUUID`, which matches every document still retained in the data stream. The paginated loop drains it at 1000 docs per page.

With persistent storage the decision DB and `subscriber_position.json` both survive. This does not cause issues as the re-ingested decisions are idempotent overwrites. The visible effect is redundant network and disk activity.
The persistent-storage improvement from #4437 is effectively shadowed by the UUID rotation.

**Multi-instance deployments.** The position advances past peer-published decisions during normal operation, so the re-fetched window on restart is bounded to the tail of recently-written self-docs (between the last peer-observed `_seq_no` and the current global checkpoint). Still redundant, but with smaller overall impact.

## Impact

Scales with throughput and ILM retention on `traces-apm.sampled-*` indices.

On restart, it leads to elevated CPU, disk, and network activity until the stream is drained.

No impact on ephemeral storage, as there the re-fetch is necessary anyway. Decisive impact on persistent storage where the re-fetch could be mostly avoided.

## Open question

Was the per-process scoping of `samplerUUID` intentional? The comment at `main.go:45-47` suggests yes but does not clarify for what purpose. Before opening a PR we need to confirm whether there is a correctness argument behind per-process identity.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.