- 
                Notifications
    You must be signed in to change notification settings 
- Fork 4k
workload/schemachange: add INSPECT operation to random schema workload #156295
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
base: master
Are you sure you want to change the base?
workload/schemachange: add INSPECT operation to random schema workload #156295
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few minor comments, nice work!
@fqazi reviewed 4 of 4 files at r1, all commit messages.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @shailendra-patel and @williamchoe3)
pkg/workload/schemachange/schemachange.go line 210 at r1 (raw file):
var totalErrors int for _, jobID := range jobIDs { query := fmt.Sprintf("SHOW INSPECT ERRORS FOR JOB %d WITH DETAILS", jobID)
Nit: Do we want to explicit select columns via SELECT <COLUMNS> FROM [SHOW ...]  for future proofing (if the columns ever change?)
pkg/workload/schemachange/schemachange.go line 350 at r1 (raw file):
shutdownErr := tracerProvider.Shutdown(ctx) s.schemaWorkloadResultAnnotator.logWorkloadStats(stdoutLog) return errors.CombineErrors(inspectErr, errors.CombineErrors(closeErr, shutdownErr))
Lets use errors.Join instead.
pkg/workload/schemachange/operation_generator.go line 3460 at r1 (raw file):
func (og *operationGenerator) inspect(ctx context.Context, tx pgx.Tx) (*opStmt, error) { stmt := makeOpStmt(OpStmtDML)
This workload runs on mixed version, is it safe to create the job in a mixed version setting? Think we may want a version gate.
fafc743    to
    f881d3f      
    Compare
  
    There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @fqazi, @shailendra-patel, and @williamchoe3)
pkg/workload/schemachange/operation_generator.go line 3460 at r1 (raw file):
Previously, fqazi (Faizan Qazi) wrote…
This workload runs on mixed version, is it safe to create the job in a mixed version setting? Think we may want a version gate.
Good call. I updated optype.go for this.
pkg/workload/schemachange/schemachange.go line 350 at r1 (raw file):
Previously, fqazi (Faizan Qazi) wrote…
Lets use errors.Join instead.
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fqazi reviewed 2 of 2 files at r2, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @shailendra-patel and @williamchoe3)
| Potential Bug(s) DetectedThe three-stage Claude Code analysis has identified potential bug(s) in this PR that may warrant investigation. Next Steps: Note: When viewing the workflow output, scroll to the bottom to find the Final Analysis Summary. After you review the findings, please tag the issue as follows: 
 | 
Adds a new inspect operation to the schema change workload, enabling random generation of INSPECT TABLE and INSPECT DATABASE statements. Features: - Support for TABLE/DB targets, AS OF SYSTEM TIME - Always runs in DETACHED mode so that it can be run inside a transaciton - Results checked post-run via SHOW INSPECT ERRORS Errors reported in JSON, consistent with existing workload logs Closes cockroachdb#155483 Epic: CRDB-55075 Release note: none
f881d3f    to
    dc72c30      
    Compare
  
    
Adds a new inspect operation to the schema change workload, enabling random generation of INSPECT TABLE and INSPECT DATABASE statements.
Features:
Errors reported in JSON, consistent with existing workload logs
Closes #155483
Epic: CRDB-55075
Release note: none