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

Tar Remote Extensions #4715

Merged
merged 14 commits into from
Aug 2, 2023
Merged

Tar Remote Extensions #4715

merged 14 commits into from
Aug 2, 2023

Conversation

awestover
Copy link
Contributor

@awestover awestover commented Jul 13, 2023

Add support for remote extensions. When requested, downloads a tar.gz file for the extension and then organizes the contained files. For instance, placing .so files in sharelib.

@awestover awestover requested review from a team as code owners July 13, 2023 19:57
@awestover awestover requested review from knizhnik, LizardWizzard, Daniel-ef and hlinnaka and removed request for a team July 13, 2023 19:57
@github-actions
Copy link

github-actions bot commented Jul 13, 2023

1264 tests run: 1213 passed, 0 failed, 51 skipped (full report)


@awestover awestover marked this pull request as draft July 13, 2023 19:59
@awestover awestover requested a review from vadim2404 July 13, 2023 20:09
@awestover awestover marked this pull request as ready for review July 13, 2023 20:53
@awestover
Copy link
Contributor Author

For local testing / testing on staging, the most relevant metric to look at is

state.metrics.load_libraries_ms = library_load_time;
"Loading shared_preload_libraries took {:?}ms", library_load_time

(because shared preload libraries are loaded during compute_start)

@awestover awestover self-assigned this Jul 14, 2023
@awestover awestover requested a review from lubennikovaav July 14, 2023 14:37
@awestover
Copy link
Contributor Author

awestover commented Jul 14, 2023

@cicdteam is working on improving the infra for this: https://github.com/neondatabase/aws/pull/405#issue-1804986517
EDIT: finished

@awestover
Copy link
Contributor Author

awestover commented Jul 20, 2023

Note: tests with real S3 are expected to fail right now because we haven't uploaded the necessary files to S3 yet.

@awestover
Copy link
Contributor Author

awestover commented Jul 20, 2023

TODO:

Merge postgres changes:

Control Plane changes:

Upload files to S3 via Dockerfile.compute-node

Infra changes:

  • create prod buckets

@cicdteam
Copy link
Contributor

This need to solve to allow computes access to S3 buckets (extension storage)
https://github.com/neondatabase/cloud/issues/6025

@awestover awestover force-pushed the alek_targz branch 2 times, most recently from cced72a to 63d5d6c Compare July 28, 2023 14:53
@awestover
Copy link
Contributor Author

awestover commented Jul 31, 2023

it looks like the first round trip to AWS is really expensive even if it's downloading a really small file? like ~3KB
never mind, this was because I was in a different region from the bucket
on github it is more like ~50ms per was round trip

@awestover awestover force-pushed the alek_targz branch 2 times, most recently from 512f010 to d82987f Compare July 31, 2023 13:47
@awestover
Copy link
Contributor Author

pretty sure the cloud test failures are unrelated

Copy link
Contributor Author

@awestover awestover left a comment

Choose a reason for hiding this comment

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

I left some comments about a couple potential changes. nothing major

compute_tools/src/compute.rs Outdated Show resolved Hide resolved
docs/rfcs/024-extension-loading.md Show resolved Hide resolved
control_plane/src/endpoint.rs Outdated Show resolved Hide resolved
libs/remote_storage/src/s3_bucket.rs Outdated Show resolved Hide resolved
test_runner/regress/test_download_extensions.py Outdated Show resolved Hide resolved
vendor/postgres-v14 Outdated Show resolved Hide resolved
vendor/revisions.json Outdated Show resolved Hide resolved
@lubennikovaav lubennikovaav merged commit d005c77 into main Aug 2, 2023
@lubennikovaav lubennikovaav deleted the alek_targz branch August 2, 2023 09:38
koivunej added a commit that referenced this pull request Aug 2, 2023
as needed since #4715 or this will happen:

```
ERROR panic{thread=main location=.../hyper-rustls-0.23.2/src/config.rs:48:9}: no CA certificates found
```
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.

6 participants