Skip to content
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.

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

feat(compute_ctl): pass compute type to pageserver with pg_options #11287

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

skyzh
Copy link
Member

@skyzh skyzh commented Mar 17, 2025

Problem

second try of #11185, part of https://github.com/neondatabase/cloud/issues/24706

Summary of changes

Tristan reminded me of the options field of the pg wire protocol, which can be used to pass configurations. This patch adds the parsing on the pageserver side, and supplies neon.endpoint_type as part of the options.

@skyzh skyzh requested review from problame and tristan957 March 17, 2025 22:33
@skyzh skyzh requested review from a team as code owners March 17, 2025 22:33
@skyzh skyzh force-pushed the skyzh/compute-passthrough-try-2 branch from 86dd34f to d60fa40 Compare March 17, 2025 22:33
Copy link

If this PR added a GUC in the Postgres fork or neon extension,
please regenerate the Postgres settings in the cloud repo:

make NEON_WORKDIR=path/to/neon/checkout \
  -C goapp/internal/shareddomain/postgres generate

If you're an external contributor, a Neon employee will assist in
making sure this step is done.

@@ -2549,6 +2549,40 @@ where
if let Some(app_name) = params.get("application_name") {
Span::current().record("application_name", field::display(app_name));
}
if let Some(options) = params.get("options") {
Copy link
Member

Choose a reason for hiding this comment

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

Would it be better to just parse it with clap?

Copy link
Member Author

Choose a reason for hiding this comment

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

there's also -cXXX which I just fixed, this syntax is not supported by clap.

Copy link
Member

@tristan957 tristan957 left a comment

Choose a reason for hiding this comment

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

Looks pretty solid to me!

Copy link

github-actions bot commented Mar 17, 2025

7964 tests run: 7580 passed, 0 failed, 384 skipped (full report)


Flaky tests (4)

Postgres 17

Code coverage* (full report)

  • functions: 32.3% (8738 of 27028 functions)
  • lines: 48.4% (74980 of 154869 lines)

* collected from Rust tests only


The comment gets automatically updated with the latest test results
7a9fe7b at 2025-03-18T22:20:34.625Z :recycle:

@skyzh skyzh requested a review from tristan957 March 18, 2025 21:10
Signed-off-by: Alex Chi Z <[email protected]>
@skyzh skyzh force-pushed the skyzh/compute-passthrough-try-2 branch from d643af6 to 7a9fe7b Compare March 18, 2025 21:18
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