sql/importer: enable async row count validation by default#163543
sql/importer: enable async row count validation by default#163543trunk-io[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
|
😎 Merged successfully - details. |
|
This change is |
c3f8e9b to
aafb676
Compare
spilchen
left a comment
There was a problem hiding this comment.
nice improvement with the new inspect job column
@spilchen made 3 comments.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on dt, mw5h, rafiss, and ZhouXing19).
pkg/sql/pgwire/command_result.go line 749 at r1 (raw file):
// Get the column index for job id based on the result header defined in // jobs.ImportJobExecutionResultHeader and jobs.DetachedJobExecutionResultHeader. func init() {
shouldn't this function still guard against drift in jobs.BulkJobExecutionResultHeader? I think it needs to validate between 3 structs now: jobs.ImportJobExecutionResultHeader, jobs.DetachedJobExecutionResultHeader and jobs.BulkJobExecutionResultHeader.
pkg/sql/importer/import_into_test.go line 521 at r1 (raw file):
// setUnsafeClusterSetting sets a cluster setting that is marked as unsafe. It // extracts the interlock key from the initial error and retries with the key. func setUnsafeClusterSetting(t *testing.T, db *gosql.DB, stmt string) {
there is some handling of the unsafe cluster setting in pkg/cmd/roachtest/tests/inspect_throughput.go (see disableRowCountValidation). Can we remove that handling too?
Row count validation after IMPORT is now enabled by default in async mode. After an IMPORT completes, a background INSPECT job validates that the imported row count matches expectations. To make the background INSPECT job discoverable, this commit adds an `inspect_job_id` column to the IMPORT statement result output. When an INSPECT job is triggered, the column contains that job's ID; otherwise it is NULL. The `bulkio.import.row_count_validation.unsafe.mode` cluster setting is made public under the name `bulkio.import.row_count_validation.mode`, and the metamorphic test default no longer includes "off" since validation is now considered stable. Resolves: cockroachdb#161279 Resolves: cockroachdb#155472 Release note (sql change): Row count validation after `IMPORT` is now enabled by default in async mode. After an `IMPORT` completes, a background `INSPECT` job validates that the imported row count matches expectations. The `IMPORT` result now includes an `inspect_job_id` column so the `INSPECT` job can be viewed separately. The `bulkio.import.row_count_validation.mode` cluster setting controls this behavior, with valid values of `off`, `async` (default), and `sync`.
aafb676 to
3633494
Compare
rafiss
left a comment
There was a problem hiding this comment.
TFTR!
/trunk merge
@rafiss made 3 comments.
Reviewable status:
pkg/sql/pgwire/command_result.go line 749 at r1 (raw file):
Previously, spilchen wrote…
shouldn't this function still guard against drift in jobs.BulkJobExecutionResultHeader? I think it needs to validate between 3 structs now: jobs.ImportJobExecutionResultHeader, jobs.DetachedJobExecutionResultHeader and jobs.BulkJobExecutionResultHeader.
done, nice catch
pkg/sql/importer/import_into_test.go line 521 at r1 (raw file):
Previously, spilchen wrote…
there is some handling of the unsafe cluster setting in pkg/cmd/roachtest/tests/inspect_throughput.go (see
disableRowCountValidation). Can we remove that handling too?
done
|
/trunk merge |
Row count validation after IMPORT is now enabled by default in async mode. After an IMPORT completes, a background INSPECT job validates that the imported row count matches expectations.
To make the background INSPECT job discoverable, this commit adds an
inspect_job_idcolumn to the IMPORT statement result output. When an INSPECT job is triggered, the column contains that job's ID; otherwise it is NULL.The
bulkio.import.row_count_validation.unsafe.modecluster setting is made public under the namebulkio.import.row_count_validation.mode, and the metamorphic test default no longer includes "off" since validation is now considered stable.Resolves: #161279
Resolves: #155472
Release note (sql change): Row count validation after
IMPORTis now enabled by default in async mode. After anIMPORTcompletes, a backgroundINSPECTjob validates that the imported row count matches expectations. TheIMPORTresult now includes aninspect_job_idcolumn so theINSPECTjob can be viewed separately. Thebulkio.import.row_count_validation.modecluster setting controls this behavior, with valid values ofoff,async(default), andsync.