Executors: enable dequeueing and heartbeating for multiple queues by sanderginn · Pull Request #52016 · sourcegraph/sourcegraph-public-snapshot

sanderginn · 2023-05-16T16:46:38Z

Closes https://github.com/sourcegraph/sourcegraph/issues/50614

Depends on the following issues:

Due to the dependency on the work listed above, all PRs were ultimately merged into this branch, so the client side implementation got a little bit messy to review, for which I apologize. The initial intent of this PR, closing #50614, was reviewed before the merges occurred.

Demos

Scenario 1: old Sourcegraph version, new executor version

batches, single queue

1.old.SG.new.exec.BC.singlequeue.mov

codeintel, single queue

2.old.SG.new.exec.CI.singlequeue.mov

Scenario 2: new Sourcegraph version, old executor version

batches, single queue

3.new.SG.old.executor.BC.singlequeue.mov

codeintel, single queue

4.new.SG.old.executor.CI.singlequeue.mov

Scenario 3: new Sourcegraph version, new executor version

batches, single queue

5.new.SG.new.executor.BC.singlequeue.mov

codeintel, single queue

6.new.SG.new.executor.CI.singlequeue.mov

batches + codeintel in parallel, multi queue

7.multiqueue.processing.both.queues.mov

Initial PR description (enable client to dequeue from multiple queues)

This PR updates the client's Dequeue method to dequeue from multiple queues. It's backwards compatible with single-queue configurations. The field Job.Queue is only set when dequeuing from the general /dequeue endpoint, so the job MarkXxx methods default to single-queue behaviour if the job does not specify a queue name.

Unrelated to the linked issue, the multi-handler will log and return no content early in the event of an empty queue list in a dequeue request (although this should never occur).

Test plan

Unit tests
Local testing
Dogfood testing
Demo

sourcegraph-bot · 2023-05-16T17:01:58Z

Codenotify: Notifying subscribers in CODENOTIFY files for diff f05f3ed...4c88db8.

Notify	File(s)
@BolajiOlajide	client/web/src/enterprise/batches/batch-spec/batch-spec.mock.ts client/web/src/enterprise/batches/batch-spec/execute/backend.ts
@courier-new	client/web/src/enterprise/batches/batch-spec/batch-spec.mock.ts client/web/src/enterprise/batches/batch-spec/execute/backend.ts
@efritz	client/web/src/enterprise/executors/instances/ExecutorNode.tsx client/web/src/enterprise/executors/instances/useExecutors.tsx enterprise/cmd/executor/internal/apiclient/queue/client.go enterprise/cmd/executor/internal/apiclient/queue/client_test.go enterprise/cmd/executor/internal/apiclient/queue/options.go enterprise/cmd/executor/internal/config/config.go enterprise/cmd/executor/internal/config/config_test.go enterprise/cmd/executor/internal/run/util.go enterprise/cmd/executor/internal/worker/worker.go enterprise/cmd/frontend/internal/executorqueue/handler/BUILD.bazel enterprise/cmd/frontend/internal/executorqueue/handler/multihandler.go enterprise/cmd/frontend/internal/executorqueue/handler/multihandler_test.go enterprise/cmd/frontend/internal/executorqueue/queuehandler.go enterprise/internal/executor/types/http.go enterprise/internal/executor/types/http_test.go
@eseliger	client/web/src/enterprise/batches/batch-spec/batch-spec.mock.ts client/web/src/enterprise/batches/batch-spec/execute/backend.ts client/web/src/enterprise/executors/instances/ExecutorNode.tsx client/web/src/enterprise/executors/instances/useExecutors.tsx enterprise/cmd/frontend/internal/executorqueue/handler/BUILD.bazel enterprise/cmd/frontend/internal/executorqueue/handler/multihandler.go enterprise/cmd/frontend/internal/executorqueue/handler/multihandler_test.go enterprise/cmd/frontend/internal/executorqueue/queuehandler.go
@sourcegraph/delivery	doc/admin/executors/deploy_executors_binary.md doc/admin/executors/deploy_executors_terraform.md doc/admin/executors/executors_troubleshooting.md

Piszmog

Looks good! Feels too straight forward 👀

Could you also run a main dry run? I would like to ensure the E2E tests pass

- Closes https://github.com/sourcegraph/sourcegraph/issues/50616 - Coupled with https://github.com/sourcegraph/sourcegraph/pull/52016 (this is a child branch) This PR: - Adds an env var `EXECUTOR_QUEUE_NAMES` to the config of an executor, which supports a comma-separated string of queue names to listen to - Validates that one and only one of `EXECUTOR_QUEUE_NAME` and `EXECUTOR_QUEUE_NAMES` is configured - Sets the value on the `apiWorkerOptions` and `queueOptions` The actual client implementation is found in the parent PR. ## TODO - [x] Update docs - [ ] Update Terraform modules: see https://github.com/sourcegraph/sourcegraph/issues/52250 ## Test plan - [x] Unit tests - [ ] Demo (will be added to parent PR)

- Closes https://github.com/sourcegraph/sourcegraph/issues/50616 - Coupled with https://github.com/sourcegraph/sourcegraph/pull/52016 (this is a child branch) This PR: - Adds an env var `EXECUTOR_QUEUE_NAMES` to the config of an executor, which supports a comma-separated string of queue names to listen to - Validates that one and only one of `EXECUTOR_QUEUE_NAME` and `EXECUTOR_QUEUE_NAMES` is configured - Sets the value on the `apiWorkerOptions` and `queueOptions` The actual client implementation is found in the parent PR. - [x] Update docs - [ ] Update Terraform modules: see https://github.com/sourcegraph/sourcegraph/issues/52250 - [x] Unit tests - [ ] Demo (will be added to parent PR)

…e queues (#52525) - Fixes https://github.com/sourcegraph/sourcegraph/issues/51658 Hierarchy: - This merges into https://github.com/sourcegraph/sourcegraph/pull/52381 - Which merges into https://github.com/sourcegraph/sourcegraph/pull/52016 - Which merges into main ## Test plan - [x] Unit tests - [ ] Demo: all the way in the parent PR

…nd UI changes (#52381) - Fixes https://github.com/sourcegraph/sourcegraph/issues/51656 - depends on https://github.com/sourcegraph/sourcegraph/issues/51658, which will be next Opening a draft PR to get some early reviews in. This is not fully complete yet, because the heartbeat depends on the client and `Record` interface to implement the necessary changes in order for heartbeats to be submitted successfully. ## TODO - [ ] UI changes need some polishing - [x] Update `heartbeat` signature to take the refactored string IDs after client implementation - [x] More unit tests ## Test plan - [x] Local testing - [x] Unit tests - [ ] Demo (parent PR)

sourcegraph-buildkite · 2023-06-01T13:48:06Z

Bundle size report 📦

Initial size	Total size	Async size	Modules
0.00% (0.00 kb)	0.00% (+0.46 kb)	0.00% (+0.46 kb)	0.00% (0)

Look at the Statoscope report for a full comparison between the commits 4b412e2 and 42be886 or learn more.

Open explanation

Initial size is the size of the initial bundle (the one that is loaded when you open the page)
Total size is the size of the initial bundle + all the async loaded chunks
Async size is the size of all the async loaded chunks
Modules is the number of modules in the initial bundle

sourcegraph-bot · 2023-06-01T13:49:33Z

📖 Storybook live preview

Piszmog

Looks great! This is awesome!

Just one minor question on terraform.

…ence single queue

sanderginn · 2023-06-04T13:02:50Z

Completed testing, added demos.

All that is left is a changelog entry!

…2016) - Closes https://github.com/sourcegraph/sourcegraph/issues/50614 Depends on the following issues: - https://github.com/sourcegraph/sourcegraph/issues/50616 - Merged in https://github.com/sourcegraph/sourcegraph/pull/52020 - https://github.com/sourcegraph/sourcegraph/issues/51656 - Merged in https://github.com/sourcegraph/sourcegraph/pull/52381 - https://github.com/sourcegraph/sourcegraph/issues/51658 - Merged in https://github.com/sourcegraph/sourcegraph/pull/52525 Due to the dependency on the work listed above, all PRs were ultimately merged into this branch, so the client side implementation got a little bit messy to review, for which I apologize. The initial intent of this PR, closing #50614, was reviewed before the merges occurred. ## Demos ### Scenario 1: old Sourcegraph version, new executor version #### batches, single queue https://github.com/sourcegraph/sourcegraph/assets/2979513/6d325326-d4f5-4e6f-a5e0-0918dbde07bf #### codeintel, single queue https://github.com/sourcegraph/sourcegraph/assets/2979513/ff3a87db-ef07-4221-a159-73b93a6ae55b ### Scenario 2: new Sourcegraph version, old executor version #### batches, single queue https://github.com/sourcegraph/sourcegraph/assets/2979513/c66a9b9f-b745-4abc-bf87-9077dd5f2959 #### codeintel, single queue https://github.com/sourcegraph/sourcegraph/assets/2979513/785d7f34-630f-4c2a-abb6-82ca3b896813 ### Scenario 3: new Sourcegraph version, new executor version #### batches, single queue https://github.com/sourcegraph/sourcegraph/assets/2979513/69c5c06b-ea1c-47f7-9196-e3f108794493 #### codeintel, single queue https://github.com/sourcegraph/sourcegraph/assets/2979513/4e791d98-06cd-4fa7-93dd-81be0a73c744 #### batches + codeintel in parallel, multi queue https://github.com/sourcegraph/sourcegraph/assets/2979513/004608dd-f98c-4305-aeb3-a01444c12224 ## Initial PR description (enable client to dequeue from multiple queues) This PR updates the client's `Dequeue` method to dequeue from multiple queues. It's backwards compatible with single-queue configurations. The field `Job.Queue` is only set when dequeuing from the general `/dequeue` endpoint, so the job `MarkXxx` methods default to single-queue behaviour if the job does not specify a queue name. Unrelated to the linked issue, the multi-handler will log and return no content early in the event of an empty queue list in a dequeue request (although this should never occur). ## Test plan - [x] Unit tests - [x] Local testing - [x] Dogfood testing - [x] Demo

sanderginn added 10 commits May 16, 2023 14:48

Add QueueNames string slice to client options

973ddc9

configure observation for multiple queues

9a00785

Add multi-queue dequeue to client

b6fa77f

Add support for multi-queue to client_test.go

9d67822

Add multi-queue success case for client dequeue

2aaa6db

Add QueueNames to client

9fbe26a

Configure MarkComplete to use the queue on the job if present

17056ea

Add info log when dequeueing zero queues

1fbd379

Support multi-queue for MarkComplete

c0bddf3

Add multi-queue support to rest of MarkXxx

248c1b2

cla-bot Bot added the cla-signed label May 16, 2023

Fix merge conflicts

e7dbedc

sanderginn mentioned this pull request May 16, 2023

Executors: add multi-queue configuration #52020

Merged

4 tasks

sanderginn requested a review from a team May 16, 2023 17:09

sanderginn changed the title ~~Executors: configure client to dequeue from multiple queues~~ Executors: enable client to dequeue from multiple queues May 16, 2023

cleanup

a1c9e1a

Piszmog approved these changes May 16, 2023

View reviewed changes

Comment thread enterprise/cmd/frontend/internal/executorqueue/handler/multihandler.go

sanderginn force-pushed the sginn/executor-multi-dequeue-client branch from d49aef5 to 3afaebf Compare May 22, 2023 12:35

sanderginn and others added 8 commits May 22, 2023 14:41

Tweak TF deploy docs

9c0b02b

Merge branch 'main' into sginn/executor-multi-dequeue-client

6eb92ce

Add unit test for empty queue list in dequeue request

bb3df46

Fix ping on startup with multiple queues

b21b00b

Add multi-queue executor to sg.config.yaml

43d9def

Temp workaround for getting the heartbeat to work

d7054f4

Fix worker naming and just ignore the heartbeat for multiqueue executors

b3d3e38

return job ids immediately on multi-queue heartbeat requests

527ab54

sanderginn commented May 22, 2023

View reviewed changes

Comment thread enterprise/cmd/executor/internal/apiclient/queue/client.go Outdated

sanderginn added 2 commits May 26, 2023 15:46

Merge branch 'main' into sginn/executor-multi-dequeue-client

e7533ef

Merge main

dc3d0bc

sanderginn mentioned this pull request May 26, 2023

executors: multi-queue: enable client to submit heartbeat for multiple queues #52525

Merged

2 tasks

Merge main

4bab424

sanderginn changed the title ~~Executors: enable client to dequeue from multiple queues~~ Executors: enable dequeueing and heartbeating for multiple queues Jun 1, 2023

sanderginn added 2 commits June 1, 2023 17:03

sg/start: Add multi-queue enterprise codeintel command

158aad5

docs: generate reference

792b7d0

Piszmog approved these changes Jun 1, 2023

View reviewed changes

Comment thread doc/admin/executors/deploy_executors_terraform.md Outdated

Comment thread enterprise/cmd/executor/internal/apiclient/queue/client.go Outdated

sanderginn added 5 commits June 2, 2023 11:26

executor/queueclient: rename errs

4ae763f

docs/executors: comment out multi-queue Terraform entries

42f978c

executor/queueclient: check for presence of multiqueue instead of abs…

4b412e2

…ence single queue

merge main

dfa2a0d

Merge branch 'main' into sginn/executor-multi-dequeue-client

3ac032e

Add changelog entry

4c88db8

sanderginn enabled auto-merge (squash) June 4, 2023 13:12

sanderginn merged commit 0d6677a into main Jun 4, 2023

sanderginn deleted the sginn/executor-multi-dequeue-client branch June 4, 2023 13:25

courier-new mentioned this pull request Jun 4, 2023

sg: re-indent commands #52899

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Executors: enable dequeueing and heartbeating for multiple queues#52016

Executors: enable dequeueing and heartbeating for multiple queues#52016
sanderginn merged 33 commits into
mainfrom
sginn/executor-multi-dequeue-client

sanderginn commented May 16, 2023 •

edited

Loading

Uh oh!

sourcegraph-bot commented May 16, 2023 •

edited

Loading

Uh oh!

Piszmog left a comment

Uh oh!

Uh oh!

Uh oh!

sourcegraph-buildkite commented Jun 1, 2023 •

edited

Loading

Uh oh!

sourcegraph-bot commented Jun 1, 2023 •

edited

Loading

Uh oh!

Piszmog left a comment

Uh oh!

Uh oh!

Uh oh!

sanderginn commented Jun 4, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

sanderginn commented May 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Demos

Scenario 1: old Sourcegraph version, new executor version

batches, single queue

codeintel, single queue

Scenario 2: new Sourcegraph version, old executor version

batches, single queue

codeintel, single queue

Scenario 3: new Sourcegraph version, new executor version

batches, single queue

codeintel, single queue

batches + codeintel in parallel, multi queue

Initial PR description (enable client to dequeue from multiple queues)

Test plan

Uh oh!

sourcegraph-bot commented May 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Piszmog left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

sourcegraph-buildkite commented Jun 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Bundle size report 📦

Uh oh!

sourcegraph-bot commented Jun 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Piszmog left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

sanderginn commented Jun 4, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

sanderginn commented May 16, 2023 •

edited

Loading

sourcegraph-bot commented May 16, 2023 •

edited

Loading

sourcegraph-buildkite commented Jun 1, 2023 •

edited

Loading

sourcegraph-bot commented Jun 1, 2023 •

edited

Loading