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

roachtest: clearrange/checks=true failed #60849

Closed
cockroach-teamcity opened this issue Feb 21, 2021 · 2 comments · Fixed by #60992
Closed

roachtest: clearrange/checks=true failed #60849

cockroach-teamcity opened this issue Feb 21, 2021 · 2 comments · Fixed by #60992
Assignees
@nvanbenschoten
Labels
branch-master Failures and bugs on the master branch. C-test-failure Broken test (automatically or manually discovered). O-roachtest O-robot Originated from a bot. release-blocker Indicates a release-blocker. Use with branch-release-2x.x label to denote which branch is blocked.

Comments

@cockroach-teamcity
Copy link
Member

(roachtest).clearrange/checks=true failed on master@64c4aef909f4382523cd9248341ca9f4448d841a:

		  | main.main()
		  | 	/go/src/github.com/cockroachdb/cockroach/pkg/cmd/cockroach/main.go:26 +0x25 fp=0xc001917f88 sp=0xc001917f78 pc=0x3ddc705
		  | runtime.main()
		  | 	/usr/local/go/src/runtime/proc.go:204 +0x209 fp=0xc001917fe0 sp=0xc001917f88 pc=0x48de09
		  | runtime.goexit()
		  | 	/usr/local/go/src/runtime/asm_amd64.s:1374 +0x1 fp=0xc001917fe8 sp=0xc001917fe0 pc=0x4c4641
		  |
		  | goroutine 2 [force gc (idle), 2 minutes]:
		  | runtime.gopark(0x5185308, 0x8446d00, 0x1411, 0x1)
		  | 	/usr/local/go/src/runtime/proc.go:306 +0xe5 fp=0xc0000acfb0 sp=0xc0000acf90 pc=0x48e205
		  | runtime.goparkunlock(...)
		  | 	/usr/local/go/src/runtime/proc.go:312
		  | runtime.forcegchelper()
		  | 	/usr/local/go/src/runtime/proc.go:255 +0xc5 fp=0xc0000acfe0 sp=0xc0000acfb0 pc=0x48e0a5
		  | runtime.goexit()
		  | 	/usr/local/go/src/runtime/asm_amd64.s:1374 +0x1 fp=0xc0000acfe8 sp=0xc0000acfe0 pc=0x4c4641
		  | created by runtime.init.6
		  | 	/usr/local/go/src/runtime/proc.go:243 +0x35
		  |
		  | goroutine 3 [GC sweep wait]:
		  | runtime.gopark(0x5185308, 0x844e580, 0x140c, 0x1)
		  | 	/usr/local/go/src/runtime/proc.go:306 +0xe5 fp=0xc0000ad7a8 sp=0xc0000ad788 pc=0x48e205
		  | runtime.goparkunlock(...)
		  | 	/usr/local/go/src/runtime/proc.go:312
		  | runtime.bgsweep(0xc0000d4000)
		  | 	/usr/local/go/src/runtime/mgcsweep.go:182 +0x13b fp=0xc0000ad7d8 sp=0xc0000ad7a8 pc=0x4787db
		  | runtime.goexit()
		  | 	/usr/local/go/src/runtime/asm_amd64.s:1374 +0x1 fp=0xc0000ad7e0 sp=0xc0000ad7d8 pc=0x4c4641
		  | created by runtime.gcenable
		  | 	/usr/local/go/src/runtime/mgc.go:217 +0x5c
		  |
		  | goroutine 4 [GC scavenge wait]:
		  | runtime.gopark(0x5185308, 0x844f600, 0x140d, 0x1)
		  | 	/usr/local/go/src/runtime/proc.go:306 +0xe5 fp=0xc0000adf78 sp=0xc0000adf58 pc=0x48e205
		  | runtime.goparkunlock(...)
		  | 	/usr/local/go/src/runtime/proc.go:312
		  | runtime.bgscavenge(0xc0000d4000)
		  | 	/usr/local/go/src/runtime/mgcscavenge.go:314 +0x29c fp=0xc0000adfd8 sp=0xc0000adf78 pc=0x47693c
		  | runtime.goexit()
		  | 	/usr/local/go/src/runtime/asm_amd64.s:1374 +0x1 fp=0xc0000adfe0 sp=0xc0000adfd8 pc=0x4c4641
		  | created by runtime.gcenable
		  | 	/usr/local/go/src/runtime/mgc.go:218 +0x7e
		  |
		  | stdout:
		Wraps: (4) secondary error attachment
		  | signal: killed
		  | (1) signal: killed
		  | Error types: (1) *exec.ExitError
		Wraps: (5) context canceled
		Error types: (1) *withstack.withStack (2) *errutil.withPrefix (3) *main.withCommandDetails (4) *secondary.withSecondaryError (5) *errors.errorString
More

Artifacts: /clearrange/checks=true

See this test on roachdash
powered by pkg/cmd/internal/issues
@cockroach-teamcity cockroach-teamcity added branch-master Failures and bugs on the master branch. C-test-failure Broken test (automatically or manually discovered). O-roachtest O-robot Originated from a bot. release-blocker Indicates a release-blocker. Use with branch-release-2x.x label to denote which branch is blocked. labels Feb 21, 2021
@cockroach-teamcity
Copy link
Member Author

(roachtest).clearrange/checks=true failed on master@bf9744bad5a416a4b06907f0f3dd42896f7342f3:

		  |
		  | goroutine 3 [GC sweep wait]:
		  | runtime.gopark(0x51874a8, 0x84534e0, 0x140c, 0x1)
		  | 	/usr/local/go/src/runtime/proc.go:306 +0xe5 fp=0xc0000ad7a8 sp=0xc0000ad788 pc=0x48e205
		  | runtime.goparkunlock(...)
		  | 	/usr/local/go/src/runtime/proc.go:312
		  | runtime.bgsweep(0xc0000d4000)
		  | 	/usr/local/go/src/runtime/mgcsweep.go:182 +0x13b fp=0xc0000ad7d8 sp=0xc0000ad7a8 pc=0x4787db
		  | runtime.goexit()
		  | 	/usr/local/go/src/runtime/asm_amd64.s:1374 +0x1 fp=0xc0000ad7e0 sp=0xc0000ad7d8 pc=0x4c4641
		  | created by runtime.gcenable
		  | 	/usr/local/go/src/runtime/mgc.go:217 +0x5c
		  |
		  | goroutine 4 [GC scavenge wait]:
		  | runtime.gopark(0x51874a8, 0x8454560, 0x140d, 0x1)
		  | 	/usr/local/go/src/runtime/proc.go:306 +0xe5 fp=0xc0000adf78 sp=0xc0000adf58 pc=0x48e205
		  | runtime.goparkunlock(...)
		  | 	/usr/local/go/src/runtime/proc.go:312
		  | runtime.bgscavenge(0xc0000d4000)
		  | 	/usr/local/go/src/runtime/mgcscavenge.go:314 +0x29c fp=0xc0000adfd8 sp=0xc0000adf78 pc=0x47693c
		  | runtime.goexit()
		  | 	/usr/local/go/src/runtime/asm_amd64.s:1374 +0x1 fp=0xc0000adfe0 sp=0xc0000adfd8 pc=0x4c4641
		  | created by runtime.gcenable
		  | 	/usr/local/go/src/runtime/mgc.go:218 +0x7e
		  |
		  | goroutine 5 [finalizer wait, 389 minutes]:
		  | runtime.gopark(0x51874a8, 0x84aa0a8, 0xc000131410, 0x1)
		  | 	/usr/local/go/src/runtime/proc.go:306 +0xe5 fp=0xc0000ac758 sp=0xc0000ac738 pc=0x48e205
		  | runtime.goparkunlock(...)
		  | 	/usr/local/go/src/runtime/proc.go:312
		  | runtime.runfinq()
		  | 	/usr/local/go/src/runtime/mfinal.go:175 +0xa9 fp=0xc0000ac7e0 sp=0xc0000ac758 pc=0x46d529
		  | runtime.goexit()
		  | 	/usr/local/go/src/runtime/asm_amd64.s:1374 +0x1 fp=0xc0000ac7e8 sp=0xc0000ac7e0 pc=0x4c4641
		  | created by runtime.createfing
		  | 	/usr/local/go/src/runtime/mfinal.go:156 +0x65
		  |
		  | goroutine 6 [chan receive]:
		  | runtime.gopark(0x5187290, 0xc00010e058, 0xc0001e170e, 0x2)
		  | 	/usr/local/go/src/runtime/proc.go:306 +0xe5 fp=0xc0000ae6a8 sp=0xc0000ae688 pc=0x48e205
		  | runtime.chanrecv(0xc00010e000, 0xc0000ae7b8, 0xf7ae01, 0xc0000ae7a0)
		  | 	/usr/local/go/src/runtime/chan.go:577 +0x36f fp=0xc0000ae738 sp=0xc0000ae6a8 pc=0x45a38f
		  | runtime.chanrecv2
		  | stdout:
		Wraps: (4) secondary error attachment
		  | signal: killed
		  | (1) signal: killed
		  | Error types: (1) *exec.ExitError
		Wraps: (5) context canceled
		Error types: (1) *withstack.withStack (2) *errutil.withPrefix (3) *main.withCommandDetails (4) *secondary.withSecondaryError (5) *errors.errorString
More

Artifacts: /clearrange/checks=true

See this test on roachdash
powered by pkg/cmd/internal/issues

nvanbenschoten added a commit to nvanbenschoten/cockroach that referenced this issue Feb 23, 2021
@nvanbenschoten
kv: make RaftCommand.ClosedTimestamp nullable
d7bb607 
Fixes cockroachdb#60852.
Fixes cockroachdb#60833.
Fixes cockroachdb#58298.
Fixes cockroachdb#59428.
Fixes cockroachdb#60756.
Fixes cockroachdb#60848.
Fixes cockroachdb#60849.

In cockroachdb#60852 and related issues, we saw that the introduction of a non-nullable
`RaftCommand.ClosedTimestamp`, coupled with the `ClosedTimestampFooter` encoding
strategy we use, led to encoded `RaftCommand` protos with their ClosedTimestamp
field set twice. This is ok from a correctness perspective, at least as protobuf
is concerned, but it led to a subtle interaction where the process of passing
through sideloading (`maybeInlineSideloadedRaftCommand(maybeSideloadEntriesImpl(e))`)
would reduce the size of an encoded RaftCommand by 3 bytes (the encoded size of
an empty `hlc.Timestamp`). This was resulting in an `uncommittedSize` leak in
Raft, which was eventually stalling on its `MaxUncommittedEntriesSize` limit.

This commit fixes this issue by making `RaftCommand.ClosedTimestamp` nullable.
With the field marked as nullable, it will no longer be encoded as an empty
timestamp when unset, ensuring that when the encoded `ClosedTimestampFooter` is
appended, it contains the only instance of the `ClosedTimestamp` field.
@nvanbenschoten nvanbenschoten mentioned this issue Feb 23, 2021
Merged
@cockroach-teamcity
Copy link
Member Author

(roachtest).clearrange/checks=true failed on master@5cfd7e5553a3072a1490d392390dddf968844215:

		  | 	/usr/local/go/src/runtime/proc.go:312
		  | runtime.bgsweep(0xc0000d2000)
		  | 	/usr/local/go/src/runtime/mgcsweep.go:182 +0x13b fp=0xc0000ab7d8 sp=0xc0000ab7a8 pc=0x4787db
		  | runtime.goexit()
		  | 	/usr/local/go/src/runtime/asm_amd64.s:1374 +0x1 fp=0xc0000ab7e0 sp=0xc0000ab7d8 pc=0x4c4641
		  | created by runtime.gcenable
		  | 	/usr/local/go/src/runtime/mgc.go:217 +0x5c
		  |
		  | goroutine 4 [GC scavenge wait]:
		  | runtime.gopark(0x518f250, 0x84638a0, 0x140d, 0x1)
		  | 	/usr/local/go/src/runtime/proc.go:306 +0xe5 fp=0xc0000abf78 sp=0xc0000abf58 pc=0x48e205
		  | runtime.goparkunlock(...)
		  | 	/usr/local/go/src/runtime/proc.go:312
		  | runtime.bgscavenge(0xc0000d2000)
		  | 	/usr/local/go/src/runtime/mgcscavenge.go:314 +0x29c fp=0xc0000abfd8 sp=0xc0000abf78 pc=0x47693c
		  | runtime.goexit()
		  | 	/usr/local/go/src/runtime/asm_amd64.s:1374 +0x1 fp=0xc0000abfe0 sp=0xc0000abfd8 pc=0x4c4641
		  | created by runtime.gcenable
		  | 	/usr/local/go/src/runtime/mgc.go:218 +0x7e
		  |
		  | goroutine 18 [finalizer wait, 389 minutes]:
		  | runtime.gopark(0x518f250, 0x84b93e8, 0xc0009f1410, 0x1)
		  | 	/usr/local/go/src/runtime/proc.go:306 +0xe5 fp=0xc0000aa758 sp=0xc0000aa738 pc=0x48e205
		  | runtime.goparkunlock(...)
		  | 	/usr/local/go/src/runtime/proc.go:312
		  | runtime.runfinq()
		  | 	/usr/local/go/src/runtime/mfinal.go:175 +0xa9 fp=0xc0000aa7e0 sp=0xc0000aa758 pc=0x46d529
		  | runtime.goexit()
		  | 	/usr/local/go/src/runtime/asm_amd64.s:1374 +0x1 fp=0xc0000aa7e8 sp=0xc0000aa7e0 pc=0x4c4641
		  | created by runtime.createfing
		  | 	/usr/local/go/src/runtime/mfinal.go:156 +0x65
		  |
		  | goroutine 19 [chan receive]:
		  | runtime.gopark(0x518f038, 0xc0000d60b8, 0xc0000a170e, 0x2)
		  | 	/usr/local/go/src/runtime/proc.go:306 +0xe5 fp=0xc0000a66a8 sp=0xc0000a6688 pc=0x48e205
		  | runtime.chanrecv(0xc0000d6060, 0xc0000a67b8, 0xf7c601, 0xc0000a67a0)
		  | 	/usr/local/go/src/runtime/chan.go:577 +0x36f fp=0xc0000a6738 sp=0xc0000a66a8 pc=0x45a38f
		  | runtime.chanrecv2(0xc0000d6060, 0xc0000a67b8, 0x0)
		  | 	/usr/local/go/src/runtime/chan.go:444 +0x2b fp=0xc0000a6768 sp=0xc0000a6738 pc=0x45a00b
		  | github.com/cockroachdb/cockroach/pkg/util/log.flushDaemon()
		  | 	/go/src/github.com/cockroachdb/cockroach/pkg/util/log/log_flush.go:75 +0x73 fp=0xc0000a67e0 sp=0xc0000a6768 pc=0xf75693
		  | runtime.goexit()
		  | 	/usr/local/go/src/runtime/asm_amd64.s:1374 +0x1 fp=
		  | stdout:
		Wraps: (4) secondary error attachment
		  | signal: killed
		  | (1) signal: killed
		  | Error types: (1) *exec.ExitError
		Wraps: (5) context canceled
		Error types: (1) *withstack.withStack (2) *errutil.withPrefix (3) *main.withCommandDetails (4) *secondary.withSecondaryError (5) *errors.errorString
More

Artifacts: /clearrange/checks=true

See this test on roachdash
powered by pkg/cmd/internal/issues

craig bot pushed a commit that referenced this issue Feb 23, 2021
@mgartner @nvanbenschoten
Merge #60836 #60992
a660767 
60836: opt: support UPDATE with partial UNIQUE WITHOUT INDEX constraints r=mgartner a=mgartner

This commit add uniqueness checks for partial `UNIQUE WITHOUT INDEX`
constraints during `UPDATE` statements.

As partial of this change, I discovered that #60535 introduced a
regression where columns not required by uniqueness checks are not
pruned. I've left TODOs in the column pruning tests and plan on fixing
this in a follow-up PR.

There is no release note because these constraints are gated behind the
experimental_enable_unique_without_index_constraints session variable.

Release note: None

60992: kv: make RaftCommand.ClosedTimestamp nullable r=nvanbenschoten a=nvanbenschoten

Fixes #60852.
Fixes #60833.
Fixes #58298.
Fixes #59428.
Fixes #60756.
Fixes #60848.
Fixes #60849.

In #60852 and related issues, we saw that the introduction of a non-nullable `RaftCommand.ClosedTimestamp`, coupled with the `ClosedTimestampFooter` encoding strategy we use, led to encoded `RaftCommand` protos with their ClosedTimestamp field set twice. This is ok from a correctness perspective, at least as protobuf is concerned, but it led to a subtle interaction where the process of passing through sideloading (`maybeInlineSideloadedRaftCommand(maybeSideloadEntriesImpl(e))`) would reduce the size of an encoded RaftCommand by 3 bytes (the encoded size of an empty `hlc.Timestamp`). This was resulting in an `uncommittedSize` leak in Raft, which was eventually stalling on its `MaxUncommittedEntriesSize` limit.

This commit fixes this issue by making `RaftCommand.ClosedTimestamp` nullable. With the field marked as nullable, it will no longer be encoded as an empty timestamp when unset, ensuring that when the encoded `ClosedTimestampFooter` is appended, it contains the only instance of the `ClosedTimestamp` field.

cc. @cockroachdb/bulk-io 

Co-authored-by: Marcus Gartner <marcus@cockroachlabs.com>
Co-authored-by: Nathan VanBenschoten <nvanbenschoten@gmail.com>
@craig craig bot closed this as completed in a2b9c19 Feb 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@nvanbenschoten nvanbenschoten

Labels
branch-master Failures and bugs on the master branch. C-test-failure Broken test (automatically or manually discovered). O-roachtest O-robot Originated from a bot. release-blocker Indicates a release-blocker. Use with branch-release-2x.x label to denote which branch is blocked.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

kv: make RaftCommand.ClosedTimestamp nullable nvanbenschoten/cockroach
2 participants
@nvanbenschoten @cockroach-teamcity