-
Notifications
You must be signed in to change notification settings - Fork 409
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
Add performance regress test_ondemand_download_churn.py
#7242
Add performance regress test_ondemand_download_churn.py
#7242
Conversation
test_runner/performance/pageserver/pagebench/test_ondemand_download_churn.py
Outdated
Show resolved
Hide resolved
test_runner/performance/pageserver/pagebench/test_ondemand_download_churn.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this! A couple of thoughts:
- Latency stats are useful, and you copied the right pattern with the task-locals; but they need to be partitioned by the request type, i.e., is it an eviction or an on-demand download. Also, I'm afraid that the histogram range might be optimized for getpage-level latencies from the get-page-latest-lsn sub-benchmark, not on-demand download latencies. So, overall, I don't know whether it makes sense to have latency stats yet.
- Instead, the idea behind on-demand download churn was to determine max throughput.
- I don't think we the
n_tenants
andpgbench_scale
cruft that you likely copied over from the get-page-latest-lsn sub-benchmark. Please read the public Notion page I linked from the issue; it describes the setup: a single tenant initialized with pgbench using the parameters described in that Notion doc should be sufficent to max out throughput form real S3 to an i3en.3xlarge EC2 instance.
Of course, if you don't want to spend the money on AWS, you can set up a local minio
instance for testing.
I don't think we have good documentation for how to run the locally run the test suite with real S3.
Probably the best instructions for that are in
neon/.github/workflows/build_and_test.yml
Lines 369 to 374 in 7d32af5
# Run separate tests for real S3 | |
export ENABLE_REAL_S3_REMOTE_STORAGE=nonempty | |
export REMOTE_STORAGE_S3_BUCKET=neon-github-ci-tests | |
export REMOTE_STORAGE_S3_REGION=eu-central-1 | |
# Avoid `$CARGO_FEATURES` since there's no `testing` feature in the e2e tests now | |
${cov_prefix} cargo nextest run $CARGO_FLAGS -E 'package(remote_storage)' -E 'test(test_real_s3)' |
Feel free to augment [
test_runner/README.md
](Useful environment variables:) once you get it running locally.
Again, thanks for taking a stab at this, as described in the issue, it's a fairly self-contained task but requires some ramp-up with the test suite.
Please re-request review once you've made changes. I'll be unavailable until next Tuesday. |
The tests with 10, and 100 |
I have found the issue and fixed it with the last commit. The requests are dropped at the end of the test and it is detected as an error in logs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the late reply.
I merged from main
and pushed some changes, mostly orthogonal to yours.
Couple of high-level remarks:
- The
ondemand_download_churn.rs
should produce absolute numbers instead of the per-second average. - the addition of
AWS_ENDPOINT_URL
: I guess you needed that forminio
. At Neon, we use AWS_PROFILE for this sort of stuff. So, you'd use a custom aws profile for the minio S3 endpoint. An example for how to configure the profile is givin here. Would be nice if we can streamline your instructions in the README to just use AWS_PROFILE.
Again, sorry for the delayed reply. Let me know if you're still willing to pick up this PR.
No issues, yes I would like to pick it up, and finish this test |
Thanks, go ahead and re-request review from me if that's possible, otherwise reply to this thread. |
I have changed the output to look like this now: {
"downloads": 447,
"evictions": 609,
"runtime": {
"nanos": 32640994,
"secs": 30
},
"timeline_restarts": 1
}
I have also added usage of ./target/release/neon_local init --config=/tmp/tmphvpqctqc --pg-version 15 --pageserver-config-override=remote_storage={ bucket_name = "neon-bucket", bucket_region = "eu-central-1", prefix_in_bucket = "2eedf453-65aa-43b7-a1ed-379824d75180/test-download-churn-release-pg15-1-tokio-epoll-uring-30-/pageserver", endpoint = "http://127.0.0.1:9000" }
--pageserver-config-override=page_cache_size=16384 --pageserver-config-override=max_file_descriptors=500000 and I have not found a nice way to extract an |
The neon/libs/remote_storage/src/lib.rs Lines 557 to 563 in c18d334
Also, this week, I merged a bunch of PRs that change how neon_local populates pageserver config; tl;dr: the config override stuff is gone, the |
I'll try out your instructions and changes and report back. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please re-request review once more when PR #7664 has been merged and this PR has been rebased 🙏
We're close to wrapping up!
3147 tests run: 3007 passed, 0 failed, 140 skipped (full report)Flaky tests (2)Postgres 16
Postgres 14
Code coverage* (full report)
* collected from Rust tests only The comment gets automatically updated with the latest test results
9e5cd29 at 2024-05-15T16:03:16.541Z :recycle: |
test_runner/performance/pageserver/pagebench/test_ondemand_download_churn.py
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pushed some final changes from my side.
Requesting review from @bayandin before merging this.
@bayandin I think this benchmark makes sense when run on EC2 against real S3. Should we mark it skip until we have that? |
Merging main and doing a CI run to get benchmarks run in #7717 (I've added a label to that PR) |
Last commit for fixing the error:
|
test_runner/performance/pageserver/pagebench/test_ondemand_download_churn.py
Show resolved
Hide resolved
It's done, @jbajic ! Thanks for your patience! |
Thank you for the support and reviews! |
Add performance regress test for on-demand download throughput. Closes #7146 Co-authored-by: Christian Schwarz <christian@neon.tech> Co-authored-by: Alexander Bayandin <alexander@neon.tech>
Summary of changes
Add performance regress test
test_ondemand_download_churn.py
.Closes #7146
Tagging @problame
Checklist before requesting a review
Checklist before merging