Skip to content
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.

Sign up for GitHub

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

Jump to bottom

streamingccl: NPE in crdb_internal.stream_ingestion_stats_json #86040

Closed
stevendanna opened this issue Aug 12, 2022 · 0 comments · Fixed by #86123
Closed

streamingccl: NPE in crdb_internal.stream_ingestion_stats_json #86040

stevendanna opened this issue Aug 12, 2022 · 0 comments · Fixed by #86123
Assignees
@stevendanna
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.
Milestone
22.2

Comments

@stevendanna
Copy link
Collaborator

stevendanna commented Aug 12, 2022 •
edited by cockroach-jira-scripts
Loading

Describe the problem

If you run crdb_internal.stream_ingestion_stats_json on the wrong job ID, we get a NPE that crashes the server:

E220812 13:47:11.190613 1525232 9@sql/conn_executor.go:1031 ⋮ [n1,client=127.0.0.1:60474,user=root] 590  a SQL panic has occurred while executing the following statement:
E220812 13:47:11.190613 1525232 9@sql/conn_executor.go:1031 ⋮ [n1,client=127.0.0.1:60474,user=root] 590 +‹SELECT ((crdb_internal.stream_ingestion_stats_json(787200830746099713)->'replication_lag_info')
->'min_ingested_timestamp')->>'wall_time'›
E220812 13:47:11.417605 1525232 1@util/log/logcrash/crash_reporting.go:174 ⋮ [n1,client=127.0.0.1:60474,user=root] 591  a panic has occurred!
E220812 13:47:11.417605 1525232 1@util/log/logcrash/crash_reporting.go:174 ⋮ [n1,client=127.0.0.1:60474,user=root] 591 +interface conversion: jobspb.Details is jobspb.StreamReplicationDetails, not jobs
pb.StreamIngestionDetails
E220812 13:47:11.417605 1525232 1@util/log/logcrash/crash_reporting.go:174 ⋮ [n1,client=127.0.0.1:60474,user=root] 591 +(1) attached stack trace
E220812 13:47:11.417605 1525232 1@util/log/logcrash/crash_reporting.go:174 ⋮ [n1,client=127.0.0.1:60474,user=root] 591 +  -- stack trace:
E220812 13:47:11.417605 1525232 1@util/log/logcrash/crash_reporting.go:174 ⋮ [n1,client=127.0.0.1:60474,user=root] 591 +  | github.com/cockroachdb/cockroach/pkg/sql.(*Server).ServeConn.func1
E220812 13:47:11.417605 1525232 1@util/log/logcrash/crash_reporting.go:174 ⋮ [n1,client=127.0.0.1:60474,user=root] 591 +  |     github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:806
E220812 13:47:11.417605 1525232 1@util/log/logcrash/crash_reporting.go:174 ⋮ [n1,client=127.0.0.1:60474,user=root] 591 +  | [...repeated from below...]
E220812 13:47:11.417605 1525232 1@util/log/logcrash/crash_reporting.go:174 ⋮ [n1,client=127.0.0.1:60474,user=root] 591 +Wraps: (2) while executing: SELECT ((crdb_internal.stream_ingestion_stats_json(_)
->_)->_)->>_

Jira issue: CRDB-18541
@stevendanna stevendanna added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label Aug 12, 2022
@stevendanna stevendanna added this to the 22.2 milestone Aug 12, 2022
@stevendanna stevendanna self-assigned this Aug 12, 2022
@stevendanna stevendanna mentioned this issue Aug 15, 2022
Merged
craig bot pushed a commit that referenced this issue Aug 17, 2022
@stevendanna @pav-kv
Merge #86123 #86130
39a86d3 
86123: streamingccl: fix NPE in crdb_internal.stream_ingestion_stats* r=ajwerner a=stevendanna

This fixes a NPE that could occur if the user passed in the Job ID of
a job that isn't a stream ingestion job.

Fixes #86040

Release note: None

Release justification: Bug fix for non-production feature.

86130: kvserver: annotate Replica.Send with pprof labels r=tbg a=pavelkalinnikov

This commit adds pprof labels to Send methods, as a proper replacement of the
no longer working (since Go 1.17) unnamed parameters hack. Adding labels is
conditional to the CPU profile cluster setting, to avoid the profiling cost and
extra allocations in pprof.Do context construction during normal operation.

Fixes #85948

Release justification: Profiling labels help investigating customer issues
Release note: None

Co-authored-by: Steven Danna <[email protected]>
Co-authored-by: Pavel Kalinnikov <[email protected]>
@craig craig bot closed this as completed in b0fc809 Aug 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@stevendanna stevendanna

Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.
Projects
None yet
Milestone
22.2
Development

Successfully merging a pull request may close this issue.

streamingccl: fix NPE in crdb_internal.stream_ingestion_stats* stevendanna/cockroach
1 participant
@stevendanna