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

test: add test_cache_cluster_data_migration #4435

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

BorysTheDev
Copy link
Contributor

@BorysTheDev BorysTheDev commented Jan 9, 2025

fixes: #4354

@BorysTheDev BorysTheDev force-pushed the eviction_during_migration branch from 8555c1d to a023cae Compare January 14, 2025 13:08
@BorysTheDev BorysTheDev marked this pull request as ready for review January 14, 2025 16:38
@BorysTheDev BorysTheDev requested a review from adiholden January 14, 2025 16:38
@BorysTheDev BorysTheDev changed the title test: add test_cache_cluster_data_migration (not ready for review) test: add test_cache_cluster_data_migration Jan 15, 2025
@BorysTheDev BorysTheDev requested a review from chakaz January 15, 2025 08:01
src/server/cluster_support.h Outdated Show resolved Hide resolved
src/server/transaction.cc Outdated Show resolved Hide resolved
@BorysTheDev BorysTheDev changed the title test: add test_cache_cluster_data_migration fix: slot calculation in transaction squashing Jan 15, 2025
src/server/cluster_support.h Outdated Show resolved Hide resolved
src/server/cluster_support.h Outdated Show resolved Hide resolved
@BorysTheDev BorysTheDev force-pushed the eviction_during_migration branch from e4f3fdc to 30094db Compare January 15, 2025 09:44
@BorysTheDev BorysTheDev force-pushed the eviction_during_migration branch from 30094db to 20c3c91 Compare January 15, 2025 10:11
@BorysTheDev BorysTheDev force-pushed the eviction_during_migration branch from 20c3c91 to 334a081 Compare January 15, 2025 12:52
@BorysTheDev BorysTheDev changed the title fix: slot calculation in transaction squashing test: add test_cache_cluster_data_migration Jan 15, 2025
@BorysTheDev BorysTheDev force-pushed the eviction_during_migration branch from 334a081 to baf2978 Compare January 17, 2025 09:58

gen_stop = False

async def expiration_gen():
Copy link
Collaborator

Choose a reason for hiding this comment

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

you can use ExpirySeeder

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will check

i = 0
try:
while not gen_stop:
await nodes[0].client.execute_command(f"SET exp_key_{i} val_{i} EX 20")
Copy link
Collaborator

Choose a reason for hiding this comment

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

When I run your test the first call here gets OOM exeptions therefor no command here is executed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's ok if test is finished successfully

Copy link
Collaborator

Choose a reason for hiding this comment

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

but it sometimes fails at least for me

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's interesting, where?

Copy link
Collaborator

Choose a reason for hiding this comment

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

last assertion assert stats["expired_keys"] > 0

fill_exp_task = asyncio.create_task(expiration_gen())

# some time to fill data
await asyncio.sleep(20)
Copy link
Collaborator

Choose a reason for hiding this comment

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

why 20 seconds?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

to fill enough data and start some keys expire

Copy link
Collaborator

Choose a reason for hiding this comment

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

the first debug populate fills enough data for evictions as far as I understand.
also you can use a smaller expiration time to check expiry f.e 2 seconds

Copy link
Contributor Author

Choose a reason for hiding this comment

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

2 seconds isn't enough because it's random

Copy link
Collaborator

Choose a reason for hiding this comment

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

what is random?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we don't know what the slot of the next key and if the interval is small we can get already migrated slots keys or they will expire before migration

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.

Add cluster migration test in cache mode
3 participants