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

Add test for last written lsn cache #1949

Merged
merged 7 commits into from
Sep 4, 2022
Merged

Add test for last written lsn cache #1949

merged 7 commits into from
Sep 4, 2022

Conversation

knizhnik
Copy link
Contributor

Main:

test_measure_read_latency_heavy_write_workload[neon-1].run_duration: 29.455 s
test_measure_read_latency_heavy_write_workload[neon-1].read_latency_avg: 4.346 s
test_measure_read_latency_heavy_write_workload[neon-1].read_latency_stdev: 5.499 s

last_written_lsn_cache:

test_measure_read_latency_heavy_write_workload[neon-1].run_duration: 34.074 s
test_measure_read_latency_heavy_write_workload[neon-1].read_latency_avg: 0.422 s
test_measure_read_latency_heavy_write_workload[neon-1].read_latency_stdev: 0.761 s

@knizhnik knizhnik requested a review from hlinnaka June 17, 2022 05:30
@aome510
Copy link
Contributor

aome510 commented Jun 17, 2022

Look like this branch is based on the code in #1891 as record_read_latency isn't defined anywhere in main.

The new test shares some similarities with the tests added in #1891. I think we can use tests in #1891 to test the last_written_lsn_cache implementation instead.

Also, looks like CircleCI doesn't run because "We have detected an anomaly that violates the CircleCI Terms of Service. Troubleshoot", which is probably because the PR is submitted from Russia.

@knizhnik knizhnik force-pushed the last_written_lsn_cache branch from 95f8d5a to 7702bcd Compare June 28, 2022 04:50
@knizhnik knizhnik force-pushed the last_written_lsn_cache branch from bf737c4 to 5351567 Compare July 23, 2022 12:13
@knizhnik knizhnik force-pushed the last_written_lsn_cache branch 2 times, most recently from 49060a9 to 1aeba2b Compare August 11, 2022 06:49
@knizhnik knizhnik force-pushed the last_written_lsn_cache branch 2 times, most recently from 419c9ff to fb60c3b Compare August 22, 2022 05:17
@knizhnik knizhnik force-pushed the last_written_lsn_cache branch 2 times, most recently from e14b163 to 543f547 Compare August 31, 2022 07:33
@hlinnaka
Copy link
Contributor

hlinnaka commented Sep 2, 2022

Can you separate the test changes and the code changes into two commits, and push them separately, please? That way, we'll get "before" and "after" measurements from the CI.

@knizhnik
Copy link
Contributor Author

knizhnik commented Sep 2, 2022

Can you separate the test changes and the code changes into two commits, and push them separately, please? That way, we'll get "before" and "after" measurements from the CI.

Do you mean create separate PR for tests or just add tests in separate commit within this PR?
But in the last case I afraid that "squash and marge" will still group them in one commit...

@hlinnaka
Copy link
Contributor

hlinnaka commented Sep 2, 2022

Can you separate the test changes and the code changes into two commits, and push them separately, please? That way, we'll get "before" and "after" measurements from the CI.

Do you mean create separate PR for tests or just add tests in separate commit within this PR? But in the last case I afraid that "squash and marge" will still group them in one commit...

You can use squash them into two commits on your laptop with "git rebase -i origin/master", and force push to the branch to update the PR. After the CI passes, you can do a regular push from your laptop with "git push origin last_written_lsn_cache:main".

Hmm. Not sure the CI would run on 'main' separately for the two commits if you do that though. So two separate PRs is probably the easiest way to do it.

@knizhnik knizhnik force-pushed the last_written_lsn_cache branch from 4dea1a5 to 7c848c6 Compare September 3, 2022 14:15
@knizhnik
Copy link
Contributor Author

knizhnik commented Sep 3, 2022

Can you separate the test changes and the code changes into two commits, and push them separately, please? That way, we'll get "before" and "after" measurements from the CI.

Do you mean create separate PR for tests or just add tests in separate commit within this PR? But in the last case I afraid that "squash and marge" will still group them in one commit...

You can use squash them into two commits on your laptop with "git rebase -i origin/master", and force push to the branch to update the PR. After the CI passes, you can do a regular push from your laptop with "git push origin last_written_lsn_cache:main".

Hmm. Not sure the CI would run on 'main' separately for the two commits if you do that though. So two separate PRs is probably the easiest way to do it.

I have merged PR with tests and rebased this PR.
But to be able to merge it I need your approval for its postgres repository part:
neondatabase/postgres#201

@knizhnik knizhnik merged commit 846d71b into main Sep 4, 2022
@knizhnik knizhnik deleted the last_written_lsn_cache branch September 4, 2022 19:25
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.

3 participants