cockroach version panic in schema change

[davepacheco]

(originally filed as #83706, moving here as requested)

Describe the problem

While trying to reproduce #82958, I found a case where cockroach version exited with status 2. Thinking maybe it would be reproducible, I started running that in a loop. I saw this panic:

panic: invalid predicate with signature *fmt.pp != func(*scpb.Column, scplan.Params) bool for *scpb.Column[0]

goroutine 1 [running]:
github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scplan.buildSchemaChangeOpGenFunc(0x703b2a0, 0x0, 0xc000e0aff0, 0xc000e0b050, 0xc000e08c18)
	/ws/gc/cockroach/cache/gopath/src/github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scplan/declarative.go:171 +0xd93
github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scplan.buildSchemaChangePlanner(0xc000e0ab40, 0xc000e0ab40)
	/ws/gc/cockroach/cache/gopath/src/github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scplan/declarative.go:60 +0x125
github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scplan.init()
	/ws/gc/cockroach/cache/gopath/src/github.co

To Reproduce

Just run cockroach version in a loop unti it exits. At first, I just did:

for ((i = 0; ; i++)) { echo "ATTEMPT $i"; cockroach version || break; }

Now I'm using this:

#!/bin/bash

# attempt to reproduce failure from `cockroach version`

# specifies a specific `cockroach` binary
export PATH="$HOME/tools/cockroachdb-v21.2.9/bin:$PATH"

date +%FT%TZ > start_time
which cockroach > which_cockroach

for ((i = 0; ; i++)) {
	echo "ATTEMPT $i"
	echo $i > attempt_number
	cockroach version > stdout 2>stderr
	rv=$?
	if [[ $rv -ne 0 ]]; then
		echo $rv > exit_status
		echo "exited with status $rv";
		break;
	fi
}

Unfortunately I lost the status code but I've fixed the above script to avoid that.

It took just over 53h and 1.4M iterations to hit this.

Expected behavior

This should run indefinitely without issue.

Environment:

$ cockroach version
Build Tag:        v21.2.9
Build Time:       2022/04/28 04:02:42
Distribution:     OSS
Platform:         illumos amd64 (x86_64-pc-solaris2.11)
Go Version:       go1.16.10
C Compiler:       gcc 10.3.0
Build Commit ID:  11787edfcfc157a0df951abc34684e4e18b3ef20
Build Type:       release

This is on helios helios-1.0.21004.

CC @knz

Jira issue: CRDB-17319

[blathers-crl]

Hello, I am Blathers. I am here to help you get the issue triaged.

Hoot - a bug! Though bugs are the bane of my existence, rest assured the wretched thing will get the best of care here.

I have CC'd a few people who may be able to assist you:

• @stevendanna (commented on cli: logconfig reset panic due to race condition in cockroach version #83706)
• @nicktrav (commented on cli: logconfig reset panic due to race condition in cockroach version #83706, commented on crash in Go runtime after port_getn returned EINVAL #82958)
• @knz (commented on cli: logconfig reset panic due to race condition in cockroach version #83706, commented on crash in Go runtime after port_getn returned EINVAL #82958)
• @ajwerner (commented on cli: logconfig reset panic due to race condition in cockroach version #83706)
• @cockroachdb/bulk-io (found keywords: export)
• @abarganier (assigned to cli: logconfig reset panic due to race condition in cockroach version #83706)
• @cockroachdb/sql-queries (found keywords: plan)

If we have not gotten back to your issue within a few business days, you can try the following:

• Join our community slack channel and ask on #cockroachdb.
• Try find someone from here if you know they worked closely on the area and CC them.

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.}

[blathers-crl]

cc @cockroachdb/bulk-io

[ajwerner]

This crash seems unreasonable without something wild going on. Maybe some sort of memory corruption? The code is here:

cockroach/pkg/sql/schemachanger/scplan/declarative.go

Line 170 in 11787ed

if pt := reflect.TypeOf(rule.predicate); pt != predicateTyp {

This is init time code processing some narrowly defined structures. There's not a ton of leeway here. The only instantiations of that struct are in the below file and given the rest of the error, we know that the structs must have been one of these.
Also, do *fmt.pp objects ever make their way out of the fmt package (see https://go.dev/src/fmt/print.go#L104)?

cockroach/pkg/sql/schemachanger/scplan/rules.go

Lines 732 to 749 in 11787ed

	predicate: columnInSecondaryIndex,
	},
	{
	dirPredicate: sameDirection,
	thatStatus: scpb.Status_DELETE_AND_WRITE_ONLY,
	predicate: columnInPrimaryIndex,
	},
	},
	scpb.Status_PUBLIC: {
	{
	dirPredicate: bothDirectionsEqual(scpb.Target_ADD),
	thatStatus: scpb.Status_PUBLIC,
	predicate: columnInSecondaryIndex,
	},
	{
	dirPredicate: bothDirectionsEqual(scpb.Target_ADD),
	thatStatus: scpb.Status_PUBLIC,
	predicate: columnInPrimaryIndex,

[knz]

They shouldnt. See the latest comment in #83706, I now suspect this particular go runtime / OS combination is borked in a way independent of crdb.

[ajwerner]

Given the evidence of runtime corruption, I'm closing this.

[davepacheco]

If the root cause is memory corruption, isn't this still a bug?

[ajwerner]

If the memory corruption is in the go runtime, is it a cockroach bug? We generally write code assuming the go runtime works according to the language specification, I think in this case, it doesn't.

[davepacheco]

I didn't think we had any evidence that the corruption was caused by the Go runtime. (Obviously if anything in the process is corrupting memory, then the corruption can appear in any subsystem -- and indeed we've seen corruption in CockroachDB as well as a few different parts of the Go runtime.) But if it's preferable, I can track this issue outside this project and reopen it if/when I have more data.

[ajwerner]

I interpreted #83706 (comment) as an indication of widespread memory corruption. I can't make a claim as to whether this corruption is in go or in some other code, perhaps some linked cgo code. Either way, I do not thing there's value in maintaining this issue separately. Any root cause here doesn't have anything to do with the code implicated by this issue.

I hope we all can get to the bottom of it.

[davepacheco]

Just to close the loop on this: see oxidecomputer/omicron#1146 for gory details, but at this point I believe the corruption in that issue was due to this illumos OS issue. The behavior there is that memory that is supposed to be zero'd by the Go runtime may not be properly zero'd. It looks to me like that could explain this bug, so that seems the likely culprit.

[knz]

thank you.