Skip to content

Commit

Permalink
roachtest: fix query used to get job status in backup/mixed-version
Browse files Browse the repository at this point in the history
At the moment, we only query job status in mixed-version state (so we
should always use `system.jobs`). However, the code added in this
commit should continue to work once we start developing 23.2, as we're
checking that the cluster version is at least 23.1 before using
`crdb_internal.system_jobs`.

Epic: none

Release note: None
  • Loading branch information
renatolabs committed Mar 28, 2023
1 parent 3fc51ba commit 2f9d252
Showing 1 changed file with 36 additions and 2 deletions.
38 changes: 36 additions & 2 deletions pkg/cmd/roachtest/tests/mixed_version_backup.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/util/protoutil"
"github.com/cockroachdb/cockroach/pkg/util/retry"
"github.com/cockroachdb/cockroach/pkg/util/timeutil"
"github.com/cockroachdb/cockroach/pkg/util/version"
"github.com/cockroachdb/errors"
"golang.org/x/sync/errgroup"
)
Expand All @@ -54,6 +55,14 @@ var (
Multiplier: 1.5,
MaxRetries: 50,
}

v231 = func() *version.Version {
v, err := version.Parse("v23.1.0")
if err != nil {
panic(fmt.Sprintf("failure parsing version: %v", err))
}
return v
}()
)

// sanitizeVersionForBackup takes the string representation of a
Expand All @@ -63,6 +72,25 @@ func sanitizeVersionForBackup(v string) string {
return invalidVersionRE.ReplaceAllString(clusterupgrade.VersionMsg(v), "")
}

// hasInternalSystemJobs returns true if the cluster is expected to
// have the `crdb_internal.system_jobs` vtable in the mixed-version
// context passed. If so, it should be used instead of `system.jobs`
// when querying job status.
func hasInternalSystemJobs(tc *mixedversion.Context) bool {
lowestVersion := tc.FromVersion // upgrades
if tc.FromVersion == clusterupgrade.MainVersion {
lowestVersion = tc.ToVersion // downgrades
}

// Add 'v' prefix expected by `version` package.
lowestVersion = "v" + lowestVersion
sv, err := version.Parse(lowestVersion)
if err != nil {
panic(fmt.Errorf("internal error: test context version (%s) expected to be parseable: %w", lowestVersion, err))
}
return sv.AtLeast(v231)
}

type (
// backupOption is an option passed to the `BACKUP` command (i.e.,
// `WITH ...` portion).
Expand Down Expand Up @@ -298,11 +326,17 @@ func (mvb *mixedVersionBackup) waitForJobSuccess(
var lastErr error
node, db := h.RandomDB(rng, mvb.roachNodes)
l.Printf("querying job status through node %d", node)

jobsQuery := "system.jobs WHERE id = $1"
if hasInternalSystemJobs(h.Context()) {
jobsQuery = fmt.Sprintf("(%s)", jobutils.InternalSystemJobsBaseQuery)
}
for r := retry.StartWithCtx(ctx, backupCompletionRetryOptions); r.Next(); {
var status string
var payloadBytes []byte
err := db.QueryRow(fmt.Sprintf(`SELECT status, payload FROM (%s)`,
jobutils.InternalSystemJobsBaseQuery), jobID).Scan(&status, &payloadBytes)
err := db.QueryRow(
fmt.Sprintf(`SELECT status, payload FROM %s`, jobsQuery), jobID,
).Scan(&status, &payloadBytes)
if err != nil {
lastErr = fmt.Errorf("error reading (status, payload) for job %d: %w", jobID, err)
l.Printf("%v", lastErr)
Expand Down

0 comments on commit 2f9d252

Please sign in to comment.