New issue
Advanced search Search tips

Issue 728124 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Feature

Blocked on:
issue 728127

Blocking:
issue 781021



Sign in to add a comment

Allow a dimension key to be specified multiple times for Swarming task request

Project Member Reported by mar...@chromium.org, May 31 2017

Issue description

This is necessary for named caches, where a task may require multiple named caches.

This requires a schema change, as the task request dimensions are currently stored as a dict, so a key can only have one value.
 

Comment 1 by mar...@chromium.org, May 31 2017

Blockedon: 728127
Project Member

Comment 2 by bugdroid1@chromium.org, Jun 8 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/external/github.com/luci/luci-py.git/+/1bd482d4014fd63fb2a263fd3e60b328f8be59ab

commit 1bd482d4014fd63fb2a263fd3e60b328f8be59ab
Author: maruel <maruel@chromium.org>
Date: Thu Jun 08 13:26:53 2017

client: Support repeated keys in task request.

- Add more flags to 'swarming.py bots': --idle, --busy, --mp, --non-mp
- Change 'swarming.py bots' dimensions filtering to be done server side.
- Refactor 'swarming.py bots' to reuse the same code as 'swarming.py query'.
- Support for the same key to be specified multiple times is needed for named
  caches, as unlike the other dimensions, it makes sense to specify the same key
  mulitple time.
- Only add support on client side, server side is to follow up shortly.

R=vadimsh@chromium.org
BUG= 728124 

Review-Url: https://codereview.chromium.org/2923633003

[modify] https://crrev.com/1bd482d4014fd63fb2a263fd3e60b328f8be59ab/client/swarming.py
[modify] https://crrev.com/1bd482d4014fd63fb2a263fd3e60b328f8be59ab/client/tests/swarming_test.py

Project Member

Comment 3 by bugdroid1@chromium.org, Jun 8 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2ce3fe82e43aaa500f5147c0232159fe5e5bd6f7

commit 2ce3fe82e43aaa500f5147c0232159fe5e5bd6f7
Author: Marc-Antoine Ruel <maruel@chromium.org>
Date: Thu Jun 08 15:00:05 2017

Roll src/tools/swarming_client/ 5c4eed888..af6b06ca6 (4 commits)

https://chromium.googlesource.com/external/swarming.client.git/+log/5c4eed888354..af6b06ca68ba

$ git log 5c4eed888..af6b06ca6 --date=short --no-merges --format='%ad %ae %s'
2017-06-08 maruel client: Support repeated keys in task request.
2017-06-02 nodir Fix GS URL regex
2017-05-31 maruel Improve swarming.py help page.
2017-05-26 maruel Overhaul swarming_task_cost.py

Created with:
  roll-dep src/tools/swarming_client

The roll is primarily for repeated keys support.

TBR=nodir@chromium.org

Bug:  728124 
Change-Id: I79a47c331aa2062b95bc5e466facf96de75805e7
Reviewed-on: https://chromium-review.googlesource.com/528053
Reviewed-by: Marc-Antoine Ruel <maruel@chromium.org>
Commit-Queue: Marc-Antoine Ruel <maruel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#477966}
[modify] https://crrev.com/2ce3fe82e43aaa500f5147c0232159fe5e5bd6f7/DEPS

What will be the logic: OR or AND? e.g if task specifies dim:A and dim:B, will it be picked up by a bot that exposes dim:A OR dim:B, or both dim:A AND dim:B?
Also, do we really need this change now? Does it block anything? It looks dangerous to me, since it changes deep guts of Swarming scheduler.
Owner: mar...@chromium.org
Status: Started (was: Available)
It's an AND. I sent a separate CL to document this right away in the message, https://codereview.chromium.org/2931063002
We need it specifically for hot caches. So you can request a bot that has two caches, let's say 'git_cache' and 'chromium_win'.

Another hypothetical use is when a task needs two different CPU flags so it would specify the 'cpu' key multiple times.
Project Member

Comment 8 by bugdroid1@chromium.org, Jun 9 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/external/github.com/luci/luci-py.git/+/6ad3c41630bbc5371ed955c750a2116c5d0b0a2a

commit 6ad3c41630bbc5371ed955c750a2116c5d0b0a2a
Author: maruel <maruel@chromium.org>
Date: Fri Jun 09 19:47:49 2017

swarming: Add more documentation to task messages

In particular note right away that NewTaskRequest.dimensions is an AND: all
values specified must match.

R=vadimsh@chromium.org
BUG= 728124 

Review-Url: https://codereview.chromium.org/2931063002

[modify] https://crrev.com/6ad3c41630bbc5371ed955c750a2116c5d0b0a2a/appengine/swarming/swarming_rpcs.py

Project Member

Comment 9 by bugdroid1@chromium.org, Feb 15 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/luci/luci-py.git/+/cdfcd4ef99627350c5d1d9b23548b0abb8ddec7a

commit cdfcd4ef99627350c5d1d9b23548b0abb8ddec7a
Author: Marc-Antoine Ruel <maruel@chromium.org>
Date: Thu Feb 15 13:47:22 2018

Support both TaskProperties.dimensions as dict of key value or key values.

This change is both a backward and forward compatible change.

The entity is still stored as the old format, so old Swarming code can still
load the new entitiy. Only the internal processing is updated.

A follow up change will start writing dict(unicode: list(unicode), which will
break backward compatibility.

The only remaining inconsistency is TaskProperties.dimensions that should have
been a StringListPair but this is not a big deal and this is not worth breaking
API compatibility.

---

Another option was to use list((unicode, unicode))} but it was decided against
as this increases the stored struct size with no immediate benefit.

R=vadimsh@chromium.org
BUG= 728124 

Change-Id: Ie4779958306cc40f246819f0a74415b2086b7930
Reviewed-on: https://chromium-review.googlesource.com/916844
Reviewed-by: Vadim Shtayura <vadimsh@chromium.org>
Commit-Queue: Marc-Antoine Ruel <maruel@chromium.org>

[modify] https://crrev.com/cdfcd4ef99627350c5d1d9b23548b0abb8ddec7a/appengine/swarming/event_mon_metrics.py
[modify] https://crrev.com/cdfcd4ef99627350c5d1d9b23548b0abb8ddec7a/appengine/swarming/handlers_bot.py
[modify] https://crrev.com/cdfcd4ef99627350c5d1d9b23548b0abb8ddec7a/appengine/swarming/message_conversion.py
[modify] https://crrev.com/cdfcd4ef99627350c5d1d9b23548b0abb8ddec7a/appengine/swarming/server/service_accounts_test.py
[modify] https://crrev.com/cdfcd4ef99627350c5d1d9b23548b0abb8ddec7a/appengine/swarming/server/task_queues.py
[modify] https://crrev.com/cdfcd4ef99627350c5d1d9b23548b0abb8ddec7a/appengine/swarming/server/task_queues_test.py
[modify] https://crrev.com/cdfcd4ef99627350c5d1d9b23548b0abb8ddec7a/appengine/swarming/server/task_request.py
[modify] https://crrev.com/cdfcd4ef99627350c5d1d9b23548b0abb8ddec7a/appengine/swarming/server/task_request_test.py
[modify] https://crrev.com/cdfcd4ef99627350c5d1d9b23548b0abb8ddec7a/appengine/swarming/server/task_result_test.py
[modify] https://crrev.com/cdfcd4ef99627350c5d1d9b23548b0abb8ddec7a/appengine/swarming/server/task_scheduler.py
[modify] https://crrev.com/cdfcd4ef99627350c5d1d9b23548b0abb8ddec7a/appengine/swarming/server/task_scheduler_test.py
[modify] https://crrev.com/cdfcd4ef99627350c5d1d9b23548b0abb8ddec7a/appengine/swarming/server/task_to_run.py
[modify] https://crrev.com/cdfcd4ef99627350c5d1d9b23548b0abb8ddec7a/appengine/swarming/server/task_to_run_test.py
[modify] https://crrev.com/cdfcd4ef99627350c5d1d9b23548b0abb8ddec7a/appengine/swarming/swarming_rpcs.py

Project Member

Comment 10 by bugdroid1@chromium.org, Feb 16 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/luci/luci-py.git/+/46d6688e2575f2bd431a5946f42298f584ee6e79

commit 46d6688e2575f2bd431a5946f42298f584ee6e79
Author: Marc-Antoine Ruel <maruel@chromium.org>
Date: Fri Feb 16 14:28:08 2018

Swarming: Send the full dict to the bot.

Split as a spearate change just in case it blows up due to an hook.

R=vadimsh@chromium.org

Bug:  728124 
Change-Id: I0b7ff14d9863f6d08b8e453f28fdc3561f9f4a71
Reviewed-on: https://chromium-review.googlesource.com/920821
Reviewed-by: Vadim Shtayura <vadimsh@chromium.org>
Commit-Queue: Marc-Antoine Ruel <maruel@chromium.org>

[modify] https://crrev.com/46d6688e2575f2bd431a5946f42298f584ee6e79/appengine/swarming/handlers_bot.py
[modify] https://crrev.com/46d6688e2575f2bd431a5946f42298f584ee6e79/appengine/swarming/handlers_bot_test.py
[modify] https://crrev.com/46d6688e2575f2bd431a5946f42298f584ee6e79/appengine/swarming/swarming_bot/config/bot_config.py

Project Member

Comment 11 by bugdroid1@chromium.org, Feb 20 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/luci/luci-py.git/+/20b3cdc91c2e99490ef8136eaf524ed34953505e

commit 20b3cdc91c2e99490ef8136eaf524ed34953505e
Author: Marc-Antoine Ruel <maruel@chromium.org>
Date: Tue Feb 20 14:59:00 2018

Swarming: Enable multiple-values and add smoke test to confirm it works.

This enable using the same key multiple times in the task properties. This is
necessary to be able to request two separate named caches.

Deny specifying the same value multiple times and add a unit test to confirm.

This commit changes properties_hash, causing instantaneous invalidation of task
deduplication algorithm. It happened before, it's not a big deal.

R=vadimsh@chromium.org

Bug:  728124 
Change-Id: I0cd85d4baf57b9c3e8a93088d8ba3b3469892a07
Reviewed-on: https://chromium-review.googlesource.com/920403
Reviewed-by: Vadim Shtayura <vadimsh@chromium.org>
Commit-Queue: Marc-Antoine Ruel <maruel@chromium.org>

[modify] https://crrev.com/20b3cdc91c2e99490ef8136eaf524ed34953505e/appengine/swarming/local_smoke_test.py
[modify] https://crrev.com/20b3cdc91c2e99490ef8136eaf524ed34953505e/appengine/swarming/message_conversion.py
[modify] https://crrev.com/20b3cdc91c2e99490ef8136eaf524ed34953505e/appengine/swarming/server/service_accounts_test.py
[modify] https://crrev.com/20b3cdc91c2e99490ef8136eaf524ed34953505e/appengine/swarming/server/task_queues_test.py
[modify] https://crrev.com/20b3cdc91c2e99490ef8136eaf524ed34953505e/appengine/swarming/server/task_request.py
[modify] https://crrev.com/20b3cdc91c2e99490ef8136eaf524ed34953505e/appengine/swarming/server/task_request_test.py
[modify] https://crrev.com/20b3cdc91c2e99490ef8136eaf524ed34953505e/appengine/swarming/server/task_result_test.py
[modify] https://crrev.com/20b3cdc91c2e99490ef8136eaf524ed34953505e/appengine/swarming/server/task_scheduler_test.py
[modify] https://crrev.com/20b3cdc91c2e99490ef8136eaf524ed34953505e/appengine/swarming/server/task_to_run_test.py

Project Member

Comment 12 by bugdroid1@chromium.org, Feb 20 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/luci/luci-py.git/+/057e88573a768916a329e28cd9cfd460027067de

commit 057e88573a768916a329e28cd9cfd460027067de
Author: Marc-Antoine Ruel <maruel@chromium.org>
Date: Tue Feb 20 19:30:40 2018

Swarming: Fix /internal/cron/count_task_bot_distribution

The cron job was broken by 3336-20b3cdc. It was detected via ereporter
because there was no unit test.

Add very minimal testing for /internal/cron/count_task_bot_distribution that
would have caught the regression.

R=vadimsh@chromium.org

Bug:  728124 
Change-Id: I3adb4502562fc32b5cb6674f0038f3274fc98982
Reviewed-on: https://chromium-review.googlesource.com/926865
Reviewed-by: Vadim Shtayura <vadimsh@chromium.org>
Commit-Queue: Marc-Antoine Ruel <maruel@chromium.org>

[modify] https://crrev.com/057e88573a768916a329e28cd9cfd460027067de/appengine/swarming/handlers_backend.py
[modify] https://crrev.com/057e88573a768916a329e28cd9cfd460027067de/appengine/swarming/handlers_test.py

Status: Fixed (was: Started)
Deployed live.
Blocking: 781021

Sign in to add a comment