feat: [Presto-Reliability] Add observability for query state transitions in Presto (#26418) by abhinavmuk04 · Pull Request #26418 · prestodb/presto

abhinavmuk04 · 2025-10-23T19:10:12Z

Summary:

Adds comprehensive monitoring for query state transitions to detect and diagnose
queries spending abnormally long time in DISPATCHING and FINISHING states. This
addresses issues observed on DKL cluster where queries were getting stuck during
dispatch and commit phases (ref: S561047).

Differential Revision: D85353849

Release Notes

Please follow release notes guidelines and fill in the release notes below.

== NO RELEASE NOTE ==

sourcery-ai · 2025-10-23T19:11:41Z

Reviewer's Guide

This PR introduces a QueryStateTransitionMonitor to record query state transition durations and detect anomalies, integrates monitoring hooks into QueryStateMachine, binds and exports the monitor metrics in the server module, and adds comprehensive unit tests for all monitoring scenarios.

Sequence diagram for query state transition monitoring integration

sequenceDiagram
    participant QSM as QueryStateMachine
    participant Monitor as QueryStateTransitionMonitor
    participant Tracker as QueryTransitionTracker
    QSM->>Monitor: setStateTransitionMonitor(monitor)
    Monitor->>QSM: registerQuery(queryId)
    QSM->>QSM: addStateChangeListener(trackStateTransition)
    QSM->>QSM: trackStateTransition(newState)
    QSM->>Monitor: recordStateTransition(queryId, prevState, newState, durationMillis)
    Monitor->>Tracker: recordTransition(prevState, newState, durationMillis)
    alt Anomaly detected
        Monitor->>Monitor: checkAndLogAnomaly(...)
    end
    alt Query done
        Monitor->>Monitor: activeQueries.remove(queryId)
    end

Class diagram for QueryStateTransitionMonitor and integration with QueryStateMachine

classDiagram
    class QueryStateTransitionMonitor {
        +registerQuery(queryId)
        +recordStateTransition(queryId, fromState, toState, durationMillis)
        +getTotalQueriesTracked()
        +getActiveQueriesCount()
        +getDispatchingTimeStats()
        +getFinishingTimeStats()
        +getPlanningTimeStats()
        +getRunningTimeStats()
        +getAnomalousDispatchingCount()
        +getAnomalousFinishingCount()
        +getAnomalousPlanningCount()
        +getAnomalousRunningCount()
        +getStatsSummary()
        -activeQueries: Map<QueryId, QueryTransitionTracker>
        -dispatchingTimeStats: TimeStat
        -finishingTimeStats: TimeStat
        -planningTimeStats: TimeStat
        -runningTimeStats: TimeStat
        -anomalousDispatchingCount: CounterStat
        -anomalousFinishingCount: CounterStat
        -anomalousPlanningCount: CounterStat
        -anomalousRunningCount: CounterStat
        -totalQueriesTracked: AtomicLong
    }
    class QueryTransitionTracker {
        -queryId: QueryId
        -lastState: QueryState
        -lastTransitionTimeMillis: long
        +recordTransition(fromState, toState, durationMillis)
        +getLastState()
        +getLastTransitionTimeMillis()
    }
    class QueryStateMachine {
        -stateTransitionMonitor: AtomicReference<QueryStateTransitionMonitor>
        -previousState: AtomicReference<QueryState>
        -previousStateTimestamp: AtomicLong
        +setStateTransitionMonitor(monitor)
        +trackStateTransition(newState)
    }
    QueryStateTransitionMonitor "1" *-- "many" QueryTransitionTracker
    QueryStateMachine "1" o-- "1" QueryStateTransitionMonitor

File-Level Changes

Change	Details	Files
Integrate state transition monitoring into QueryStateMachine	Added AtomicReferences for monitor, previous state and timestamp Implemented setStateTransitionMonitor to register monitor and attach listener Added trackStateTransition to capture durations and update monitor	`presto-main-base/src/main/java/com/facebook/presto/execution/QueryStateMachine.java`
Implement QueryStateTransitionMonitor for timing and anomaly detection	Created monitor class with TimeStat and CounterStat for each state Implemented registerQuery, recordStateTransition logic, and anomaly logging Exposed metrics and stats summary via JMX getters	`presto-main-base/src/main/java/com/facebook/presto/execution/QueryStateTransitionMonitor.java`
Bind and export monitor in server DI module	Registered QueryStateTransitionMonitor as a singleton Exported monitor via JMX/ODS exporter	`presto-main/src/main/java/com/facebook/presto/server/ServerMainModule.java`
Add comprehensive unit tests for state transition monitoring	Tests for query registration, lifecycle transitions, stats collection and anomaly detection Coverage for terminal state cleanup, JMX metrics, concurrent scenarios	`presto-main-base/src/test/java/com/facebook/presto/execution/TestQueryStateTransitionMonitor.java`

Tips and commands

Interacting with Sourcery

Trigger a new review: Comment @sourcery-ai review on the pull request.
Continue discussions: Reply directly to Sourcery's review comments.
Generate a GitHub issue from a review comment: Ask Sourcery to create an
issue from a review comment by replying to it. You can also reply to a
review comment with @sourcery-ai issue to create an issue from it.
Generate a pull request title: Write @sourcery-ai anywhere in the pull
request title to generate a title at any time. You can also comment
@sourcery-ai title on the pull request to (re-)generate the title at any time.
Generate a pull request summary: Write @sourcery-ai summary anywhere in
the pull request body to generate a PR summary at any time exactly where you
want it. You can also comment @sourcery-ai summary on the pull request to
(re-)generate the summary at any time.
Generate reviewer's guide: Comment @sourcery-ai guide on the pull
request to (re-)generate the reviewer's guide at any time.
Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
pull request to resolve all Sourcery comments. Useful if you've already
addressed all the comments and don't want to see them anymore.
Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
request to dismiss all existing Sourcery reviews. Especially useful if you
want to start fresh with a new review - don't forget to comment
@sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

Enable or disable review features such as the Sourcery-generated pull request
summary, the reviewer's guide, and others.
Change the review language.
Add, remove or edit custom review instructions.
Adjust other review settings.

Getting Help

Contact our support team for questions or feedback.
Visit our documentation for detailed guides and information.
Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

sourcery-ai

Hey there - I've reviewed your changes and they look great!

Prompt for AI Agents

Please address the comments from this code review:

## Individual Comments

### Comment 1
<location> `presto-main-base/src/main/java/com/facebook/presto/execution/QueryStateMachine.java:1013-1023` </location>
<code_context>
+
+        QueryState prevState = previousState.get();
+        long prevTimestamp = previousStateTimestamp.get();
+        long currentTimestamp = System.currentTimeMillis();
+        long durationMillis = currentTimestamp - prevTimestamp;
+
+        if (prevState != null) {
</code_context>

<issue_to_address>
**suggestion (bug_risk):** Using System.currentTimeMillis() for duration may be affected by system clock changes.

System.nanoTime() is preferred for measuring elapsed time, as it is not affected by system clock changes.

```suggestion
        QueryState prevState = previousState.get();
        long prevTimestampNanos = previousStateTimestamp.get();
        long currentTimestampNanos = System.nanoTime();
        long durationMillis = (currentTimestampNanos - prevTimestampNanos) / 1_000_000;

        if (prevState != null) {
            monitor.recordStateTransition(queryId, prevState, newState, durationMillis);
        }

        previousState.set(newState);
        previousStateTimestamp.set(currentTimestampNanos);
```
</issue_to_address>

### Comment 2
<location> `presto-main-base/src/test/java/com/facebook/presto/execution/TestQueryStateTransitionMonitor.java:50-59` </location>
<code_context>
+        assertNotNull(monitor.getStatsSummary());
+    }
+
+    @Test
+    public void testConcurrentQueryRegistration()
+    {
+        int numQueries = 100;
+        for (int i = 0; i < numQueries; i++) {
+            QueryId queryId = new QueryId("concurrent_query_" + i);
+            monitor.registerQuery(queryId);
+            monitor.recordStateTransition(queryId, DISPATCHING, PLANNING, 1000L + i);
+        }
+
+        assertEquals(monitor.getTotalQueriesTracked(), numQueries);
+    }
+
+    @Test
+    public void testStateTransitionWithZeroDuration()
+    {
+        monitor.registerQuery(testQueryId);
</code_context>

<issue_to_address>
**suggestion (testing):** Edge case for negative duration is not tested.

Please add a test for negative duration values in state transitions to verify the monitor's behavior in these cases.

Suggested implementation:

```java
    @Test
    public void testStateTransitionWithNegativeDuration()
    {
        monitor.registerQuery(testQueryId);
        // Attempt to record a state transition with a negative duration
        monitor.recordStateTransition(testQueryId, DISPATCHING, PLANNING, -500L);
        // Assert that the query is still tracked and no exception is thrown
        assertEquals(monitor.getTotalQueriesTracked(), 1);
        assertEquals(monitor.getActiveQueriesCount(), 1);
        // Optionally, check stats summary if available
        assertNotNull(monitor.getStatsSummary());
    }

    @Test
    public void testStateTransitionWithZeroDuration()
    {
        monitor.registerQuery(testQueryId);

```

If the monitor is expected to handle negative durations in a specific way (e.g., clamp to zero, ignore, or log a warning), you may want to add more specific assertions based on the implementation of `recordStateTransition` and `getStatsSummary()`. Adjust the assertions as needed to match the expected behavior.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

github-actions · 2025-10-23T19:13:34Z

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Snapshot Warnings

⚠️: No snapshots were found for the head SHA 0fa560b.

Ensure that dependencies are being submitted on PR branches. Re-running this action after a short time may resolve the issue. See the documentation for more information and troubleshooting advice.

Scanned Files

None

… Presto (#26418) Summary: Adds comprehensive monitoring for query state transitions to detect and diagnose queries spending abnormally long time in DISPATCHING and FINISHING states. This addresses issues observed on DKL cluster where queries were getting stuck during dispatch and commit phases (ref: S561047). Differential Revision: D85353849

… Presto (#26418) Summary: Pull Request resolved: #26418 Adds comprehensive monitoring for query state transitions to detect and diagnose queries spending abnormally long time in DISPATCHING and FINISHING states. This addresses issues observed on DKL cluster where queries were getting stuck during dispatch and commit phases (ref: S561047). Differential Revision: D85353849

abhinavmuk04 requested a review from a team as a code owner October 23, 2025 19:10

prestodb-ci added the from:Meta PR from Meta label Oct 23, 2025

sourcery-ai Bot reviewed Oct 23, 2025

View reviewed changes

Comment thread presto-main-base/src/main/java/com/facebook/presto/execution/QueryStateMachine.java

abhinavmuk04 changed the title ~~[Presto-Reliability] Add observability for query state transitions in Presto~~ feat: Add observability for query state transitions in Presto Oct 23, 2025

facebook-github-bot force-pushed the export-D85353849 branch from 270569f to 6df29ed Compare November 4, 2025 21:00

facebook-github-bot added fb-exported meta-exported labels Nov 4, 2025

github-code-quality Bot found potential problems Nov 4, 2025

View reviewed changes

facebook-github-bot force-pushed the export-D85353849 branch from 6df29ed to b0f5dc2 Compare November 5, 2025 01:16

facebook-github-bot force-pushed the export-D85353849 branch from b0f5dc2 to 0dd7d61 Compare November 5, 2025 02:44

facebook-github-bot force-pushed the export-D85353849 branch from 0dd7d61 to 73ecaf2 Compare November 5, 2025 19:30

facebook-github-bot force-pushed the export-D85353849 branch from 73ecaf2 to d2e053f Compare November 5, 2025 22:20

abhinavmuk04 requested review from feilong-liu and prashantgolash November 7, 2025 20:13

facebook-github-bot force-pushed the export-D85353849 branch from d2e053f to a1a6650 Compare November 11, 2025 19:57

facebook-github-bot force-pushed the export-D85353849 branch from a1a6650 to 1d2cc6e Compare December 4, 2025 19:44

facebook-github-bot force-pushed the export-D85353849 branch from 1d2cc6e to f7d9b13 Compare December 4, 2025 19:45

facebook-github-bot force-pushed the export-D85353849 branch from f7d9b13 to d59ac3f Compare December 10, 2025 00:41

github-code-quality Bot found potential problems Dec 10, 2025

View reviewed changes

Comment thread presto-main-base/src/main/java/com/facebook/presto/execution/QueryStateTransitionMonitor.java Fixed

Comment thread presto-main-base/src/main/java/com/facebook/presto/execution/QueryStateTransitionMonitor.java Fixed

facebook-github-bot force-pushed the export-D85353849 branch from 6a9fb63 to 59c49cf Compare February 3, 2026 22:19

facebook-github-bot force-pushed the export-D85353849 branch from 59c49cf to 723cc3d Compare February 3, 2026 22:21

github-code-quality Bot found potential problems Feb 3, 2026

View reviewed changes

facebook-github-bot force-pushed the export-D85353849 branch from 723cc3d to 137de1e Compare February 10, 2026 17:36

facebook-github-bot force-pushed the export-D85353849 branch from 137de1e to 66bea34 Compare February 10, 2026 17:43

facebook-github-bot force-pushed the export-D85353849 branch from 66bea34 to 32e786f Compare February 10, 2026 17:43

github-code-quality Bot found potential problems Feb 10, 2026

View reviewed changes

facebook-github-bot force-pushed the export-D85353849 branch from 32e786f to 0858971 Compare February 10, 2026 17:59

github-code-quality Bot found potential problems Feb 10, 2026

View reviewed changes

Comment thread ...o-main-base/src/test/java/com/facebook/presto/execution/TestQueryStateTransitionMonitor.java Dismissed

facebook-github-bot force-pushed the export-D85353849 branch from 0858971 to a0b8205 Compare February 11, 2026 19:17

prashantgolash approved these changes Mar 13, 2026

View reviewed changes

feilong-liu approved these changes Mar 14, 2026

View reviewed changes

meta-codesync Bot changed the title ~~feat: Add observability for query state transitions in Presto~~ [Presto-Reliability] Add observability for query state transitions in Presto (#26418) Apr 5, 2026

meta-codesync Bot force-pushed the export-D85353849 branch from a0b8205 to 567a6ba Compare April 5, 2026 17:07

meta-codesync Bot force-pushed the export-D85353849 branch from 567a6ba to e70a2b0 Compare April 5, 2026 17:07

meta-codesync Bot force-pushed the export-D85353849 branch from e70a2b0 to 0dbf103 Compare April 5, 2026 17:08

abhinavmuk04 changed the title ~~[Presto-Reliability] Add observability for query state transitions in Presto (#26418)~~ feat: Add observability for query state transitions in Presto (#26418) Apr 5, 2026

unidevel mentioned this pull request May 28, 2026

docs: Add release notes for 0.298 unix280/presto#69

Open

prestodb-ci mentioned this pull request May 29, 2026

docs: Add release notes for 0.298 #27897

Merged

31 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: [Presto-Reliability] Add observability for query state transitions in Presto (#26418)#26418

feat: [Presto-Reliability] Add observability for query state transitions in Presto (#26418)#26418
abhinavmuk04 merged 1 commit into
masterfrom
export-D85353849

abhinavmuk04 commented Oct 23, 2025 •

edited

Loading

Uh oh!

sourcery-ai Bot commented Oct 23, 2025 •

edited

Loading

Interacting with Sourcery

Customizing Your Experience

Getting Help

Uh oh!

sourcery-ai Bot left a comment

Uh oh!

Uh oh!

github-actions Bot commented Oct 23, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

abhinavmuk04 commented Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Release Notes

Uh oh!

sourcery-ai Bot commented Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviewer's Guide

Sequence diagram for query state transition monitoring integration

Class diagram for QueryStateTransitionMonitor and integration with QueryStateMachine

File-Level Changes

Interacting with Sourcery

Customizing Your Experience

Getting Help

Uh oh!

sourcery-ai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions Bot commented Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Dependency Review

Snapshot Warnings

Scanned Files

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

abhinavmuk04 commented Oct 23, 2025 •

edited

Loading

sourcery-ai Bot commented Oct 23, 2025 •

edited

Loading

github-actions Bot commented Oct 23, 2025 •

edited

Loading