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

Make test_random_updates and test_read_at_max_lsn compatible with new compaction #7551

Merged
merged 2 commits into from
Apr 30, 2024

Conversation

arpad-m
Copy link
Member

@arpad-m arpad-m commented Apr 29, 2024

Makes two of the tests work with the tiered compaction that I had to ignore in #7283.

The issue was that tiered compaction actually created image layers, but the keys didn't appear in them as collect_keyspace didn't include them. Not a compaction problem, but due to how the test is structured.

Fixes #7287

@arpad-m arpad-m requested a review from a team as a code owner April 29, 2024 23:16
@arpad-m arpad-m requested a review from skyzh April 29, 2024 23:16
Copy link

github-actions bot commented Apr 29, 2024

2874 tests run: 2753 passed, 0 failed, 121 skipped (full report)


Flaky tests (3)

Postgres 15

  • test_gc_of_remote_layers: release
  • test_vm_bit_clear_on_heap_lock: debug

Postgres 14

  • test_create_churn_during_restart: release

Code coverage* (full report)

  • functions: 28.2% (6580 of 23351 functions)
  • lines: 46.9% (46744 of 99680 lines)

* collected from Rust tests only


The comment gets automatically updated with the latest test results
8df3169 at 2024-04-30T11:56:20.006Z :recycle:

skyzh
skyzh previously approved these changes Apr 30, 2024
@skyzh skyzh dismissed their stale review April 30, 2024 14:18

note a minor issue

Copy link
Member

@skyzh skyzh left a comment

Choose a reason for hiding this comment

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

LGTM, but I feel a better fix is to temporary add this keyspace to collect_keyspace in testing mode, or have some tests with both random update + compaction in the future.

@arpad-m arpad-m merged commit 010f0a3 into main Apr 30, 2024
54 checks passed
@arpad-m arpad-m deleted the arpad/compaction_last_key branch April 30, 2024 14:52
@arpad-m
Copy link
Member Author

arpad-m commented Apr 30, 2024

I feel a better fix is to temporary add this keyspace to collect_keyspace in testing mode, or have some tests with both random update + compaction in the future.

Yeah it's not perfect, but I'm sure there is other tests in the testsuite which do updates plus compaction. Note that the purpose of the tests is not to test compaction, otherwise probably greater care would have been taken to make the keys show up in collect_keyspace.

If you know of a good mechanism to make the keys be included in collect_keyspace, feel free to suggest it.

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.

tiered compaction: can't get last key
2 participants