-
Notifications
You must be signed in to change notification settings - Fork 6.3k
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
split RandomizedHarnessTest more ways #3467
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@miasantreble has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
table/table_test.cc
Outdated
RandomizedHarnessTest(part, total); | ||
} | ||
|
||
TEST_F(HarnessTest, Randomized5) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where is Randomized 2 3 and 4?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
naming is a bit off, maybe rename randomized0 to randomized1n2, randomized1 to randomized3n4?
table/table_test.cc
Outdated
const size_t part = 2; | ||
const size_t total = 2; | ||
const size_t total = 4; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should not this be 8?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
randomized0 does first quarter of work, randomized1 does second quarter.
The remaining half are split 4 ways into randomized5 through randomized 8.
In other words, [0, 1] is split into [0, 0.25, 0.5, 0.625, 0.75, 0.875, 1] segments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I see. How about keeping "total = 8" and can call RandomizedHarnessTest(part, total) twice when you want it to do twice the work, like
RandomizedHarnessTest(part, total);
RandomizedHarnessTest(part + 1, total);
@miasantreble has updated the pull request. View: changes, changes since last import |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@miasantreble has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@miasantreble is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
RandomizedHarnessTest enumerates different combinations of test type, compression type, restart interval, etc. For some combinations it takes very long to finish, causing the test to time out in test infrastructure.
This PR split the test input into smaller trunks in the hope that they will fit in the timeout window. Another possibility is to reduce
num_entries
of course