[pull] master from prestodb:master - #997
Merged
Merged
Conversation
…ndling (#28429) ## Description Hardens the native sidecar HTTP client against transient transport failures by introducing a shared `SidecarRetryDriver` with configurable exponential-backoff retry. ## Motivation and Context Before this change, any transient transport error (TCP reset, SSL handshake timeout, brief native worker restart) during a sidecar call immediately failed the planning phase with a generic internal error, causing the query to fail rather than retry. The four call sites each had ad-hoc, inconsistent error handling — some silently dropped errors, others propagated them directly. ## Impact Transient sidecar failures are now retried for up to 5 seconds (configurable) before propagating as a query failure. ## Test Plan CI, unit tests ## Contributor checklist - [x] Please make sure your submission complies with our [contributing guide](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md), in particular [code style](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#code-style) and [commit standards](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#commit-standards). - [x] PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced. - [x] Documented new properties (with its default value), SQL syntax, functions, or other functionality. - [x] If release notes are required, they follow the [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines). - [x] Adequate tests were added if applicable. - [x] CI passed. - [x] If adding new dependencies, verified they have an [OpenSSF Scorecard](https://securityscorecards.dev/#the-checks) score of 5.0 or higher (or obtained explicit TSC approval for lower scores). ## Release Notes Please follow [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines) and fill in the release notes below. ``` == RELEASE NOTES == Prestissimo (Native Execution) Changes * Add configurable exponential-backoff retry for all native sidecar HTTP calls so that transient sidecar failures are retried for up to 5 seconds (configurable) before propagating as a query failure. ``` ## Summary by Sourcery Harden native sidecar communication against transient failures and ensure safe native worker shutdown. New Features: - Add configurable exponential-backoff retries for transient native sidecar HTTP failures, with a 30-second default limit across expression optimization, plan validation, function definitions, and session properties. Bug Fixes: - Fix native worker shutdown ordering to drain HTTP server CPU tasks before the driver executor is destroyed, preventing use-after-free during expression requests. - Improve sidecar failure classification and propagation so definitive server, protocol, cancellation, and deterministic errors are not retried and exhausted failures retain useful diagnostic context. Enhancements: - Move the shared Backoff utility into presto-common for reuse by native sidecar and existing remote task clients. Build: - Make the Airlift units dependency available from presto-common and add the validation API dependency to the native sidecar plugin. Documentation: - Document the native sidecar retry configuration and its default behavior. Tests: - Add unit coverage for sidecar retry behavior, exception classification, bounded failure history, retry configuration wiring, and shutdown ordering.
## Description Adds a `legacy_timestamp_with_timezone` session property that the coordinator forwards to the native worker as the Velox query config of the same name. It defaults to `true`, which is also the Velox default, so behavior is unchanged until it is set to `false`. This PR only plumbs the property through. The rendering change it gates lands in follow-ups. ## Motivation and Context `TIMESTAMP WITH TIME ZONE` values render in each value's embedded time zone, so two values that compare equal as instants can render differently. Setting this property to `false` switches the field, render, and date-arithmetic functions to render the UTC instant in the session time zone, so values that compare equal render identically. The property name is bare rather than `execution.`-prefixed because Presto reads a dotted name as a catalog-scoped property and drops it when that catalog does not exist, so only the bare form reaches the worker. The cluster-wide default comes from the `legacy-timestamp-with-timezone` config. ## Impact No impact. The property defaults to `true`, preserving current behavior, and nothing reads it until the follow-up commits land. ## Test Plan Added `legacy_timestamp_with_timezone` coverage to `TestFunctionsConfig`, `SessionPropertiesTest`, and `PrestoToVeloxQueryConfigTest`, asserting the default, the config-to-session mapping, and that setting it to `false` reaches the Velox `QueryConfig`. ## Release Notes Please follow [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines) and fill in the release notes below. ``` == NO RELEASE NOTE == ```
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )