Skip to content

Distinguish metastore retry attempts from final errors - #6698

Merged
Mallets merged 13 commits into
mainfrom
mallets/metastore-metrics
Aug 18, 2026
Merged

Distinguish metastore retry attempts from final errors#6698
Mallets merged 13 commits into
mainfrom
mallets/metastore-metrics

Conversation

@Mallets

@Mallets Mallets commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Summary

Tower gRPC request metrics are emitted when the request future is dropped. At that point, a failed attempt is recorded as status="error", even if the retry policy will retry it. Consequently, transient failures appear as hard errors in metric queries.

This PR introduces GrpcRetryPolicy, a gRPC-aware wrapper around the generic retry policy. It observes retry decisions and identifies failed attempts that will be retried. These attempts are recorded with status="retry", while preserving the original gRPC status code in the code label. Final failures continue to use status="error".

The metastore client is updated to use this policy and to place the gRPC metrics layer outside the retry loop. As a result:

  • retryable failures that lead to another attempt are recorded as status="retry"
  • status="error" is reserved for the final client-visible failure, including exhausted retries
  • status="success" represents the final successful outcome, including success after retries
  • in-flight and duration metrics describe the complete logical RPC rather than each individual attempt

Scope

Metastore is the only service behaviorally impacted by this change because it is the only service that combines GrpcMetricsLayer with Quickwit's RetryPolicy. Control plane, indexing, ingest, and cluster also use GrpcMetricsLayer, but they do not use it with this retry policy, so their metric semantics remain unchanged.

The shared gRPC metastore metrics and retry status semantics are also documented.

Contributions

Integrates changes proposed in https://github.com/quickwit-oss/quickwit/compare/guilload/retry-metrics

Validation

  • cargo +nightly fmt --all
  • cargo test -p quickwit-common test_grpc_metrics
  • cargo clippy -p quickwit-common --all-features --tests
  • cargo clippy -p quickwit-serve --all-features --tests

Record scheduled retries separately so final error metrics reflect only client-visible failures.
Keep this change focused on retry metric behavior without expanding the metrics reference.
@Mallets
Mallets force-pushed the mallets/metastore-metrics branch from 9fad660 to 9d0eb17 Compare August 17, 2026 16:48
@Mallets
Mallets marked this pull request as ready for review August 17, 2026 16:50
@Mallets
Mallets requested a review from a team as a code owner August 17, 2026 16:50
Document why request metrics must wrap retries to avoid counting retry attempts as final errors.
Document Tower's outermost-first stacking and why metrics must wrap retries.
Decouple gRPC retry metrics from Quickwit's concrete retry policy while preserving retry outcome coverage.
Align the metrics wrapper and tests with the proposed generic policy implementation.
@Mallets
Mallets merged commit 7222efa into main Aug 18, 2026
9 checks passed
@Mallets
Mallets deleted the mallets/metastore-metrics branch August 18, 2026 13:36
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