Executors: multi-queue: add heartbeat endpoint stub, db migrations, and UI changes by sanderginn · Pull Request #52381 · sourcegraph/sourcegraph-public-snapshot

sanderginn · 2023-05-24T17:57:35Z

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
Update heartbeat signature to take the refactored string IDs after client implementation
More unit tests

Test plan

Local testing
Unit tests
Demo (parent PR)

sourcegraph-buildkite · 2023-05-24T18:04:03Z

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 5d7af86 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

courier-new

Frontend approved! 🫡

Piszmog

Overall, I think it looks great! Lots of changes you made to support this feature. Great job!

kalanchan · 2023-05-24T19:06:16Z

let's get a demo of this in demo days once it's complete :)

Piszmog

Nice job!

- 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

sourcegraph-bot · 2023-05-31T17:25:25Z

Codenotify: Notifying subscribers in CODENOTIFY files for diff 4bab424...5d7af86.

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/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_terraform.md

sanderginn · 2023-05-31T17:26:53Z

For the ones who got pinged when I changed this to a regular PR from draft, nothing has changed aside from merging in a child PR that was reviewed already, so no action needed

sourcegraph-bot · 2023-05-31T17:33:28Z

📖 Storybook live preview

sanderginn · 2023-05-31T18:30:24Z

@sourcegraph/design
This PR slightly tweaks how executors are displayed in the site admin. It adds a badge for every queue name that is configured on the executor. There's currently no margin between each badge. Long executor names also cause multiline strings which doesn't look very good, but this is not introduced in this PR and has been in main for as long as I know.

Am I right to ping design on this? It doesn't require immediate attention but could be polished. Not sure if I should tag frontend instead.

sanderginn · 2023-06-01T13:41:15Z

FYI, I added a mr-1 class to all but the last badge. Good enough for now.

Piszmog · 2023-06-01T14:24:45Z

Looks good. I think I remember hearing that it is up to devs whether to pull in design or things.

I think since this is a minor change, what you did doesn't need review.

…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

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

sanderginn requested a review from a team May 24, 2023 17:57