cockroach icon indicating copy to clipboard operation
cockroach copied to clipboard

Published 20 hours ago • verified publisher icon cockroachdb

pkg/sql/logictest/tests/fakedist-vec-off/fakedist-vec-off_test: TestLogic_enums failed

Open cockroach-teamcity opened this issue 3 years ago • 8 comments

pkg/sql/logictest/tests/fakedist-vec-off/fakedist-vec-off_test.TestLogic_enums failed with artifacts on master @ 5fe052748de0406d7cb2c8a1e9230143f54f7534:

=== RUN   TestLogic_enums/regression_63138

Parameters: TAGS=bazel,gss

Help

See also: How To Investigate a Go Test Failure (internal)

This test on roachdash | Improve this report!

Jira issue: CRDB-18214

cockroach-teamcity avatar Jul 31 '22 08:07 cockroach-teamcity

This one is a panic with an enum when serializing the session:

[08:00:48][Run stress] === RUN   TestLogic_enums/regression_63138
[08:00:48][Run stress] panic: runtime error: invalid memory address or nil pointer dereference
[08:00:48][Run stress] [signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x18a86fb]
[08:00:48][Run stress] 
[08:00:48][Run stress] goroutine 600684 [running]:
[08:00:48][Run stress] github.com/cockroachdb/cockroach/pkg/sql/types.(*T).SQLString(0xc0099e0840)
[08:00:48][Run stress] 	github.com/cockroachdb/cockroach/pkg/sql/types/pkg/sql/types/types.go:1864 +0x131b
[08:00:48][Run stress] github.com/cockroachdb/cockroach/pkg/sql/types.(*T).SQLString(0xc0099e0240)
[08:00:48][Run stress] 	github.com/cockroachdb/cockroach/pkg/sql/types/pkg/sql/types/types.go:1859 +0xd3c
[08:00:48][Run stress] github.com/cockroachdb/cockroach/pkg/sql/sem/tree.(*FmtCtx).FormatTypeReference(0xc007dc7dc0, {0x645a120, 0xc0099e0240})
[08:00:48][Run stress] 	github.com/cockroachdb/cockroach/pkg/sql/sem/tree/type_name.go:189 +0x28e
[08:00:48][Run stress] github.com/cockroachdb/cockroach/pkg/sql/sem/tree.(*AnnotateTypeExpr).Format(0xc00d6858f0, 0xc007dc7dc0)
[08:00:48][Run stress] 	github.com/cockroachdb/cockroach/pkg/sql/sem/tree/expr.go:1567 +0xbc
[08:00:48][Run stress] github.com/cockroachdb/cockroach/pkg/sql/sem/tree.(*FmtCtx).formatNodeOrHideConstants(0xc007dc7dc0, {0x6456860, 0xc00d6858f0})
[08:00:48][Run stress] 	github.com/cockroachdb/cockroach/pkg/sql/sem/tree/hide_constants.go:46 +0x36d
[08:00:48][Run stress] github.com/cockroachdb/cockroach/pkg/sql/sem/tree.(*FmtCtx).FormatNode(0xc007dc7dc0, {0x6456860, 0xc00d6858f0})
[08:00:48][Run stress] 	github.com/cockroachdb/cockroach/pkg/sql/sem/tree/format.go:449 +0x3b2
[08:00:48][Run stress] github.com/cockroachdb/cockroach/pkg/sql/sem/tree.exprFmtWithParen(0xc007dc7dc0, {0x64a3f20, 0xc00d6858f0})
[08:00:48][Run stress] 	github.com/cockroachdb/cockroach/pkg/sql/sem/tree/expr.go:134 +0x12f
[08:00:48][Run stress] github.com/cockroachdb/cockroach/pkg/sql/sem/tree.binExprFmtWithParenAndSubOp(0xc007dc7dc0, {0x64a4cb0, 0xc009b245a0}, {0x5c3a31f, 0x1}, {0x5c3ac91, 0x3}, {0x64a3f20, 0xc00d6858f0})
[08:00:48][Run stress] 	github.com/cockroachdb/cockroach/pkg/sql/sem/tree/expr.go:187 +0xff
[08:00:48][Run stress] github.com/cockroachdb/cockroach/pkg/sql/sem/tree.(*ComparisonExpr).Format(0xc009b24690, 0xc007dc7dc0)
[08:00:48][Run stress] 	github.com/cockroachdb/cockroach/pkg/sql/sem/tree/expr.go:387 +0x37c
[08:00:48][Run stress] github.com/cockroachdb/cockroach/pkg/sql/sem/tree.(*FmtCtx).formatNodeOrHideConstants(0xc007dc7dc0, {0x6456f20, 0xc009b24690})
[08:00:48][Run stress] 	github.com/cockroachdb/cockroach/pkg/sql/sem/tree/hide_constants.go:46 +0x36d
[08:00:48][Run stress] github.com/cockroachdb/cockroach/pkg/sql/sem/tree.(*FmtCtx).FormatNode(0xc007dc7dc0, {0x6456f20, 0xc009b24690})
[08:00:48][Run stress] 	github.com/cockroachdb/cockroach/pkg/sql/sem/tree/format.go:449 +0x3b2
[08:00:48][Run stress] github.com/cockroachdb/cockroach/pkg/sql/sem/tree.(*FuncExpr).Format(0xc009275a00, 0xc007dc7dc0)
[08:00:48][Run stress] 	github.com/cockroachdb/cockroach/pkg/sql/sem/tree/expr.go:1367 +0x4a5
[08:00:48][Run stress] github.com/cockroachdb/cockroach/pkg/sql/sem/tree.(*FmtCtx).formatNodeOrHideConstants(0xc007dc7dc0, {0x6457bc0, 0xc009275a00})
[08:00:48][Run stress] 	github.com/cockroachdb/cockroach/pkg/sql/sem/tree/hide_constants.go:46 +0x36d
[08:00:48][Run stress] github.com/cockroachdb/cockroach/pkg/sql/sem/tree.(*FmtCtx).FormatNode(0xc007dc7dc0, {0x6457bc0, 0xc009275a00})
[08:00:48][Run stress] 	github.com/cockroachdb/cockroach/pkg/sql/sem/tree/format.go:449 +0x3b2
[08:00:48][Run stress] github.com/cockroachdb/cockroach/pkg/sql/sem/tree.(*SelectExpr).Format(0xc00ad3f2e0, 0xc007dc7dc0)
[08:00:48][Run stress] 	github.com/cockroachdb/cockroach/pkg/sql/sem/tree/select.go:183 +0x65
[08:00:48][Run stress] github.com/cockroachdb/cockroach/pkg/sql/sem/tree.(*FmtCtx).formatNodeOrHideConstants(0xc007dc7dc0, {0x64591c0, 0xc00ad3f2e0})
[08:00:48][Run stress] 	github.com/cockroachdb/cockroach/pkg/sql/sem/tree/hide_constants.go:46 +0x36d
[08:00:48][Run stress] github.com/cockroachdb/cockroach/pkg/sql/sem/tree.(*FmtCtx).FormatNode(0xc007dc7dc0, {0x64591c0, 0xc00ad3f2e0})
[08:00:48][Run stress] 	github.com/cockroachdb/cockroach/pkg/sql/sem/tree/format.go:449 +0x3b2
[08:00:48][Run stress] github.com/cockroachdb/cockroach/pkg/sql/sem/tree.(*SelectExprs).Format(0xc00066e860, 0xc007dc7dc0)
[08:00:48][Run stress] 	github.com/cockroachdb/cockroach/pkg/sql/sem/tree/select.go:150 +0xb5
[08:00:48][Run stress] github.com/cockroachdb/cockroach/pkg/sql/sem/tree.(*FmtCtx).formatNodeOrHideConstants(0xc007dc7dc0, {0x64591e0, 0xc00066e860})
[08:00:48][Run stress] 	github.com/cockroachdb/cockroach/pkg/sql/sem/tree/hide_constants.go:46 +0x36d
[08:00:48][Run stress] github.com/cockroachdb/cockroach/pkg/sql/sem/tree.(*FmtCtx).FormatNode(0xc007dc7dc0, {0x64591e0, 0xc00066e860})
[08:00:48][Run stress] 	github.com/cockroachdb/cockroach/pkg/sql/sem/tree/format.go:449 +0x3b2
[08:00:48][Run stress] github.com/cockroachdb/cockroach/pkg/sql/sem/tree.(*SelectClause).Format(0xc00066e820, 0xc007dc7dc0)
[08:00:48][Run stress] 	github.com/cockroachdb/cockroach/pkg/sql/sem/tree/select.go:117 +0x27b
[08:00:48][Run stress] github.com/cockroachdb/cockroach/pkg/sql/sem/tree.(*FmtCtx).formatNodeOrHideConstants(0xc007dc7dc0, {0x64591a0, 0xc00066e820})
[08:00:48][Run stress] 	github.com/cockroachdb/cockroach/pkg/sql/sem/tree/hide_constants.go:46 +0x36d
[08:00:48][Run stress] github.com/cockroachdb/cockroach/pkg/sql/sem/tree.(*FmtCtx).FormatNode(0xc007dc7dc0, {0x64591a0, 0xc00066e820})
[08:00:48][Run stress] 	github.com/cockroachdb/cockroach/pkg/sql/sem/tree/format.go:449 +0x3b2
[08:00:48][Run stress] github.com/cockroachdb/cockroach/pkg/sql/sem/tree.(*Select).Format(0xc009b248c0, 0xc007dc7dc0)
[08:00:48][Run stress] 	github.com/cockroachdb/cockroach/pkg/sql/sem/tree/select.go:56 +0xa7
[08:00:48][Run stress] github.com/cockroachdb/cockroach/pkg/sql/sem/tree.(*FmtCtx).formatNodeOrHideConstants(0xc007dc7dc0, {0x6459180, 0xc009b248c0})
[08:00:48][Run stress] 	github.com/cockroachdb/cockroach/pkg/sql/sem/tree/hide_constants.go:46 +0x36d
[08:00:48][Run stress] github.com/cockroachdb/cockroach/pkg/sql/sem/tree.(*FmtCtx).FormatNode(0xc007dc7dc0, {0x6459180, 0xc009b248c0})
[08:00:48][Run stress] 	github.com/cockroachdb/cockroach/pkg/sql/sem/tree/format.go:449 +0x3b2
[08:00:48][Run stress] github.com/cockroachdb/cockroach/pkg/sql/sem/tree.AsStringWithFlags({0x6459180, 0xc009b248c0}, 0x0, {0x0, 0x0, 0x0})
[08:00:48][Run stress] 	github.com/cockroachdb/cockroach/pkg/sql/sem/tree/format.go:587 +0xb0
[08:00:48][Run stress] github.com/cockroachdb/cockroach/pkg/sql/sem/tree.AsString({0x6459180, 0xc009b248c0})
[08:00:48][Run stress] 	github.com/cockroachdb/cockroach/pkg/sql/sem/tree/format.go:602 +0x3c
[08:00:48][Run stress] github.com/cockroachdb/cockroach/pkg/sql/sem/tree.(*Select).String(0xc009b248c0)
[08:00:48][Run stress] 	github.com/cockroachdb/cockroach/pkg/sql/sem/tree/stmt.go:1969 +0x35
[08:00:48][Run stress] github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).serialize(_)
[08:00:48][Run stress] 	github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:3158 +0xb89
[08:00:48][Run stress] github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).run.func1()
[08:00:48][Run stress] 	github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:1829 +0xe7
[08:00:48][Run stress] github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).run(0xc006c50a00, {0x6488bd0, 0xc00d685560}, 0xc004561600, 0xc00d685e60, 0x0)
[08:00:48][Run stress] 	github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:1844 +0x4b0
[08:00:48][Run stress] github.com/cockroachdb/cockroach/pkg/sql.(*InternalExecutor).initConnEx.func1()
[08:00:48][Run stress] 	github.com/cockroachdb/cockroach/pkg/sql/internal.go:212 +0xef
[08:00:48][Run stress] created by github.com/cockroachdb/cockroach/pkg/sql.(*InternalExecutor).initConnEx
[08:00:48][Run stress] 	github.com/cockroachdb/cockroach/pkg/sql/internal.go:211 +0x6c5
[08:00:48][Run stress] I220731 08:00:47.961309 1 (gostd) testmain.go:642  [-] 1  Test //pkg/sql/logictest/tests/fakedist-vec-off:fakedist-vec-off_test exited with error code 2
[08:00:48][Run stress] 
[08:00:48][Run stress] 
[08:00:48][Run stress] ERROR: <nil>
[08:00:48][Run stress] 
[08:00:48][Run stress] 5 runs completed, 1 failures, over 26m31s

I think there are two problems here:

  • the fact that we're crashing. This stack trace suggests that we were executing VERIFY INDEX internal query from here during backfill of CREATE INDEX foo ON tab(k) WHERE k = ANY (ARRAY['a', 'a']:::typ2[]). It's not immediately clear what exactly was nil causing the crash under stress, but it has something to do with enums. cc @rafiss @cockroachdb/sql-schema in case some of you know something.
  • the fact that the internal executor is registered with the closed session cache. I think it was an omission and I'll fix that.

The first point mostly looks like the schema issue, so putting it on the corresponding board.

yuzefovich avatar Aug 10 '22 01:08 yuzefovich

What was nil here was the Name of the UDT: https://github.com/cockroachdb/cockroach/blob/ddabd1890635ce088ec60af5cbf39bda2fe60ace/pkg/sql/types/types.go#L191-L192

I don't know that we have a proof that the type has been hydrated. I have a feeling that the issue runs a little bit deeper in that maybe we're hydrating the array type but not the contents?

ajwerner avatar Aug 10 '22 01:08 ajwerner

Okay, I did some digging. I think the core problem ends up being here: https://github.com/cockroachdb/cockroach/blob/9804737e205f7de96c1f0383d90340b974e1ed18/pkg/sql/sem/tree/type_name.go#L153-L157

In TypeCheck we call ResolveType and overwrite the reference to the type in the AST to the resolved type. Now, the problem here is that we don't go and resolve the reference type. I think I have a fix I want.

ajwerner avatar Aug 10 '22 19:08 ajwerner

Err nevermind that does not make sense.

ajwerner avatar Aug 10 '22 19:08 ajwerner

@ajwerner do you have an idea for why this diff

diff --git a/pkg/sql/catalog/typedesc/type_desc.go b/pkg/sql/catalog/typedesc/type_desc.go
index 02e77e5a40..08820572d9 100644
--- a/pkg/sql/catalog/typedesc/type_desc.go
+++ b/pkg/sql/catalog/typedesc/type_desc.go
@@ -796,6 +796,9 @@ func EnsureTypeIsHydrated(
                                return err
                        }
                }
+       } else if t.Family() == types.ArrayFamily {
+               // Hydrate the element type.
+               return EnsureTypeIsHydrated(ctx, t.ArrayContents(), res)
        }
        if !t.UserDefined() || t.IsHydrated() {
                return nil
@@ -897,11 +900,7 @@ func (desc *immutable) HydrateTypeInfoWithName(
        case descpb.TypeDescriptor_ALIAS:
                if typ.UserDefined() {
                        switch typ.Family() {
-                       case types.ArrayFamily:
-                               // Hydrate the element type.
-                               elemType := typ.ArrayContents()
-                               return EnsureTypeIsHydrated(ctx, elemType, res)
-                       case types.TupleFamily:
+                       case types.ArrayFamily, types.TupleFamily:
                                return EnsureTypeIsHydrated(ctx, typ, res)
                        default:
                                return errors.AssertionFailedf("unhandled alias type family %s", typ.Family())

would actually break things?

panic: runtime error: invalid memory address or nil pointer dereference [recovered]
	panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x4d265c5]

goroutine 4573 [running]:
github.com/cockroachdb/cockroach/pkg/util/stop.(*Stopper).recover(0xc00024fdd0, {0x8cee300, 0xc0014a9890})
	github.com/cockroachdb/cockroach/pkg/util/stop/stopper.go:248 +0xa5
panic({0x7ad82c0, 0xb33daa0})
	GOROOT/src/runtime/panic.go:838 +0x207
github.com/cockroachdb/cockroach/pkg/sql/types.(*T).PGName(0xc003df0240)
	github.com/cockroachdb/cockroach/pkg/sql/types/pkg/sql/types/types.go:1535 +0xc5
github.com/cockroachdb/cockroach/pkg/sql.glob..func109.1({0x8d546e0?, 0xc001779050?}, {0xc002b26588, 0x6}, {0x8d75c48, 0xc003def900})
	github.com/cockroachdb/cockroach/pkg/sql/information_schema.go:530 +0xcb3
github.com/cockroachdb/cockroach/pkg/sql.forEachTableDesc.func1({0x8d546e0?, 0xc001779050?}, {0xc002b26588?, 0x6800000069?}, {0x8d75c48?, 0xc003def900?}, 0xc001779050?)
	github.com/cockroachdb/cockroach/pkg/sql/information_schema.go:2292 +0x37
github.com/cockroachdb/cockroach/pkg/sql.forEachTableDescWithTableLookupInternalFromDescriptors({0x8cee300, 0xc0014a9890}, 0xc00164dc00, {0x8d546e0, 0xc0015d7320}, 0x0, 0x64?, {{{0xc003cb2080}, {0xc003cb2040}, 0xc003952db0}, ...}, ...)
	github.com/cockroachdb/cockroach/pkg/sql/information_schema.go:2549 +0x518

yuzefovich avatar Aug 10 '22 19:08 yuzefovich

Doesn't your patch fail to actually hydrate the Array type with a name?

ajwerner avatar Aug 10 '22 19:08 ajwerner

Why do you say so? The patch seems like a noop to me.

yuzefovich avatar Aug 10 '22 20:08 yuzefovich

I solved the mystery! It took way too long :(

The type hydration code short-circuits if the type is already hydrated. The determination as to whether it is hydrated is: https://github.com/cockroachdb/cockroach/blob/ddabd1890635ce088ec60af5cbf39bda2fe60ace/pkg/sql/types/types.go#L2690

You'll notice that we populate some of these fields, and then proceed to try to hydrate the array contents, which can fail.

https://github.com/cockroachdb/cockroach/blob/8a9ebff7ea0aea85f72b49fd336e8462d72d2c89/pkg/sql/catalog/typedesc/type_desc.go#L882-L887

If we do fail to hydrate the array contents, then we'll be in a bad state.

https://github.com/cockroachdb/cockroach/pull/85940

ajwerner avatar Aug 11 '22 02:08 ajwerner