Navigation Menu

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.

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

importccl: support IMPORT in mixed-version cluster #57382

Merged
merged 3 commits into from Dec 4, 2020

Conversation

pbardea
Copy link
Contributor

@pbardea pbardea commented Dec 2, 2020

This commit adds support for running IMPORT in a mixed-version cluster.
Note, that it does not add support for running an IMPORT during the
upgrade of a node however. The IMPORT job would need to be restarted in
that case.

Release note (sql change): Add support for running IMPORT in a
mixed-version cluster.

@cockroach-teamcity
Copy link
Member

This change is Reviewable

pkg/sql/distsql_plan_bulk.go Outdated Show resolved Hide resolved
@pbardea pbardea changed the title [wip] importccl: support IMPORT in mixed-version cluster importccl: support IMPORT in mixed-version cluster Dec 3, 2020
@pbardea
Copy link
Contributor Author

pbardea commented Dec 3, 2020

I believe that this also fixes #57503.

Will add tests.

@pbardea pbardea added backport-19.2.x Flags PRs that need to be backported to 19.2. backport-20.1.x backport-20.2.x Flags PRs that need to be backported to 20.2 labels Dec 3, 2020
@pbardea pbardea force-pushed the mixed-version-import branch 3 times, most recently from 99a81cc to 5a15125 Compare December 4, 2020 15:32
This commit adds support for running IMPORT in a mixed-version cluster.
Note, that it does not add support for running an IMPORT during the
upgrade of a node however. The IMPORT job would need to be restarted in
that case.

Release note (sql change): Add support for running IMPORT in a
mixed-version cluster.
Before supporting mixed-versions runs of IMPORT this test would fail
with:
```
importing fixture: importing table history: pq: version mismatch in flow request: 42; this node accepts 35 through 36
```

It now succeeds. Note that this test exercises that IMPORT can run in a
mixed-version cluster, but not during an upgrade of nodes in the
cluster.

Release note: None
This commit adds a roachtest that runs an IMPORT on a cluster with a
decommissioned node.

Release note: None
@pbardea pbardea requested a review from dt December 4, 2020 16:38
@pbardea
Copy link
Contributor Author

pbardea commented Dec 4, 2020

Updated with some roachtests that exercise running import in mixed-version clusters and clusters with a decommissioned node.
RFAL

@pbardea pbardea marked this pull request as ready for review December 4, 2020 16:39
@pbardea pbardea requested a review from a team December 4, 2020 16:39
@pbardea
Copy link
Contributor Author

pbardea commented Dec 4, 2020

TFTR!
bors r=dt

@craig
Copy link
Contributor

craig bot commented Dec 4, 2020

Build succeeded:

@craig craig bot merged commit ce3f9b2 into cockroachdb:master Dec 4, 2020
@pbardea
Copy link
Contributor Author

pbardea commented Dec 9, 2020

It doesn't look like this needs backporting to versions before 20.2. It looks like https://github.com/cockroachdb/cockroach/blob/release-20.1/pkg/sql/distsql_physical_planner.go#L1064 never populates the NodeAddresses map for unhealthy nodes so https://github.com/cockroachdb/cockroach/blob/release-20.1/pkg/sql/distsql_plan_csv.go#L123 just did the right thing before

@rafiss rafiss added this to the 20.2 milestone Apr 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-20.2.x Flags PRs that need to be backported to 20.2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants