-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
roachtest: backup/KMS/n3cpu4 failed #60833
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
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 20, 2021
(roachtest).backup/KMS/n3cpu4 failed on master@64c4aef909f4382523cd9248341ca9f4448d841a:
|
(roachtest).backup/KMS/n3cpu4 failed on master@bf9744bad5a416a4b06907f0f3dd42896f7342f3:
|
nvanbenschoten
added a commit
to nvanbenschoten/cockroach
that referenced
this issue
Feb 23, 2021
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.
(roachtest).backup/KMS/n3cpu4 failed on master@5cfd7e5553a3072a1490d392390dddf968844215:
|
craig bot
pushed a commit
that referenced
this issue
Feb 23, 2021
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 <[email protected]> Co-authored-by: Nathan VanBenschoten <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
(roachtest).backup/KMS/n3cpu4 failed on master@8b6f3c84cc256debeeb4d4055c5f0d5c9a481213:
More
Artifacts: /backup/KMS/n3cpu4
See this test on roachdash
powered by pkg/cmd/internal/issues
The text was updated successfully, but these errors were encountered: