sql: add is_draining to system.sql_instances table.#126765
Closed
shubhamdhama wants to merge 1 commit intocockroachdb:masterfrom
Closed
sql: add is_draining to system.sql_instances table.#126765shubhamdhama wants to merge 1 commit intocockroachdb:masterfrom
shubhamdhama wants to merge 1 commit intocockroachdb:masterfrom
Conversation
Member
|
This change is |
stevendanna
reviewed
Jul 5, 2024
886cfdf to
48e7ace
Compare
d855ae0 to
d6b7dda
Compare
d6b7dda to
6f6420e
Compare
shubhamdhama
added a commit
to shubhamdhama/cockroach
that referenced
this pull request
Aug 4, 2024
This change would prevent distsql planner to include a draining node in a plan by leveraging the system.sql_instances table's `is_draining` field added in cockroachdb#126765. Somewhat related work to avoid planning on an unavailable node: cockroachdb#124986 Fixes: cockroachdb#100578 Epic: CRDB-31575 Release note: None
jeffswenson
reviewed
Aug 5, 2024
Collaborator
|
@shubhamdhama Just as a heads up, I believe we have the new versions on master now so this should be good to proceed again. |
6f6420e to
d361800
Compare
shubhamdhama
commented
Aug 26, 2024
This is a preliminary commit toward improving the DistSQL to avoid planning on an instance that is draining. The idea here is to set this column to true as soon as the draining request is received by the server and use that through instance reader in the planner at gateway. Adding this column requires writing a migration for the upgrade. And since this new column is nullable we don't need to backfill and we are good with just swapping the descriptors. Informs: cockroachdb#100578 Release note: None --- Development notes: To update `pkg/sql/catalog/bootstrap/testdata/testdata` I had to manually update the hash values for `system`/`tenant` in this file and pass the `--rewrite` option to do the rest: ``` dev test //pkg/sql/catalog/bootstrap:bootstrap_test --filter=TestInitialValuesToString/testdata --rewrite ``` To update tests in `pkg/sql/catalog/systemschema_test/testdata/bootstrap_{system,tenant}` ``` dev test //pkg/sql/catalog/systemschema_test:systemschema_test_test --rewrite ```
d361800 to
9133722
Compare
shubhamdhama
added a commit
to shubhamdhama/cockroach
that referenced
this pull request
Aug 26, 2024
This change would prevent distsql planner to include a draining node in a plan by leveraging the system.sql_instances table's `is_draining` field added in cockroachdb#126765. Somewhat related work to avoid planning on an unavailable node: cockroachdb#124986 Fixes: cockroachdb#100578 Epic: CRDB-31575 Release note: None
Contributor
Author
|
I've updated the PR. Please have a look. |
Contributor
Author
|
Stacking PR doesn't work well with GitHub (I had to update two PRs every time I update the commit in this PR). I'm closing this PR since changes in this PR are closely tied to #128283. |
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
5 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 is a preliminary commit toward improving the DistSQL to avoid planning on an instance that is draining (#128283). The idea here is to set this column to true as soon as the draining request is received by the server and use that through instance reader in the planner at gateway.
Adding this column requires writing a migration for the upgrade. And since this new column is nullable we don't need to backfill and we are good with just swapping the descriptors.
Informs: #100578
Release note: None