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

bench: run branch_creation_many at 500, seeded #6959

Merged
merged 13 commits into from Mar 7, 2024
Merged

bench: run branch_creation_many at 500, seeded #6959

merged 13 commits into from Mar 7, 2024

Conversation

koivunej
Copy link
Contributor

@koivunej koivunej commented Feb 29, 2024

We have a benchmark for creating a lot of branches, but it does random things, and the branch count is not what we is the largest maximum we aim to support. If this PR would stabilize the benchmark total duration it means that there are some structures which are very much slower than others. Then we should add a seed-outputting variant to help find and reproduce such cases.

Additionally, record for the benchmark:

  • shutdown duration
  • startup metrics once done (on restart)
  • duration of first compaction completion via debug logging

@koivunej koivunej requested review from a team and VladLazar and removed request for a team February 29, 2024 09:26
Copy link
Contributor

@VladLazar VladLazar left a comment

Choose a reason for hiding this comment

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

Looks good. I'd suggest adding a temporary pytest.mark.repeat() on the test to check for flakiness before merging.

Copy link

github-actions bot commented Feb 29, 2024

2570 tests run: 2437 passed, 0 failed, 133 skipped (full report)


Flaky tests (2)

Postgres 16

  • test_compute_pageserver_connection_stress: release
  • test_vm_bit_clear_on_heap_lock: debug

Code coverage* (full report)

  • functions: 28.8% (6992 of 24266 functions)
  • lines: 47.4% (43008 of 90731 lines)

* collected from Rust tests only


The comment gets automatically updated with the latest test results
8e2913d at 2024-03-07T13:40:10.407Z :recycle:

@koivunej
Copy link
Contributor Author

Looks good. I'd suggest adding a temporary pytest.mark.repeat() on the test to check for flakiness before merging.

You mean the seeding could make it flaky or what? Note we don't even run these without run-benchmarks label, which I was not thinking this would require.

@koivunej koivunej added the run-benchmarks Indicates to the CI that benchmarks should be run for PR marked with this label label Feb 29, 2024
@koivunej koivunej marked this pull request as draft February 29, 2024 13:39
@koivunej
Copy link
Contributor Author

Drafted to avoid the e2e while waiting the new token.

@koivunej
Copy link
Contributor Author

koivunej commented Mar 1, 2024

Well, this PR took long enough to get going, might as well continue with the added metrics.

@koivunej
Copy link
Contributor Author

koivunej commented Mar 6, 2024

Example run output for me:

test_branch_creation_many[release-pg15-one_ancestor-500].branch_creation_duration_max: 0.042 s
test_branch_creation_many[release-pg15-one_ancestor-500].branch_creation_duration_avg: 0.030 s
test_branch_creation_many[release-pg15-one_ancestor-500].branch_creation_duration_stdev: 0.002 s
test_branch_creation_many[release-pg15-one_ancestor-500].shutdown: 0.713 s
test_branch_creation_many[release-pg15-one_ancestor-500].restart_after.background_jobs_can_start: 1.025 s
test_branch_creation_many[release-pg15-one_ancestor-500].restart_after.complete: 1.025 s
test_branch_creation_many[release-pg15-one_ancestor-500].restart_after.initial: 0.011 s
test_branch_creation_many[release-pg15-one_ancestor-500].restart_after.initial_tenant_load: 1.025 s
test_branch_creation_many[release-pg15-one_ancestor-500].restart_after.initial_tenant_load_remote: 1.025 s
test_branch_creation_many[release-pg15-one_ancestor-500].compaction: 5.689 s

@koivunej koivunej requested a review from VladLazar March 6, 2024 14:24
@koivunej koivunej marked this pull request as ready for review March 6, 2024 14:29
@koivunej koivunej requested a review from a team as a code owner March 6, 2024 14:29
it is unnecessary because we know we are on the second run already.
@koivunej koivunej merged commit 602a4da into main Mar 7, 2024
57 checks passed
@koivunej koivunej deleted the n_branches branch March 7, 2024 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
run-benchmarks Indicates to the CI that benchmarks should be run for PR marked with this label
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants