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: add 19.2 version gate check #41684

Merged
merged 1 commit into from
Oct 21, 2019

Conversation

miretskiy
Copy link
Contributor

Ensure the cluster is fully upgraded when running import.

Release note: ensure cluster fully upgraded when running import.

@miretskiy miretskiy requested a review from dt October 17, 2019 15:49
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@@ -126,6 +126,10 @@ func importJobDescription(
func importPlanHook(
_ context.Context, stmt tree.Statement, p sql.PlanHookState,
) (sql.PlanHookRowFn, sqlbase.ResultColumns, []sql.PlanNode, bool, error) {
if !p.ExecCfg().Settings.Version.IsActive(cluster.VersionPartitionedBackup) {
return nil, nil, nil, false, errors.Errorf("INSERT requires a cluster fully upgraded to version >= 19.2")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/INSERT/IMPORT/

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This whole check needs to be below the importStmt, ok := below or it will just error out every sql statement during an upgrade.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@@ -126,6 +126,10 @@ func importJobDescription(
func importPlanHook(
_ context.Context, stmt tree.Statement, p sql.PlanHookState,
) (sql.PlanHookRowFn, sqlbase.ResultColumns, []sql.PlanNode, bool, error) {
if !p.ExecCfg().Settings.Version.IsActive(cluster.VersionPartitionedBackup) {
return nil, nil, nil, false, errors.Errorf("INSERT requires a cluster fully upgraded to version >= 19.2")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This whole check needs to be below the importStmt, ok := below or it will just error out every sql statement during an upgrade.

Ensure the cluster is fully upgraded when running import.

Release note: ensure cluster fully upgraded when running import.
@miretskiy
Copy link
Contributor Author

Comments addressed.

@miretskiy
Copy link
Contributor Author

bors+

@dt
Copy link
Member

dt commented Oct 18, 2019

Let’s backport this in time for the mid-Monday RC as well.

@miretskiy
Copy link
Contributor Author

miretskiy commented Oct 18, 2019 via email

@miretskiy
Copy link
Contributor Author

bors+

@miretskiy
Copy link
Contributor Author

bors r+

craig bot pushed a commit that referenced this pull request Oct 21, 2019
41684: importccl: add 19.2 version gate check r=miretskiy a=miretskiy

Ensure the cluster is fully upgraded when running import.

Release note: ensure cluster fully upgraded when running import.

41711: storage/report: don't deserialize zone configs over and over r=andreimatei a=andreimatei

This patch is expected to speedup reports generation considerably by not
unmarshalling zone config protos for every range. Instead, the
report-geneating visitors now keep state around the zone that a visited
range is in and reuse that state if they're told that the following
range is in the same zone. The range iteration infrastructure was
enhanced to figure out when zones change from range to range and tell
that to the visitors.

Without this patch, generating the reports was pinning a core for
minutes for a cluster with partitioning and 200k ranges. The profiles
showed that it's all zone config unmarshalling.

Fixes #41609

Release note (performance improvement): The performance of generating
the system.replication_* reports was greatly improved for large
clusters.

41761: storage: write to local storage before updating liveness r=bdarnell a=irfansharif

Previously a disk stall could allow a node to continue heartbeating its
liveness record and prevent other nodes from taking over its leases,
despite being completely unresponsive.

This was first addressed in #24591 (+ #33122). This was undone in #32978
(which introduced a stricter version of a similar check). #32978 was
later disabled by default in #36484, leaving us without the protections
first introduced in #24591. This PR re-adds the logic from #24591.

Part of #41683.

Release note: None.

Co-authored-by: Yevgeniy Miretskiy <[email protected]>
Co-authored-by: Andrei Matei <[email protected]>
Co-authored-by: irfan sharif <[email protected]>
@craig
Copy link
Contributor

craig bot commented Oct 21, 2019

Build succeeded

@craig craig bot merged commit cc6cae1 into cockroachdb:master Oct 21, 2019
@miretskiy miretskiy deleted the version_check branch October 21, 2019 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants