Skip to content

Commit

Permalink
builtins: add duration column to check_consistency
Browse files Browse the repository at this point in the history
Noticed that some checks were taking a very long time but it's hard to
tell which ones unless setting up and watching streaming output.

Add a duration column. This can also help us reason about the general
speed of the consistency checker; it does do very expensive hashing
at the moment (sha256) when something much cheaper and weaker would
do.

Release justification: debug improvement for an internal builtin
Release note: None
  • Loading branch information
tbg committed Sep 9, 2022
1 parent 967b163 commit b52735c
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 4 deletions.
2 changes: 1 addition & 1 deletion docs/generated/sql/functions.md
Original file line number Diff line number Diff line change
Expand Up @@ -3018,7 +3018,7 @@ may increase either contention or retry errors, or both.</p>
</span></td><td>Immutable</td></tr>
<tr><td><a name="crdb_internal.assignment_cast"></a><code>crdb_internal.assignment_cast(val: anyelement, type: anyelement) &rarr; anyelement</code></td><td><span class="funcdesc"><p>This function is used internally to perform assignment casts during mutations.</p>
</span></td><td>Stable</td></tr>
<tr><td><a name="crdb_internal.check_consistency"></a><code>crdb_internal.check_consistency(stats_only: <a href="bool.html">bool</a>, start_key: <a href="bytes.html">bytes</a>, end_key: <a href="bytes.html">bytes</a>) &rarr; tuple{int AS range_id, bytes AS start_key, string AS start_key_pretty, string AS status, string AS detail}</code></td><td><span class="funcdesc"><p>Runs a consistency check on ranges touching the specified key range. an empty start or end key is treated as the minimum and maximum possible, respectively. stats_only should only be set to false when targeting a small number of ranges to avoid overloading the cluster. Each returned row contains the range ID, the status (a roachpb.CheckConsistencyResponse_Status), and verbose detail.</p>
<tr><td><a name="crdb_internal.check_consistency"></a><code>crdb_internal.check_consistency(stats_only: <a href="bool.html">bool</a>, start_key: <a href="bytes.html">bytes</a>, end_key: <a href="bytes.html">bytes</a>) &rarr; tuple{int AS range_id, bytes AS start_key, string AS start_key_pretty, string AS status, string AS detail, interval AS duration}</code></td><td><span class="funcdesc"><p>Runs a consistency check on ranges touching the specified key range. an empty start or end key is treated as the minimum and maximum possible, respectively. stats_only should only be set to false when targeting a small number of ranges to avoid overloading the cluster. Each returned row contains the range ID, the status (a roachpb.CheckConsistencyResponse_Status), and verbose detail.</p>
<p>Example usage:
SELECT * FROM crdb_internal.check_consistency(true, ‘\x02’, ‘\x04’)</p>
</span></td><td>Volatile</td></tr>
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/sem/builtins/fixed_oids.go
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,7 @@ var builtinOidsBySignature = map[string]oid.Oid{
`crdb_internal.active_version() -> jsonb`: 1296,
`crdb_internal.approximate_timestamp(timestamp: decimal) -> timestamp`: 1298,
`crdb_internal.assignment_cast(val: anyelement, type: anyelement) -> anyelement`: 1341,
`crdb_internal.check_consistency(stats_only: bool, start_key: bytes, end_key: bytes) -> tuple{int AS range_id, bytes AS start_key, string AS start_key_pretty, string AS status, string AS detail}`: 347,
`crdb_internal.check_consistency(stats_only: bool, start_key: bytes, end_key: bytes) -> tuple{int AS range_id, bytes AS start_key, string AS start_key_pretty, string AS status, string AS detail, interval AS duration}`: 347,
`crdb_internal.check_password_hash_format(password: bytes) -> string`: 1376,
`crdb_internal.cluster_id() -> uuid`: 1299,
`crdb_internal.cluster_name() -> string`: 1301,
Expand Down
17 changes: 15 additions & 2 deletions pkg/sql/sem/builtins/generator_builtins.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/util/errorutil"
"github.com/cockroachdb/cockroach/pkg/util/json"
"github.com/cockroachdb/cockroach/pkg/util/mon"
"github.com/cockroachdb/cockroach/pkg/util/timeutil"
"github.com/cockroachdb/cockroach/pkg/util/tracing"
"github.com/cockroachdb/cockroach/pkg/util/tracing/tracingpb"
"github.com/cockroachdb/errors"
Expand Down Expand Up @@ -1837,6 +1838,7 @@ type checkConsistencyGenerator struct {
// row and moves it to curRow. When empty, consumes from 'descs' to produce
// more rows.
remainingRows []roachpb.CheckConsistencyResponse_Result
curDuration time.Duration
curRow roachpb.CheckConsistencyResponse_Result
}

Expand Down Expand Up @@ -1889,8 +1891,8 @@ func makeCheckConsistencyGenerator(
}

var checkConsistencyGeneratorType = types.MakeLabeledTuple(
[]*types.T{types.Int, types.Bytes, types.String, types.String, types.String},
[]string{"range_id", "start_key", "start_key_pretty", "status", "detail"},
[]*types.T{types.Int, types.Bytes, types.String, types.String, types.String, types.Interval},
[]string{"range_id", "start_key", "start_key_pretty", "status", "detail", "duration"},
)

// ResolvedType is part of the tree.ValueGenerator interface.
Expand Down Expand Up @@ -1929,6 +1931,7 @@ func (c *checkConsistencyGenerator) Start(ctx context.Context, _ *kv.Txn) error

// Next is part of the tree.ValueGenerator interface.
func (c *checkConsistencyGenerator) Next(ctx context.Context) (bool, error) {
tBegin := timeutil.Now()
if len(c.remainingRows) == 0 {
if len(c.descs) == 0 {
return false, nil
Expand All @@ -1954,22 +1957,32 @@ func (c *checkConsistencyGenerator) Next(ctx context.Context) (bool, error) {
},
}
} else {
// NB: this could have more than one entry, if a range split in the
// meantime.
c.remainingRows = resp.Result
}
}

c.curDuration = timeutil.Since(tBegin)
c.curRow = c.remainingRows[0]
c.remainingRows = c.remainingRows[1:]
return true, nil
}

// Values is part of the tree.ValueGenerator interface.
func (c *checkConsistencyGenerator) Values() (tree.Datums, error) {
intervalMeta := types.IntervalTypeMetadata{
DurationField: types.IntervalDurationField{
DurationType: types.IntervalDurationType_MILLISECOND,
},
}
return tree.Datums{
tree.NewDInt(tree.DInt(c.curRow.RangeID)),
tree.NewDBytes(tree.DBytes(c.curRow.StartKey)),
tree.NewDString(roachpb.Key(c.curRow.StartKey).String()),
tree.NewDString(c.curRow.Status.String()),
tree.NewDString(c.curRow.Detail),
tree.NewDInterval(duration.MakeDuration(c.curDuration.Nanoseconds(), 0 /* days */, 0 /* months */), intervalMeta),
}, nil
}

Expand Down

0 comments on commit b52735c

Please sign in to comment.