Skip to content

Refactor SLRU download interface between Postgres and the neon extension #12799

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

hlinnaka
Copy link
Contributor

Move the responsibility of writing the SLRU segment contents to the neon extension. The neon extension hook now downloads the file and places it to the correct path, and returns a true/false to the caller to indicate if the file existed. On successful download, the caller retries opening the file.

Corresponding Postgres PRs:

Move the responsibility of writing the SLRU segment contents to the
neon extension. The neon extension hook now downloads the file and
places it to the correct path, and returns a true/false to the caller
to indicate if the file existed. On successful download, the caller
retries opening the file.
@hlinnaka hlinnaka requested review from a team as code owners July 31, 2025 15:01
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.

@hlinnaka
Copy link
Contributor Author

This is in preparation for the new communicator implementation. That implementation will write the downloaded segment directly to the file, rather than a palloc'd buffer. This refactors the API to be like that even with the current communicator implementation, to keep minimize the differences between the two.

(The current implementation was slightly broken with the new communicator: SimpleLruDownloadSegment did pfree(NULL), which segfaults. That was the immediate impetus for doing this right now, but I've wanted to do this refactoring for the sake of tidiness anyway)

@hlinnaka hlinnaka removed the request for review from myrrc July 31, 2025 15:06
Copy link

9119 tests run: 8465 passed, 0 failed, 654 skipped (full report)


Code coverage* (full report)

  • functions: 34.7% (8840 of 25482 functions)
  • lines: 45.7% (71640 of 156719 lines)

* collected from Rust tests only


The comment gets automatically updated with the latest test results
1d59754 at 2025-07-31T16:35:36.670Z :recycle:

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.

1 participant