traced_probes: kFlushTimeoutMs (1s) is too restrictive and redundant
Nobody has claimed this yet.
- Dominant language
- C++
- Stars
- 6.5k
- Forks
- 872
- Avg merge
- 2d 1h
- Merged PRs (30d)
- 218
Description
ProbesProducer::kFlushTimeoutMs is set to 1000ms and controls the max time the producer waits for its data sources to ack a flush.
There is already a service-side flush timeout (kDefaultFlushTimeoutMs) which is also configurable by the user using flush_timeout_ms. So:
- we are disregarding the user's request
- we are masking data loss if any as we erase any pending flushes and invoke NotifyFlushComplete as a normal ack.
We should either remove it altogether and let the service-side timeout handle it. Or raise kFlushTimeoutMs value to be a safety net.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by locating ProbesProducer::kFlushTimeoutMs and tracing how it handles pending flushes and NotifyFlushComplete. Compare that path with the service-side kDefaultFlushTimeoutMs and the user-configurable flush_timeout_ms behavior. Done means the producer no longer overrides the requested timeout or provides a clearly justified safety-net value without masking data loss.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- observability
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100