Skip to content

Add request-data-sizes-max-wait-sec to session properties #24918

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
HeidiHan0000 merged 6 commits into from
May 9, 2025
Merged

Add request-data-sizes-max-wait-sec to session properties #24918

HeidiHan0000 merged 6 commits into from
May 9, 2025

Conversation

@HeidiHan0000
Copy link
Contributor

@HeidiHan0000 HeidiHan0000 commented Apr 15, 2025

Summary: As title

Differential Revision: D73019179

@HeidiHan0000 HeidiHan0000 requested review from a team, elharo and steveburnett as code owners April 15, 2025 06:53
@prestodb-ci prestodb-ci added the from:Meta PR from Meta label Apr 15, 2025
@facebook-github-bot
Copy link
Collaborator

facebook-github-bot commented Apr 15, 2025

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

steveburnett
steveburnett previously approved these changes Apr 15, 2025
View reviewed changes
Copy link
Contributor

@steveburnett steveburnett left a comment

Choose a reason for hiding this comment

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

LGTM! (docs)

Pull branch, local doc build, looks good.

Thanks for the doc!

jaystarshot
jaystarshot reviewed Apr 15, 2025
View reviewed changes
presto-docs/src/main/sphinx/presto_cpp/properties-session.rst Outdated
* **Type:** ``integer``
* **Default value:** ``10``

Maximum wait time for exchange request in seconds.
Copy link
Member

@jaystarshot jaystarshot Apr 15, 2025

Choose a reason for hiding this comment

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

Can you please add what happens when the limit is exceeded?

Copy link
Collaborator

@kewang1024 kewang1024 Apr 15, 2025

Choose a reason for hiding this comment

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

This is the max wait time for exchange long-poll protocol

Copy link
Contributor

@rschlussel rschlussel Apr 16, 2025

Choose a reason for hiding this comment

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

Can you expand the description (here and in the code) to make that more explicit? e.g. maximum wait time for exchange long poll requests.

kewang1024
kewang1024 previously approved these changes Apr 15, 2025
View reviewed changes
presto-docs/src/main/sphinx/presto_cpp/properties-session.rst Outdated
* **Type:** ``integer``
* **Default value:** ``10``

Maximum wait time for exchange request in seconds.
Copy link
Collaborator

@kewang1024 kewang1024 Apr 15, 2025

Choose a reason for hiding this comment

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

This is the max wait time for exchange long-poll protocol

@kewang1024 kewang1024 requested a review from rschlussel April 15, 2025 23:54
HeidiHan0000 added a commit to HeidiHan0000/presto that referenced this pull request Apr 19, 2025
@HeidiHan0000 @facebook-github-bot
Add request-data-sizes-max-wait-sec to session properties (prestodb#2…
d4360b0 
…4918)

Summary:

As title

Reviewed By: kewang1024

Differential Revision: D73019179
@HeidiHan0000 HeidiHan0000 dismissed stale reviews from kewang1024 and steveburnett via d4360b0 April 19, 2025 02:03
@facebook-github-bot
Copy link
Collaborator

facebook-github-bot commented Apr 19, 2025

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

HeidiHan0000 added a commit to HeidiHan0000/presto that referenced this pull request Apr 19, 2025
@HeidiHan0000 @facebook-github-bot
Add request-data-sizes-max-wait-sec to session properties (prestodb#2…
41ddb79 
…4918)

Summary:
Pull Request resolved: prestodb#24918

As title

Reviewed By: kewang1024

Differential Revision: D73019179
HeidiHan0000 added a commit to HeidiHan0000/presto that referenced this pull request Apr 20, 2025
@HeidiHan0000 @facebook-github-bot
Add request-data-sizes-max-wait-sec to session properties (prestodb#2…
4099dbc 
…4918)

Summary:

As title

Reviewed By: kewang1024

Differential Revision: D73019179
@facebook-github-bot
Copy link
Collaborator

facebook-github-bot commented Apr 20, 2025

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

rschlussel
rschlussel previously approved these changes Apr 22, 2025
View reviewed changes
HeidiHan0000 added a commit to HeidiHan0000/presto that referenced this pull request Apr 26, 2025
@HeidiHan0000 @facebook-github-bot
Add request-data-sizes-max-wait-sec to session properties (prestodb#2…
a7284d3 
…4918)

Summary:

As title

Reviewed By: kewang1024

Differential Revision: D73019179
HeidiHan0000 added a commit to HeidiHan0000/presto that referenced this pull request Apr 26, 2025
@HeidiHan0000 @facebook-github-bot
Add request-data-sizes-max-wait-sec to session properties (prestodb#2…
f924b5f 
…4918)

Summary:

As title

Reviewed By: kewang1024

Differential Revision: D73019179
@facebook-github-bot
Copy link
Collaborator

facebook-github-bot commented Apr 26, 2025

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

1 similar comment
@facebook-github-bot
Copy link
Collaborator

facebook-github-bot commented Apr 26, 2025

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

@facebook-github-bot
Copy link
Collaborator

facebook-github-bot commented May 6, 2025

@HeidiHan0000 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

1 similar comment
@facebook-github-bot
Copy link
Collaborator

facebook-github-bot commented May 6, 2025

@HeidiHan0000 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

rschlussel
rschlussel previously approved these changes May 6, 2025
View reviewed changes
@HeidiHan0000 HeidiHan0000 requested a review from steveburnett May 6, 2025 17:38
steveburnett
steveburnett previously approved these changes May 6, 2025
View reviewed changes
Copy link
Contributor

@steveburnett steveburnett left a comment

Choose a reason for hiding this comment

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

LGTM! (docs)

Pull updated branch, new local doc build, looks good. Thanks!

kewang1024
kewang1024 previously approved these changes May 7, 2025
View reviewed changes
@kewang1024
Copy link
Collaborator

kewang1024 commented May 7, 2025

Thanks @HeidiHan0000, looks like there is a build error?

@facebook-github-bot
Copy link
Collaborator

facebook-github-bot commented May 7, 2025

@HeidiHan0000 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@HeidiHan0000 HeidiHan0000 dismissed stale reviews from kewang1024, steveburnett, and rschlussel via 4c46fb0 May 8, 2025 11:53
@facebook-github-bot
Copy link
Collaborator

facebook-github-bot commented May 8, 2025

@HeidiHan0000 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@kewang1024
Copy link
Collaborator

kewang1024 commented May 9, 2025

Can we merge all the commits? looks like we have 6 commits

@rschlussel
Copy link
Contributor

rschlussel commented May 9, 2025

oh yes, please rebase and squash to one commit before merging.

steveburnett
steveburnett approved these changes May 9, 2025
View reviewed changes
Copy link
Contributor

@steveburnett steveburnett left a comment

Choose a reason for hiding this comment

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

LGTM! (docs)

@HeidiHan0000 HeidiHan0000 merged commit 941ffb2 into prestodb:master May 9, 2025
108 checks passed
@ZacBlanco ZacBlanco mentioned this pull request May 29, 2025
Merged
21 tasks
This was referenced May 30, 2025
Closed
Closed
@aditi-pandit aditi-pandit mentioned this pull request Jun 4, 2025
Merged
AnuragKDwivedi pushed a commit to AnuragKDwivedi/presto-1 that referenced this pull request Jul 8, 2025
@HeidiHan0000 @AnuragKDwivedi
Add request-data-sizes-max-wait-sec to session properties (prestodb#2…
f7a9d2e 
…4918)

Summary: As title

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

Reviewers

@steveburnett steveburnett steveburnett approved these changes

@elharo elharo Awaiting requested review from elharo elharo is a code owner

@jaystarshot jaystarshot Awaiting requested review from jaystarshot

@sdruzkin sdruzkin Awaiting requested review from sdruzkin

@shrinidhijoshi shrinidhijoshi Awaiting requested review from shrinidhijoshi

@feilong-liu feilong-liu Awaiting requested review from feilong-liu

@ClarenceThreepwood ClarenceThreepwood Awaiting requested review from ClarenceThreepwood

@rschlussel rschlussel Awaiting requested review from rschlussel

@kewang1024 kewang1024 Awaiting requested review from kewang1024

Assignees

No one assigned

Labels

fb-exported from:Meta PR from Meta

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@HeidiHan0000 @facebook-github-bot @kewang1024 @rschlussel @steveburnett @jaystarshot @prestodb-ci