Bug 75958 – nrwt should use multiple http shards

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Kristóf Kosztyó]

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.

[Dirk Pranke]

Sure, that seems fine. Feel free to add a patch, or I will get around to it sooner or later.

[Kristóf Kosztyó]

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 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 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]

(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ó]

Created attachment 122223 [details]
add --max-locked-shard option

I fixed the grammar issues :)

[Csaba Osztrogonác]

Comment on attachment 122223 [details]
add --max-locked-shard option

Let's go forward, r=me.

[Csaba Osztrogonác]

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ó]

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 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]

All reviewed patches have been landed.  Closing bug.