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

fix(dragonfly_test): failing bug207 on build opt #1919

Merged
merged 3 commits into from Sep 25, 2023

Conversation

kostasrim
Copy link
Contributor

@kostasrim kostasrim commented Sep 23, 2023

The issue was that the Heartbeat would run every 1ms and this is problematic because the for-loop would take less than 1ms to finish. Therefore, the memory pool would not adjust and items would not be evicted from the store. By doubling the amount of elements created in the for-loop, we give enough time for the first heartbeat to run and adjust the memory available (which will cause the evictions to happen).

This passes locally but if we see flakes on the CI we can slightly increase the elements we create in the forloop again.

@kostasrim kostasrim self-assigned this Sep 23, 2023
@kostasrim kostasrim enabled auto-merge (squash) September 23, 2023 10:10
@kostasrim
Copy link
Contributor Author

kostasrim commented Sep 23, 2023

Do not review, I will make another change and make RunPeriodic use nanoseconds instead of milliseconds. a) They get converted anyway to nanoseconds since helio uses steady_clock::duration which uses nanoseconds. b) it's a better solution

Small update @romange, since your suggestion fixes the issue I am fine for now with this fix (That is 10k calls to SETEX instead of 5k). as my suggestion above needs to do changes in helio and I don't want to delay this PR (since our tests are broken atm)

@romange
Copy link
Collaborator

romange commented Sep 23, 2023

dcheck at db_slice.cc:155 is incorrect. should be DCHECK_LE

@kostasrim kostasrim merged commit 91e8ead into main Sep 25, 2023
10 checks passed
@kostasrim kostasrim deleted the fix_failing_bug207_on_build_opt branch September 25, 2023 08:02
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.

None yet

3 participants