Bug 75958

Summary: nrwt should use multiple http shards
Product: WebKit Reporter: Kristóf Kosztyó <kkristof>
Component: Tools / TestsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, dpranke, ojan, ossy, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 63116    
Bug Blocks:    
Attachments:
Description Flags
add --max-locked-shard option
none
add --max-locked-shard option
ossy: review-, ossy: commit-queue-
add --max-locked-shard option none

Kristóf Kosztyó
Reported 2012-01-10 06:40:24 PST
We ran some tests with multiple http shards on the Qt port and it seems stable, we haven't met any flaky http test. Maybe we can allow it by default or if the other ports still have problems with it we can make an option like --max-http-shards=i and set it's default value to 1.
Attachments
add --max-locked-shard option (2.58 KB, patch)
2012-01-11 01:12 PST, Kristóf Kosztyó 		no flags
DetailsFormatted DiffDiff
add --max-locked-shard option (2.65 KB, patch)
2012-01-12 05:52 PST, Kristóf Kosztyó 		ossy: review-
ossy: commit-queue-
DetailsFormatted DiffDiff
add --max-locked-shard option (5.23 KB, patch)
2012-01-13 04:56 PST, Kristóf Kosztyó 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Dirk Pranke
Comment 1 2012-01-10 11:29:21 PST
Sure, that seems fine. Feel free to add a patch, or I will get around to it sooner or later.
Kristóf Kosztyó
Comment 2 2012-01-11 01:12:50 PST
Created attachment 121992 [details] add --max-locked-shard option I added the option to set it manual so now we can test it easier.
Csaba Osztrogonác
Comment 3 2012-01-11 07:00:06 PST
Comment on attachment 121992 [details] add --max-locked-shard option View in context: https://bugs.webkit.org/attachment.cgi?id=121992&action=review LGTM. Any objection? > Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py:383 > + help="Set the maximum of locked shards"), only a little nit: "Maximum number of locked shards"
Ojan Vafai
Comment 4 2012-01-11 10:14:46 PST
Comment on attachment 121992 [details] add --max-locked-shard option View in context: https://bugs.webkit.org/attachment.cgi?id=121992&action=review > Tools/Scripts/webkitpy/layout_tests/controllers/manager.py:679 > + if self._options.max_locked_shards: > + num_of_locked_shards = self._options.max_locked_shards > + else: > + num_of_locked_shards = 1 How about adding a boolean option instead (e.g. --shard-http-tests)? Then we can return "max(math.ceil(num_workers / 4.0), 1)" and tweak that math until we get reasonable stability.
Csaba Osztrogonác
Comment 5 2012-01-12 05:21:19 PST
(In reply to comment #4) > How about adding a boolean option instead (e.g. --shard-http-tests)? Then we can return "max(math.ceil(num_workers / 4.0), 1)" and tweak that math until we get reasonable stability. I prefer the integer parameter, which let us use what we want. max(math.ceil(num_workers / 4.0) wouldn't be good for 4 or 8 workers, because 1 or 2 http shards isn't so useful ... I think num_of_locked_shards should be an integer option now, and we can move forward to increase the default value of it.
Kristóf Kosztyó
Comment 6 2012-01-12 05:52:01 PST
Created attachment 122223 [details] add --max-locked-shard option I fixed the grammar issues :)
Csaba Osztrogonác
Comment 7 2012-01-12 06:20:28 PST
Comment on attachment 122223 [details] add --max-locked-shard option Let's go forward, r=me.
Csaba Osztrogonác
Comment 8 2012-01-12 06:28:29 PST
Comment on attachment 122223 [details] add --max-locked-shard option r-, because it broke a unittest. :-/ ERROR: test_shard_by_dir (webkitpy.layout_tests.controllers.manager_unittest.ShardingTests) ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/oszi/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/manager_unittest.py", line 82, in test_shard_by_dir locked, unlocked = self.get_shards(num_workers=2, fully_parallel=False) File "/home/oszi/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/manager_unittest.py", line 79, in get_shards return self.manager._shard_tests(test_list, num_workers, fully_parallel) File "/home/oszi/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py", line 593, in _shard_tests return self._shard_by_directory(test_files, num_workers) File "/home/oszi/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py", line 664, in _shard_by_directory 'locked_shard'), File "/home/oszi/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py", line 706, in _resize_shards num_old_per_new = divide_and_round_up(len(old_shards), max_new_shards) File "/home/oszi/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py", line 695, in divide_and_round_up return int(math.ceil(float(numerator) / divisor)) TypeError: unsupported operand type(s) for /: 'float' and 'Mock' Could you check it and fix?
Kristóf Kosztyó
Comment 9 2012-01-13 04:56:28 PST
Created attachment 122412 [details] add --max-locked-shard option I fixed the unit test sorry about that, and I also write a new unit test for test it works properly.
WebKit Review Bot
Comment 10 2012-01-13 11:55:26 PST
Comment on attachment 122412 [details] add --max-locked-shard option Clearing flags on attachment: 122412 Committed r104967: <http://trac.webkit.org/changeset/104967>
WebKit Review Bot
Comment 11 2012-01-13 11:55:32 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.