Skip to content

Commit e0f7fb4

Browse files
committed
roachtestutil: use DETACHED option for INSPECT jobs
Previously, CheckInspectDatabase used a statement timeout hack to background INSPECT jobs - it set a 5-second timeout and relied on the timeout error to leave jobs running. Now that INSPECT supports the DETACHED option, we use `INSPECT DATABASE <name> WITH OPTIONS DETACHED` to properly run jobs in the background. This provides a cleaner way to background the job. Informs #155676 Epic: CRDB-55075 Release note: none
1 parent e1c7374 commit e0f7fb4

File tree

1 file changed

+4
-34
lines changed

1 file changed

+4
-34
lines changed

pkg/cmd/roachtest/roachtestutil/validation_check.go

Lines changed: 4 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -206,17 +206,6 @@ func discoverUserDatabases(ctx context.Context, db *gosql.DB) ([]string, error)
206206
return databases, rows.Err()
207207
}
208208

209-
// isStatementTimeoutError returns true if the error is a statement timeout error.
210-
// Statement timeout errors are expected when launching INSPECT jobs since we only
211-
// want to start the job, not wait for it to complete.
212-
func isStatementTimeoutError(err error) bool {
213-
var pqErr *pq.Error
214-
if errors.As(err, &pqErr) {
215-
return pgcode.MakeCode(string(pqErr.Code)) == pgcode.QueryCanceled
216-
}
217-
return false
218-
}
219-
220209
// isFeatureNotSupportedError returns true if the error is a feature not supported error.
221210
// This can occur when the cluster version is not yet upgraded to support INSPECT.
222211
func isFeatureNotSupportedError(err error) bool {
@@ -229,8 +218,7 @@ func isFeatureNotSupportedError(err error) bool {
229218

230219
// launchInspectJobs launches INSPECT DATABASE commands in parallel for all
231220
// provided databases using task manager for concurrency control. Each INSPECT
232-
// command enables the inspect command for that connection. Statement timeout
233-
// errors are ignored as they indicate the job was successfully started.
221+
// command is launched with the DETACHED option to run in the background.
234222
// Feature not supported errors are returned to the caller, indicating the
235223
// cluster version does not support INSPECT.
236224
func launchInspectJobs(
@@ -242,7 +230,6 @@ func launchInspectJobs(
242230
return errors.Wrap(err, "failed to disable INSPECT admission control")
243231
}
244232

245-
statementTimeout := 5 * time.Second
246233
tm := task.NewManager(ctx, l)
247234
g := tm.NewErrorGroup()
248235

@@ -253,30 +240,13 @@ func launchInspectJobs(
253240

254241
statements := []string{
255242
"SET enable_inspect_command = true",
256-
fmt.Sprintf("SET statement_timeout = '%s'", statementTimeout.String()),
257-
fmt.Sprintf("INSPECT DATABASE %s", lexbase.EscapeSQLIdent(dbName)),
243+
fmt.Sprintf("INSPECT DATABASE %s WITH OPTIONS DETACHED", lexbase.EscapeSQLIdent(dbName)),
258244
}
259245

260-
var stmtErr error
261246
for _, stmt := range statements {
262247
if _, err := db.ExecContext(ctx, stmt); err != nil {
263-
stmtErr = err
264-
break
265-
}
266-
}
267-
268-
// Always reset statement timeout back to default.
269-
if _, err := db.ExecContext(ctx, "RESET statement_timeout"); err != nil {
270-
l.Printf("Warning: failed to reset statement timeout: %v", err)
271-
}
272-
273-
// Check for errors from the statements loop.
274-
if stmtErr != nil {
275-
// Statement timeout is expected - it means the job started but didn't complete
276-
// within the timeout. The job is still running in the background.
277-
if !isStatementTimeoutError(stmtErr) {
278-
l.Printf("INSPECT DATABASE %s failed to start: %v", dbName, stmtErr)
279-
return errors.Wrapf(stmtErr, "failed to start INSPECT DATABASE %s", dbName)
248+
l.Printf("INSPECT DATABASE %s failed to start: %v", dbName, err)
249+
return errors.Wrapf(err, "failed to start INSPECT DATABASE %s", dbName)
280250
}
281251
}
282252

0 commit comments

Comments
 (0)