Closed Bug 1229132 Opened 10 years ago Closed 10 years ago

Add docker-worker support for coalescing

Categories

(Taskcluster :: Workers, defect)

Product:
Taskcluster
Component:
Workers
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: dustin, Assigned: dustin)

References

Details

Attachments

(1 file)

coalescer PR 193
53 bytes, text/x-github-pull-request
garndt
: review+
Details | Review
In order to support coalescing, docker-worker needs to be able to call out to a task-defined URL after claiming a task, and get back a list of tasks to claim in addition. It then claims as many of those tasks as possible, performing the last one it successfully claims. When the task is complete, it is resolved as usual, with the other tasks resolved as "coalesced" or some similar status. Per Greg's advice, this will involve modifying the queueservice to return a set of tasks from claimTask, all already claimed and with a "distinguished" task that should actually be performed. This feature should be controllable per workerType, so on workerTypes where it is not enabled, claimTask will just return a single task in the taskset, and the behavior will be the same as it is now. When the feature is enabled, claimTask will make the appropriate call to the coalescing service.
Depends on: 1230344
Attached file coalescer PR 193 —
Comments in the PR
Attachment #8702927 - Flags: review+
Testing this now. A few things of note: - payload.coalescer.routePrefix is not used, preferring a constant in lib/task_listener.js. And actually, the two differ (coalescer.v1, vs coalesce.v1)
https://github.com/taskcluster/docker-worker/pull/199 adds some logging as to why things aren't coalesced, and also fixes the issue above.
..and after that, the error messages made it clear that the coalescer responses were not what I thought when I wrote the unit tests -- the coalescer returns an object {coalescerKey: [..taskIds..]}. Fixed that in https://github.com/djmitche/docker-worker/commit/8614aa16342bd8b9b4413231061d95dd0fa1985e Now, {"type":"bad coalescer response","source":"top","provisionerId":"aws-provisioner-v1","workerId":"i-82c19a5b","workerGroup":"us-west-2b","workerType":"ami-test","workerNodeType":"r3.xlarge","taskId":"ZFeXUtE5TRGXlOb9dj6bQg","runId":0,"response":["KSPcK2SwSs2rHNYyBDWtAw","ZDcTMFp6R2WNe4uN1fU_gA","fBN5HKJ6TiKmMgRQpu6Ynw","avq0eqHXSmikL2430xw7Gg"],"message":"taskIds do not include ZFeXUtE5TRGXlOb9dj6bQg"} The test tasks, in order of creation, were DuGztd-zRKySN1roV2Tafg completed 0:completed f4bbPiV6RZ6mrMFQKvAXRA completed 0:completed A0YMUgjQQHmIHBqHJ1fKfg completed 0:completed UnfUXY2VTUm8pjIrqgqcSg completed 0:completed a3G0Mkf8QJG7YCUWVTOCzw completed 0:completed ZFeXUtE5TRGXlOb9dj6bQg pending 0:pending avq0eqHXSmikL2430xw7Gg pending 0:pending fBN5HKJ6TiKmMgRQpu6Ynw pending 0:pending ZDcTMFp6R2WNe4uN1fU_gA pending 0:pending KSPcK2SwSs2rHNYyBDWtAw pending 0:pending The coalescer call came right after the call to queue.claimTask('ZFeXUtE5TRGXlOb9dj6bQg'), which would cause the queue to send a message to the task-running exchange. On receipt of such a message, the coalescer removes the task from the list. So clearly we have a race condition, but I'm surprised that the pulse message is winning the race. I suppose there's no reason to care if that task is included or not. https://github.com/djmitche/docker-worker/commit/890579d8f8870da66dcc72abbf42c88b5099e698
Hoewever, no artifacts were added. Here's why: Dec 30 13:23:17 docker-worker.aws-provisioner.us-east-1d.ami-4ad38120.r3-xlarge.i-26383890 docker-worker: {"type":"task error","source":"top","provisionerId":"aws-provisioner-v1","workerId":"i-26383890","workerGroup":"us-east-1d","workerType":"ami-test","workerNodeType":"r3.xlarge","taskId":"KGt8egfvRaqxczIRgOScaw","message":"ReferenceError: taskId is not defined","stack":"ReferenceError: taskId is not defined\n at Task.resolveSuperseded$ (/home/ubuntu/docker_worker/lib/task.js:614:9)\n at tryCatch (/usr/local/lib/node_modules/babel/node_modules/regenerator-babel/runtime.js:53:40)\n at GeneratorFunctionPrototype.invoke (/usr/local/lib/node_modules/babel/node_modules/regenerator-babel/runtime.js:209:22)\n at tryCatch (/usr/local/lib/node_modules/babel/node_modules/regenerator-babel/runtime.js:53:40)\n at Function.step (/usr/local/lib/node_modules/babel/node_modules/regenerator-babel/runtime.js:103:22)\n at runMicrotasksCallback (node.js:337:7)\n at process._tickDomainCallback (node.js:381:11)","err":{}}
OK, well, that's it for this year. For my reference: * task injector at euclid:~/tmp/injector.py * latest bug ("TypeError: Cannot read property 'runtime' of undefined") is fixed but no new AMI generated yet; that bug is masking another bug which will be discovered in 2016.
{"type":"while resolving superseded task: ReferenceError: contentJson is not defined","source":"top","provisionerId":"aws-provisioner-v1","workerId":"i-126a46cb","workerGroup":"us-west-2b","workerType":"ami-test","workerNodeType":"r3.xlarge","primaryTaskId":"bm0O21YlQ3GPFYf8N2WBiw","primaryRunId":0,"taskId":"MH5uhGxtSgOMwuMsDAIGsg","runId":0} I'm willing to accept that maybe -- JUST MAYBE -- this code could use more integration testing..
r+ on the PR, looks good to me. Awesome to see it working!
OK, I updated the opt-linux64 workerType to use this AMI. Let's see if that fails for some other reason; if not, I'll add it to the other workerTypes: android-api-11 dbg-linux32 dbg-linux64 dbg-macosx64 opt-linux32 opt-macosx64
Well, opt-linux64 is green, so I've changed the others. I think this is done!
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Depends on: 1213039
Blocks: 1213039
No longer depends on: 1213039
Component: Docker-Worker → Workers
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: