This repository was archived by the owner on Sep 30, 2024. It is now read-only.
Executors: add multi-queue configuration#52020
Merged
sanderginn merged 8 commits intoMay 22, 2023
Merged
Conversation
4 tasks
Contributor
|
Codenotify: Notifying subscribers in CODENOTIFY files for diff e7dbedc...01c3774.
|
Piszmog
reviewed
May 16, 2023
Piszmog
left a comment
Contributor
There was a problem hiding this comment.
Great work! Some minor nits and an update to the docs.
Contributor
Author
|
Created https://github.com/sourcegraph/sourcegraph/issues/52250 to support multiple queues for Terraform deployments |
Contributor
Author
|
Build fails on some unrelated stuff, should be OK in the parent branch which merges into main |
sanderginn
added a commit
that referenced
this pull request
May 26, 2023
- 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) <!-- All pull requests REQUIRE a test plan: https://docs.sourcegraph.com/dev/background-information/testing_principles -->
sanderginn
referenced
this pull request
Jun 4, 2023
…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 <!-- All pull requests REQUIRE a test plan: https://docs.sourcegraph.com/dev/background-information/testing_principles -->
ErikaRS
referenced
this pull request
Jun 22, 2023
…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 <!-- All pull requests REQUIRE a test plan: https://docs.sourcegraph.com/dev/background-information/testing_principles -->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR:
EXECUTOR_QUEUE_NAMESto the config of an executor, which supports a comma-separated string of queue names to listen toEXECUTOR_QUEUE_NAMEandEXECUTOR_QUEUE_NAMESis configuredapiWorkerOptionsandqueueOptionsThe actual client implementation is found in the parent PR.
TODO
Test plan