Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

loqrecovery: when processing data, validate that it belongs to the right cluster #74135

Closed
aliher1911 opened this issue Dec 21, 2021 · 1 comment · Fixed by #95405
Closed

loqrecovery: when processing data, validate that it belongs to the right cluster #74135

aliher1911 opened this issue Dec 21, 2021 · 1 comment · Fixed by #95405
Assignees
@aliher1911
Labels
A-kv-replication Relating to Raft, consensus, and coordination. C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)

Comments

@aliher1911
Copy link
Contributor

aliher1911 commented Dec 21, 2021 •
edited by exalate-issue-sync bot
Loading

If you create a plan of one cluster and apply it to another you may get unexpected breakages or tool will say there's nothing to apply on the node.

It likely not very common case, but having additional safety checks is beneficial because it would prevent mix up of recovery plans and providing or mounting incorrect data directories for recovery.

Follow up to: #71996

Jira issue: CRDB-11935

Epic CRDB-14205
@aliher1911 aliher1911 added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) A-kv-replication Relating to Raft, consensus, and coordination. T-kv-replication labels Dec 21, 2021
@aliher1911 aliher1911 self-assigned this Dec 21, 2021
@aliher1911 aliher1911 removed their assignment Mar 2, 2022
@aliher1911 aliher1911 changed the title loss of quorum recovery tools: when processing data, validate that it belongs to the right cluster loss of quorum recovery: when processing data, validate that it belongs to the right cluster Mar 2, 2022
@exalate-issue-sync exalate-issue-sync bot changed the title loss of quorum recovery: when processing data, validate that it belongs to the right cluster loqrecovery: when processing data, validate that it belongs to the right cluster May 31, 2022
@aliher1911 aliher1911 mentioned this issue Jan 17, 2023
Merged
craig bot pushed a commit that referenced this issue Jan 24, 2023
@aliher1911 @mgartner
Merge #95405 #95545 #95599
c8d92f3 
95405: loqrecovery,admin,cli: check staged plans, stage recovery plan on cluster r=erikgrinaker a=aliher1911

loqrecovery,admin,cli: stage recovery plan on cluster

This commit adds loss of quorum recovery plan staging on nodes.
RecoveryStagePlan admin call is distributing recovery plan to
relevant nodes of the cluster. To do so, it first verifies that
cluster state is unchanged from the state where plan was created
and there are no previously staged plans.
Then it distributes plan to all cluster nodes using fan-out mechanism.
Each node in turn markes dead nodes as decommissioned and if there
are planned changes for the node it saves plan in the local store.
Admin call is backed by debug recover apply-plan command when using
--host flag to work in half-online mode.

Release note: None

----

loqrecovery,admin: implement endpoint to check staged plans

This commit adds loss of quorum recovery verify call to admin
interface. Call allows querying loss of quorum recovery status
from all nodes of the cluster. It provides info about loss of
quorum recovery plans staged on each node.

Release note: None

----

State checkpoint is included in the PR as it only provide partial functionality of state needed for stage phase to work.

Fixes #93044
Touches #74135
Touches #93043

When doing staging, cli would present following reports in happy case:
```
$ cockroach debug recover apply-plan  --host=127.0.0.1:26257 --insecure=true recover-plan.json
Proposed changes in plan 66200d2c-e0e1-4af4-b890-ef5bb6e9ccc4:
  range r93:/Table/106/1/"boston"/"333333D\x00\x80\x00\x00\x00\x00\x00\x00\n" updating replica 2 to 16.
  range r92:/Table/106/1/"los angeles"/"\x99\x99\x99\x99\x99\x99H\x00\x80\x00\x00\x00\x00\x00\x00\x1e" updating replica 2 to 16.
  range r91:/Table/106/1/"seattle"/"ffffffH\x00\x80\x00\x00\x00\x00\x00\x00\x14" updating replica 2 to 16.
  range r115:/Table/106/1/"washington dc"/"L\xcc\xcc\xcc\xcc\xccL\x00\x80\x00\x00\x00\x00\x00\x00\x0f" updating replica 2 to 15.
  range r80:/Table/107 updating replica 1 to 15.
  range r96:/Table/107/1/"san francisco"/"\x88\x88\x88\x88\x88\x88H\x00\x80\x00\x00\x00\x00\x00\x00\b" updating replica 1 to 15.
  range r102:/Table/107/1/"seattle"/"UUUUUUD\x00\x80\x00\x00\x00\x00\x00\x00\x05" updating replica 4 to 16.
  range r89:/Table/107/2 updating replica 1 to 15.
  range r126:/Table/108/1/"amsterdam"/"\xc5\x1e\xb8Q\xeb\x85@\x00\x80\x00\x00\x00\x00\x00\x01\x81" updating replica 2 to 16.
  range r104:/Table/108/1/"los angeles"/"\xa8\xf5\u008f\\(H\x00\x80\x00\x00\x00\x00\x00\x01J" updating replica 3 to 16.
  range r119:/Table/108/1/"san francisco"/"\x8c\xcc\xcc\xcc\xcc\xcc@\x00\x80\x00\x00\x00\x00\x00\x01\x13" updating replica 6 to 18.
  range r117:/Table/108/1/"seattle"/"p\xa3\xd7\n=pD\x00\x80\x00\x00\x00\x00\x00\x00\xdc" updating replica 4 to 17.
  range r155:/Table/108/1/"washington dc"/"Tz\xe1G\xae\x14L\x00\x80\x00\x00\x00\x00\x00\x00\xa5" updating replica 3 to 15.
  range r82:/Table/108/3 updating replica 1 to 15.

Nodes n4, n5 will be marked as decommissioned.


Proceed with staging plan [y/N] y

Plan staged. To complete recovery restart nodes n1, n2, n3.

To verify recovery status invoke

'cockroach debug recover verify  --host=127.0.0.1:26257 --insecure=true recover-plan.json'

```

And allow overwriting of currently stages plans if need be:

```
$ cockroach debug recover apply-plan  --host=127.0.0.1:26257 --insecure=true recover-plan-2.json
Proposed changes in plan 576f3d2e-518c-4dbc-9af4-b416629bbf1a:
  range r93:/Table/106/1/"boston"/"333333D\x00\x80\x00\x00\x00\x00\x00\x00\n" updating replica 2 to 16.
  range r92:/Table/106/1/"los angeles"/"\x99\x99\x99\x99\x99\x99H\x00\x80\x00\x00\x00\x00\x00\x00\x1e" updating replica 2 to 16.
  range r91:/Table/106/1/"seattle"/"ffffffH\x00\x80\x00\x00\x00\x00\x00\x00\x14" updating replica 2 to 16.
  range r115:/Table/106/1/"washington dc"/"L\xcc\xcc\xcc\xcc\xccL\x00\x80\x00\x00\x00\x00\x00\x00\x0f" updating replica 2 to 15.
  range r80:/Table/107 updating replica 1 to 15.
  range r96:/Table/107/1/"san francisco"/"\x88\x88\x88\x88\x88\x88H\x00\x80\x00\x00\x00\x00\x00\x00\b" updating replica 1 to 15.
  range r102:/Table/107/1/"seattle"/"UUUUUUD\x00\x80\x00\x00\x00\x00\x00\x00\x05" updating replica 4 to 16.
  range r89:/Table/107/2 updating replica 1 to 15.
  range r126:/Table/108/1/"amsterdam"/"\xc5\x1e\xb8Q\xeb\x85@\x00\x80\x00\x00\x00\x00\x00\x01\x81" updating replica 2 to 16.
  range r104:/Table/108/1/"los angeles"/"\xa8\xf5\u008f\\(H\x00\x80\x00\x00\x00\x00\x00\x01J" updating replica 3 to 16.
  range r119:/Table/108/1/"san francisco"/"\x8c\xcc\xcc\xcc\xcc\xcc@\x00\x80\x00\x00\x00\x00\x00\x01\x13" updating replica 6 to 18.
  range r117:/Table/108/1/"seattle"/"p\xa3\xd7\n=pD\x00\x80\x00\x00\x00\x00\x00\x00\xdc" updating replica 4 to 17.
  range r155:/Table/108/1/"washington dc"/"Tz\xe1G\xae\x14L\x00\x80\x00\x00\x00\x00\x00\x00\xa5" updating replica 3 to 15.
  range r82:/Table/108/3 updating replica 1 to 15.

Nodes n4, n5 will be marked as decommissioned.

Conflicting staged plans will be replaced:
  plan 66200d2c-e0e1-4af4-b890-ef5bb6e9ccc4 is staged on node n1.
  plan 66200d2c-e0e1-4af4-b890-ef5bb6e9ccc4 is staged on node n3.
  plan 66200d2c-e0e1-4af4-b890-ef5bb6e9ccc4 is staged on node n2.


Proceed with staging plan [y/N] y

Plan staged. To complete recovery restart nodes n1, n2, n3.

To verify recovery status invoke

'cockroach debug recover verify  --host=127.0.0.1:26257 --insecure=true recover-plan-2.json'

```


95545: sql: cache result of nullary UDFs and uncorrelated subqueries r=mgartner a=mgartner

#### sql: add tests showing duplicate execution of uncorrelated subqueries

Release note: None

#### logictest: fix bug in let variable replacement

This commit fixes a bug in the implementation of the `let` command that
incorrectly matched and replaced parts of queries with dollar-quotes,
like `CREATE FUNCTION ... AS $$SELECT 1$$`.

Release note: None

#### sql: cache result of nullary UDFs and uncorrelated subqueries

Nullary (zero-argument), non-volatile UDFs and lazily-evaluated,
uncorrelated subqueries now cache the result of their first invocation.
On subsequent invocations, the cached result is returned rather than
evaluating the full UDF or subquery. This eliminates duplicate work, and
makes the behavior of lazily-evaluated, uncorrelated subqueries match
that of eagerly-evaluated, uncorrelated subqueries which are evaluated
once before the main query is evaluated.

Epic: CRDB-20370

Release note: None


95599: opt: do not hoist correlated, non-leakproof subqueries in COALESCE r=mgartner a=mgartner

Previously, correlated, non-leakproof subqueries within a `COALESCE`
expression would be hoisted into an apply-join. This caused those
arguments to be evaluated when they shouldn't have been, i.e., when
previous arguments always evaluated to non-NULL values. This could cause
errors to propagate from arguments that should have never been
evaluated, and it was inconsistent with Postgres:

> Like a CASE expression, COALESCE only evaluates the arguments that are
needed to determine the result; that is, arguments to the right of the
first non-null argument are not evaluated[^1].

Now, non-leakproof subqueries are only hoisted from within a `COALESCE`
expression if they are not in the first argument. It is safe to hoist a
non-leakproof subquery in the first argument because it is always
evaluated.

Fixes #95560

Release note (bug fix): A bug has been fixed that could arguments of a
`COALESCE` to be evaluated when previous arguments always evaluated to
non-NULL values. This bug could cause query errors to originate from
arguments of a `COALESCE` that should have never been evaluated.

[^1]: https://www.postgresql.org/docs/15/functions-conditional.html#FUNCTIONS-COALESCE-NVL-IFNULL


Co-authored-by: Oleg Afanasyev <[email protected]>
Co-authored-by: Marcus Gartner <[email protected]>
This was referenced Jan 31, 2023
Merged
Merged
@aliher1911
Copy link
Contributor Author

Fixed by mentioned PR's.
First check is done in replica reading to ensure that all info files belong to the same cluster.
Second check is done in admin server when handling staging request to ensure plan belongs to the current cluster.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@aliher1911 aliher1911

Labels
A-kv-replication Relating to Raft, consensus, and coordination. C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

loqrecovery,admin,cli: check staged plans, stage recovery plan on cluster aliher1911/cockroach
1 participant
@aliher1911