Skip to content

[native][Coordinator throttling] Endpoint on worker reporting node load metrics#25686

Open
prashantgolash wants to merge 1 commit into
prestodb:masterfrom
prashantgolash:export-D76357677
Open

[native][Coordinator throttling] Endpoint on worker reporting node load metrics#25686
prashantgolash wants to merge 1 commit into
prestodb:masterfrom
prashantgolash:export-D76357677

Conversation

@prashantgolash

@prashantgolash prashantgolash commented Aug 5, 2025

Copy link
Copy Markdown
Contributor

Summary:
Context:
We want to consider worker load metrics to make scheduling decision from coordinator. Currently in single coordinator setup, it invokes /v1/info/state to get worker's state.

Approach:
Added a new end point to get nodeStats. The idea is that we would replace
"/v1/info/state" -> "/v1/info/nodestate" which will include nodeState + . This will help coordinator making scheduling decisions

Differential Revision: D76357677

Testing
curl <worker_host>/v1/info/stats
{
"loadMetrics": {
"cpuOverload": false,
"cpuUsedPercent": 0,
"memoryOverload": true,
"memoryUsedInBytes": 0,
"numQueuedDrivers": 0
},
"nodeState": "ACTIVE"
}

@prashantgolash prashantgolash requested review from a team as code owners August 5, 2025 06:40
@prestodb-ci prestodb-ci added the from:Meta PR from Meta label Aug 5, 2025
@facebook-github-bot

Copy link
Copy Markdown
Collaborator

This pull request was exported from Phabricator. Differential Revision: D76357677

prashantgolash added a commit to prashantgolash/presto that referenced this pull request Aug 5, 2025
Summary:

**Context:**
We want to consider worker load metrics to make scheduling decision from coordinator. Currently in single coordinator setup, it invokes /v1/info/state to get worker's state. 

**Approach:**
Added a new end point to get nodeStats. The idea is that we would replace 
"/v1/info/state" -> "/v1/info/nodestate" which will include nodeState + <worker load metrics>. This will help coordinator making scheduling decisions

Differential Revision: D76357677
prashantgolash added a commit to prashantgolash/presto that referenced this pull request Aug 5, 2025
Summary:

**Context:**
We want to consider worker load metrics to make scheduling decision from coordinator. Currently in single coordinator setup, it invokes /v1/info/state to get worker's state. 

**Approach:**
Added a new end point to get nodeStats. The idea is that we would replace 
"/v1/info/state" -> "/v1/info/nodestate" which will include nodeState + <worker load metrics>. This will help coordinator making scheduling decisions

Differential Revision: D76357677
prashantgolash added a commit to prashantgolash/presto that referenced this pull request Aug 5, 2025
Summary:

**Context:**
We want to consider worker load metrics to make scheduling decision from coordinator. Currently in single coordinator setup, it invokes /v1/info/state to get worker's state. 

**Approach:**
Added a new end point to get nodeStats. The idea is that we would replace 
"/v1/info/state" -> "/v1/info/nodestate" which will include nodeState + <worker load metrics>. This will help coordinator making scheduling decisions

Differential Revision: D76357677
prashantgolash added a commit to prashantgolash/presto that referenced this pull request Aug 5, 2025
Summary:

**Context:**
We want to consider worker load metrics to make scheduling decision from coordinator. Currently in single coordinator setup, it invokes /v1/info/state to get worker's state. 

**Approach:**
Added a new end point to get nodeStats. The idea is that we would replace 
"/v1/info/state" -> "/v1/info/nodestate" which will include nodeState + <worker load metrics>. This will help coordinator making scheduling decisions

Differential Revision: D76357677
@facebook-github-bot

Copy link
Copy Markdown
Collaborator

This pull request was exported from Phabricator. Differential Revision: D76357677

prashantgolash added a commit to prashantgolash/presto that referenced this pull request Aug 5, 2025
…cs (prestodb#25686)

Summary:

**Context:**
We want to consider worker load metrics to make scheduling decision from coordinator. Currently in single coordinator setup, it invokes /v1/info/state to get worker's state. 

**Approach:**
Added a new end point to get nodeStats. The idea is that we would replace 
"/v1/info/state" -> "/v1/info/nodestate" which will include nodeState + <worker load metrics>. This will help coordinator making scheduling decisions

Differential Revision: D76357677
@facebook-github-bot

Copy link
Copy Markdown
Collaborator

This pull request was exported from Phabricator. Differential Revision: D76357677

@prashantgolash prashantgolash changed the title Worker reporting nodestate [Coordinator throttling] Endpoint on worker reporting node load metrics Aug 5, 2025
@facebook-github-bot

Copy link
Copy Markdown
Collaborator

This pull request was exported from Phabricator. Differential Revision: D76357677

@facebook-github-bot

Copy link
Copy Markdown
Collaborator

This pull request was exported from Phabricator. Differential Revision: D76357677

@prashantgolash prashantgolash requested a review from spershin August 5, 2025 07:01
prashantgolash added a commit to prashantgolash/presto that referenced this pull request Aug 5, 2025
…cs (prestodb#25686)

Summary:

**Context:**
We want to consider worker load metrics to make scheduling decision from coordinator. Currently in single coordinator setup, it invokes /v1/info/state to get worker's state. 

**Approach:**
Added a new end point to get nodeStats. The idea is that we would replace 
"/v1/info/state" -> "/v1/info/nodestate" which will include nodeState + <worker load metrics>. This will help coordinator making scheduling decisions

Differential Revision: D76357677
prashantgolash added a commit to prashantgolash/presto that referenced this pull request Aug 5, 2025
…cs (prestodb#25686)

Summary:

**Context:**
We want to consider worker load metrics to make scheduling decision from coordinator. Currently in single coordinator setup, it invokes /v1/info/state to get worker's state. 

**Approach:**
Added a new end point to get nodeStats. The idea is that we would replace 
"/v1/info/state" -> "/v1/info/nodestate" which will include nodeState + <worker load metrics>. This will help coordinator making scheduling decisions

Differential Revision: D76357677
@facebook-github-bot

Copy link
Copy Markdown
Collaborator

This pull request was exported from Phabricator. Differential Revision: D76357677

prashantgolash added a commit to prashantgolash/presto that referenced this pull request Aug 5, 2025
…cs (prestodb#25686)

Summary:

**Context:**
We want to consider worker load metrics to make scheduling decision from coordinator. Currently in single coordinator setup, it invokes /v1/info/state to get worker's state. 

**Approach:**
Added a new end point to get nodeStats. The idea is that we would replace 
"/v1/info/state" -> "/v1/info/nodestate" which will include nodeState + <worker load metrics>. This will help coordinator making scheduling decisions

Differential Revision: D76357677
prashantgolash added a commit to prashantgolash/presto that referenced this pull request Aug 5, 2025
…cs (prestodb#25686)

Summary:

**Context:**
We want to consider worker load metrics to make scheduling decision from coordinator. Currently in single coordinator setup, it invokes /v1/info/state to get worker's state. 

**Approach:**
Added a new end point to get nodeStats. The idea is that we would replace 
"/v1/info/state" -> "/v1/info/nodestate" which will include nodeState + <worker load metrics>. This will help coordinator making scheduling decisions

Differential Revision: D76357677
@facebook-github-bot

Copy link
Copy Markdown
Collaborator

This pull request was exported from Phabricator. Differential Revision: D76357677

prashantgolash added a commit to prashantgolash/presto that referenced this pull request Aug 5, 2025
…cs (prestodb#25686)

Summary:
Pull Request resolved: prestodb#25686

**Context:**
We want to consider worker load metrics to make scheduling decision from coordinator. Currently in single coordinator setup, it invokes /v1/info/state to get worker's state.

**Approach:**
Added a new end point to get nodeStats. The idea is that we would replace
"/v1/info/state" -> "/v1/info/nodestate" which will include nodeState + <worker load metrics>. This will help coordinator making scheduling decisions

Differential Revision: D76357677
prashantgolash added a commit to prashantgolash/presto that referenced this pull request Aug 11, 2025
…cs (prestodb#25686)

Summary:

**Context:**
We want to consider worker load metrics to make scheduling decision from coordinator. Currently in single coordinator setup, it invokes /v1/info/state to get worker's state. 

**Approach:**
Added a new end point to get nodeStats. The idea is that we would replace 
"/v1/info/state" -> "/v1/info/nodestate" which will include nodeState + <worker load metrics>. This will help coordinator making scheduling decisions

Differential Revision: D76357677
@facebook-github-bot

Copy link
Copy Markdown
Collaborator

This pull request was exported from Phabricator. Differential Revision: D76357677

prashantgolash added a commit to prashantgolash/presto that referenced this pull request Aug 11, 2025
…cs (prestodb#25686)

Summary:
Pull Request resolved: prestodb#25686

**Context:**
We want to consider worker load metrics to make scheduling decision from coordinator. Currently in single coordinator setup, it invokes /v1/info/state to get worker's state.

**Approach:**
Added a new end point to get nodeStats. The idea is that we would replace
"/v1/info/state" -> "/v1/info/nodestate" which will include nodeState + <worker load metrics>. This will help coordinator making scheduling decisions

Differential Revision: D76357677
prashantgolash added a commit to prashantgolash/presto that referenced this pull request Aug 11, 2025
…cs (prestodb#25686)

Summary:

**Context:**
We want to consider worker load metrics to make scheduling decision from coordinator. Currently in single coordinator setup, it invokes /v1/info/state to get worker's state. 

**Approach:**
Added a new end point to get nodeStats. The idea is that we would replace 
"/v1/info/state" -> "/v1/info/nodestate" which will include nodeState + <worker load metrics>. This will help coordinator making scheduling decisions

Differential Revision: D76357677
prashantgolash added a commit to prashantgolash/presto that referenced this pull request Aug 11, 2025
…cs (prestodb#25686)

Summary:

**Context:**
We want to consider worker load metrics to make scheduling decision from coordinator. Currently in single coordinator setup, it invokes /v1/info/state to get worker's state. 

**Approach:**
Added a new end point to get nodeStats. The idea is that we would replace 
"/v1/info/state" -> "/v1/info/nodestate" which will include nodeState + <worker load metrics>. This will help coordinator making scheduling decisions

Differential Revision: D76357677
prashantgolash added a commit to prashantgolash/presto that referenced this pull request Aug 11, 2025
…cs (prestodb#25686)

Summary:

**Context:**
We want to consider worker load metrics to make scheduling decision from coordinator. Currently in single coordinator setup, it invokes /v1/info/state to get worker's state. 

**Approach:**
Added a new end point to get nodeStats. The idea is that we would replace 
"/v1/info/state" -> "/v1/info/nodestate" which will include nodeState + <worker load metrics>. This will help coordinator making scheduling decisions

Differential Revision: D76357677
prashantgolash added a commit to prashantgolash/presto that referenced this pull request Aug 11, 2025
…cs (prestodb#25686)

Summary:

**Context:**
We want to consider worker load metrics to make scheduling decision from coordinator. Currently in single coordinator setup, it invokes /v1/info/state to get worker's state. 

**Approach:**
Added a new end point to get nodeStats. The idea is that we would replace 
"/v1/info/state" -> "/v1/info/nodestate" which will include nodeState + <worker load metrics>. This will help coordinator making scheduling decisions

Differential Revision: D76357677
prashantgolash added a commit to prashantgolash/presto that referenced this pull request Aug 11, 2025
…cs (prestodb#25686)

Summary:

**Context:**
We want to consider worker load metrics to make scheduling decision from coordinator. Currently in single coordinator setup, it invokes /v1/info/state to get worker's state. 

**Approach:**
Added a new end point to get nodeStats. The idea is that we would replace 
"/v1/info/state" -> "/v1/info/nodestate" which will include nodeState + <worker load metrics>. This will help coordinator making scheduling decisions

Differential Revision: D76357677
prashantgolash added a commit to prashantgolash/presto that referenced this pull request Aug 11, 2025
…cs (prestodb#25686)

Summary:

**Context:**
We want to consider worker load metrics to make scheduling decision from coordinator. Currently in single coordinator setup, it invokes /v1/info/state to get worker's state. 

**Approach:**
Added a new end point to get nodeStats. The idea is that we would replace 
"/v1/info/state" -> "/v1/info/nodestate" which will include nodeState + <worker load metrics>. This will help coordinator making scheduling decisions

Differential Revision: D76357677
@facebook-github-bot

Copy link
Copy Markdown
Collaborator

This pull request was exported from Phabricator. Differential Revision: D76357677

prashantgolash added a commit to prashantgolash/presto that referenced this pull request Aug 11, 2025
…cs (prestodb#25686)

Summary:
Pull Request resolved: prestodb#25686

**Context:**
We want to consider worker load metrics to make scheduling decision from coordinator. Currently in single coordinator setup, it invokes /v1/info/state to get worker's state.

**Approach:**
Added a new end point to get nodeStats. The idea is that we would replace
"/v1/info/state" -> "/v1/info/nodestate" which will include nodeState + <worker load metrics>. This will help coordinator making scheduling decisions

Differential Revision: D76357677
@facebook-github-bot

Copy link
Copy Markdown
Collaborator

This pull request was exported from Phabricator. Differential Revision: D76357677

@prashantgolash

Copy link
Copy Markdown
Contributor Author

Test case doesn’t look related to this change.

@tdcmeehan tdcmeehan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to test this endpoint?

Comment on lines +49 to +52
@JsonProperty("cpuUsedPercent") Double cpuUsedPercent,
@JsonProperty("memoryUsedInBytes") Double memoryUsedInBytes,
@JsonProperty("cpuOverload") Boolean cpuOverload,
@JsonProperty("memoryOverload") Boolean memoryOverload)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
@JsonProperty("cpuUsedPercent") Double cpuUsedPercent,
@JsonProperty("memoryUsedInBytes") Double memoryUsedInBytes,
@JsonProperty("cpuOverload") Boolean cpuOverload,
@JsonProperty("memoryOverload") Boolean memoryOverload)
@JsonProperty("cpuUsedPercent") double cpuUsedPercent,
@JsonProperty("memoryUsedInBytes") double memoryUsedInBytes,
@JsonProperty("cpuOverload") boolean cpuOverload,
@JsonProperty("memoryOverload") boolean memoryOverload)

Comment thread presto-spi/src/main/java/com/facebook/presto/spi/NodeLoadMetrics.java Outdated
@facebook-github-bot

Copy link
Copy Markdown
Collaborator

This pull request was exported from Phabricator. Differential Revision: D76357677

@prashantgolash

Copy link
Copy Markdown
Contributor Author

Is it possible to test this endpoint?

Added the stand alone end point testing.

@spershin spershin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tim was asking for some endpoint test. Do we have tests for other endpoints, so we can craft this was similarly?

Comment thread presto-native-execution/presto_cpp/main/PrestoServer.cpp Outdated
Comment thread presto-native-execution/presto_cpp/main/PrestoServer.cpp Outdated
Comment thread presto-native-execution/presto_cpp/main/PrestoServer.cpp Outdated
@facebook-github-bot

Copy link
Copy Markdown
Collaborator

This pull request was exported from Phabricator. Differential Revision: D76357677

@prashantgolash

Copy link
Copy Markdown
Contributor Author

Tim was asking for some endpoint test. Do we have tests for other endpoints, so we can craft this was similarly?

We don't have any existing tests for endpoints. I updated my test plan.

@facebook-github-bot

Copy link
Copy Markdown
Collaborator

This pull request was exported from Phabricator. Differential Revision: D76357677

spershin
spershin previously approved these changes Aug 26, 2025

@spershin spershin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

Comment thread presto-native-execution/presto_cpp/main/PrestoServer.cpp
@facebook-github-bot

Copy link
Copy Markdown
Collaborator

This pull request was exported from Phabricator. Differential Revision: D76357677

1 similar comment
@facebook-github-bot

Copy link
Copy Markdown
Collaborator

This pull request was exported from Phabricator. Differential Revision: D76357677

…ad metrics (prestodb#25686)

Summary:

**Context:**
We want to consider worker load metrics to make scheduling decision from coordinator. Currently in single coordinator setup, it invokes /v1/info/state to get worker's state. 

**Approach:**
Added a new end point to get nodeStats. The idea is that we would replace 
"/v1/info/state" -> "/v1/info/nodestate" which will include nodeState + <worker load metrics>. This will help coordinator making scheduling decisions

Reviewed By: spershin

Differential Revision: D76357677
@facebook-github-bot

Copy link
Copy Markdown
Collaborator

This pull request was exported from Phabricator. Differential Revision: D76357677

@spershin spershin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.
Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants