crosscluster/logical: commit planner txn early#134997
Merged
craig[bot] merged 2 commits intocockroachdb:masterfrom Nov 13, 2024
Merged
crosscluster/logical: commit planner txn early#134997craig[bot] merged 2 commits intocockroachdb:masterfrom
craig[bot] merged 2 commits intocockroachdb:masterfrom
Conversation
|
It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR? 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
Member
|
This change is |
a5c58f6 to
df6f002
Compare
dt
reviewed
Nov 12, 2024
Collaborator
Author
|
unrelated flake. |
df6f002 to
0c53dad
Compare
During large scale testing, we have seen the CreateForTables remote rpc take several seconds or even minutes in LDR planning, and was previously within scope of the planner txn, which could in turn cause txn refresh errors. This patch commits the planner txn earlier in the job hook, before the CreateForTables call, to prevent the planner txn from retrying. Now, the destination side schema validation, locking, and job writing occur in a subsequent txn. Epic: none Release note: none
Release the planner txn leases, so the subsequent txn which writes the job record and locks the replicating tables is guaranteed to not wait for the planner txn leases to expire. Epic: none Release note: none
0c53dad to
0488e61
Compare
dt
approved these changes
Nov 13, 2024
Collaborator
Author
|
TFTR! bors r=dt |
Contributor
This was referenced Nov 13, 2024
msbutler
added a commit
to msbutler/cockroach
that referenced
this pull request
Nov 22, 2024
PR cockroachdb#134997 introduced a bug where CREATE LOGICAL REPLICATION could return early with a bogus job id if the txn which writes the job record retried. This patch prevents this. Note: I plan to open a follow up PR that moves the ldr planning txn into its own helper func. Fixes cockroachdb#135176 Fixes cockroachdb#135490 Release note: none
msbutler
added a commit
to msbutler/cockroach
that referenced
this pull request
Nov 22, 2024
PR cockroachdb#134997 introduced a bug where CREATE LOGICAL REPLICATION could return early with a bogus job id if the txn which writes the job record retried. This patch prevents this. Note: I plan to open a follow up PR that moves the ldr planning txn into its own helper func. Fixes cockroachdb#135176 Fixes cockroachdb#135490 Release note: none
craig bot
pushed a commit
that referenced
this pull request
Nov 22, 2024
135994: crosscluster/logical: only return jobID if planner txn succeeded r=kev-cao a=msbutler PR #134997 introduced a bug where CREATE LOGICAL REPLICATION could return early with a bogus job id if the txn which writes the job record retried. This patch prevents this. Note: I plan to open a follow up PR that moves the ldr planning txn into its own helper func. Fixes #135176 Fixes #135490 Release note: none Co-authored-by: Michael Butler <butler@cockroachlabs.com>
blathers-crl bot
pushed a commit
that referenced
this pull request
Nov 22, 2024
PR #134997 introduced a bug where CREATE LOGICAL REPLICATION could return early with a bogus job id if the txn which writes the job record retried. This patch prevents this. Note: I plan to open a follow up PR that moves the ldr planning txn into its own helper func. Fixes #135176 Fixes #135490 Release note: none
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
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.
During large scale testing, we have seen the CreateForTables remote rpc take
several seconds or even minutes in LDR planning, and was previously within
scope of the planner txn, which could in turn cause txn refresh errors. This
patch commits the planner txn earlier in the job hook, before the
CreateForTables call, to prevent the planner txn from retrying. Now, the
destination side schema validation, locking, and job writing occur in a
subsequent txn.
Epic: none
Release note: none