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

CASSANDRA-18131 Fix LongBTreeTest test timeout #2178

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

Conversation

Mmuzaf
Copy link
Contributor

@Mmuzaf Mmuzaf commented Feb 27, 2023

Thanks for sending a pull request! Here are some tips if you're new here:

  • Ensure you have added or run the appropriate tests for your PR.
  • Be sure to keep the PR description updated to reflect all changes.
  • Write your PR title to summarize what this PR proposes.
  • If possible, provide a concise example to reproduce the issue for a faster review.
  • Read our contributor guidelines
  • If you're making a documentation change, see our guide to documentation contribution

Commit messages should follow the following format:

<One sentence description, usually Jira title or CHANGES.txt summary>

<Optional lengthier description (context on patch)>

patch by <Authors>; reviewed by <Reviewers> for CASSANDRA-#####

Co-authored-by: Name1 <email1>
Co-authored-by: Name2 <email2>

The Cassandra Jira

@@ -785,7 +784,7 @@ public void testBatchesSmallOverlappingRange() throws ExecutionException, Interr
@Test
public void testIndividualInsertsMediumSparseRange() throws ExecutionException, InterruptedException
{
testInsertions(randomSeed(), perThreadTrees / 10, 500, 10, 1, true);
testInsertions(randomSeed(), 500, 10, 1, true);
Copy link
Member

Choose a reason for hiding this comment

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

the default value of this tests parameter is perThreadTrees * threads, so we could be replacing values of 1000 with 25600 (where there are 32 availableProcessors). Note in ant we limit this to 2 processors here so a value of 1600 (which is still an increase).

is this what you intended @Mmuzaf ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct, I think we can safely increase the number of local runs for testLargeBatchesLargeRange, and testIndividualInsertsMediumSparseRange while still maintaining a reasonable execution time for the LongBTreeTest as a whole.

For the tests that fire testInsertions with parameters the main concern we have is - we perform different numbers of test attempts to check tree insertions for variations of ranges and batches, but the same equal number of tests should be used for all of them. I think perThreadTrees * threads is the better choice for us since increasing the perThreadTrees (for testInsertions it's not exactly trees per thread as it says, it's just a multiplier) will give us the ability to find patterns e.g. for a heap consumption during a particular run.

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 is also possible to just set the perThreadTrees for 10000 back to 100 as an easy fix, I guess. I just wanted to fix the problem wider and maybe I overdid 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
2 participants