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.

Sign up for GitHub

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

Jump to bottom

test/cql-pytest/test_tombstone_limit.py fails with tablets #16486

Closed
nyh opened this issue Dec 20, 2023 · 3 comments
Closed

test/cql-pytest/test_tombstone_limit.py fails with tablets #16486

nyh opened this issue Dec 20, 2023 · 3 comments
Assignees
@denesb
Labels
area/tablets area/test Issues related to the testing system code and environment Backport candidate

Comments

@nyh
Copy link
Contributor

nyh commented Dec 20, 2023

Four tests in test/cql-pytest/test_tombstone_limit.py fail with tablets enabled (see #16473 for a hack to enable tablets by default):

  • test_partition_tombstone_prefix
  • test_partition_tombstone_span
  • test_static_row_tombstone_prefix
  • test_static_row_tombstone_span

The failure in all of them is in the same place - the function check_pages_many_partitions() expects to have one empty page in addition to the desired page, but apparently doesn't get one. The comment above this function even mentions vnodes, but because I'm not familiar with this test (@denesb wrote it) it's not clear to me how they are related or why this empty page doesn't appear with tablets - and is this a real problem or just the test needs to be fixed.

CC @tgrabiec
@nyh nyh added area/test Issues related to the testing system code and environment area/tablets labels Dec 20, 2023
@denesb
Copy link
Contributor

denesb commented Dec 20, 2023

This is test problem. This test wants to test that range scans respect the tombstone limit. Range scans scan one vnode at a time (for the most part), so for this test to work, there has to be at least on vnode with enough partition tombstones to trip the limit. With tablest, the test will have to ensure at least one tablet has enough partition tombstones to do the same.

@denesb denesb self-assigned this Dec 20, 2023
@nyh
Copy link
Contributor Author

nyh commented Dec 20, 2023

This is test problem. This test wants to test that range scans respect the tombstone limit. Range scans scan one vnode at a time (for the most part), so for this test to work, there has to be at least on vnode with enough partition tombstones to trip the limit. With tablest, the test will have to ensure at least one tablet has enough partition tombstones to do the same.

Oh, I see, thanks. Then I guess the fact I ran with test with initial_tablets = 100 forced on it (this is not yet automatic) didn't help - it is much more than the number of vnodes that test/cql-pytest/run usually uses (16).

@denesb
Copy link
Contributor

denesb commented Dec 21, 2023

Oh, I see, thanks. Then I guess the fact I ran with test with initial_tablets = 100 forced on it (this is not yet automatic) didn't help - it is much more than the number of vnodes that test/cql-pytest/run usually uses (16).

Yes, this test will want as small as possible tablet count.

denesb added a commit to denesb/scylla that referenced this issue May 14, 2024
@denesb
test/cql-pytest: test_tombstone_limit.py: enable xfailing tests
5fe2d75 
These tests were marked as xfail because they use to fail with tablets.
They don't anymore, so remove the xfail.

Fixes: scylladb#16486
@denesb denesb mentioned this issue May 14, 2024
Closed
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@denesb denesb

Labels
area/tablets area/test Issues related to the testing system code and environment Backport candidate
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

test/cql-pytest: test_tombstone_limit.py: enable xfailing tests denesb/scylla
3 participants
@nyh @denesb @scylladb-promoter