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

kv/kvserver: TestReplicateRange failed #57560

Closed
cockroach-teamcity opened this issue Dec 4, 2020 · 2 comments · Fixed by #56812
Closed

kv/kvserver: TestReplicateRange failed #57560

cockroach-teamcity opened this issue Dec 4, 2020 · 2 comments · Fixed by #56812
Assignees
@tbg
Labels
branch-master Failures and bugs on the master branch. C-test-failure Broken test (automatically or manually discovered). O-robot Originated from a bot.

Comments

@cockroach-teamcity
Copy link
Member

(kv/kvserver).TestReplicateRange failed on master@a1759d1a6938b50fcd59f71258b97b6299c0960c:

=== RUN   TestReplicateRange
    test_log_scope.go:72: test logs captured to: /go/src/github.com/cockroachdb/cockroach/artifacts/logTestReplicateRange572431741
    test_log_scope.go:73: use -show-logs to present logs inline
    client_raft_test.go:297: change replicas of r1 failed: descriptor changed: [expected] r1:/M{in-ax} [(n1,s1):1, (n2,s2):2LEARNER, next=3, gen=1] != [actual] r1:/M{in-ax} [(n1,s1):1, next=3, gen=2]
    panic.go:617: -- test log scope end --
test logs left over in: /go/src/github.com/cockroachdb/cockroach/artifacts/logTestReplicateRange572431741
--- FAIL: TestReplicateRange (0.11s)
More

Parameters:

  • GOFLAGS=-json
make stressrace TESTS=TestReplicateRange PKG=./pkg/kv/kvserver TESTTIMEOUT=5m STRESSFLAGS='-timeout 5m' 2>&1

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-robot Originated from a bot. labels Dec 4, 2020
@cockroach-teamcity
Copy link
Member Author

(kv/kvserver).TestReplicateRange failed on master@09aa7c869cd0e90982b90fefb7f140ea6e460037:

=== RUN   TestReplicateRange
    test_log_scope.go:72: test logs captured to: /go/src/github.com/cockroachdb/cockroach/artifacts/logTestReplicateRange497000489
    test_log_scope.go:73: use -show-logs to present logs inline
    client_raft_test.go:297: change replicas of r1 failed: descriptor changed: [expected] r1:/M{in-ax} [(n1,s1):1, (n2,s2):2LEARNER, next=3, gen=1] != [actual] r1:/M{in-ax} [(n1,s1):1, next=3, gen=2]
    panic.go:617: -- test log scope end --
test logs left over in: /go/src/github.com/cockroachdb/cockroach/artifacts/logTestReplicateRange497000489
--- FAIL: TestReplicateRange (0.18s)
More

Parameters:

  • GOFLAGS=-json
make stressrace TESTS=TestReplicateRange PKG=./pkg/kv/kvserver TESTTIMEOUT=5m STRESSFLAGS='-timeout 5m' 2>&1

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

@cockroach-teamcity
Copy link
Member Author

(kv/kvserver).TestReplicateRange failed on master@2807d09091584a0224c2a6f9c9ec99db6fe490c6:

=== RUN   TestReplicateRange
    test_log_scope.go:72: test logs captured to: /go/src/github.com/cockroachdb/cockroach/artifacts/logTestReplicateRange815238612
    test_log_scope.go:73: use -show-logs to present logs inline
    client_raft_test.go:297: change replicas of r1 failed: descriptor changed: [expected] r1:/M{in-ax} [(n1,s1):1, (n2,s2):2LEARNER, next=3, gen=1] != [actual] r1:/M{in-ax} [(n1,s1):1, next=3, gen=2]
    panic.go:617: -- test log scope end --
test logs left over in: /go/src/github.com/cockroachdb/cockroach/artifacts/logTestReplicateRange815238612
--- FAIL: TestReplicateRange (0.16s)
More

Parameters:

  • GOFLAGS=-json
make stressrace TESTS=TestReplicateRange PKG=./pkg/kv/kvserver TESTTIMEOUT=5m STRESSFLAGS='-timeout 5m' 2>&1

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

jbowens added a commit to jbowens/cockroach that referenced this issue Dec 8, 2020
@jbowens
kv/kvserver: skip TestReplicateRange
3daeabc 
Refs: cockroachdb#57560

Reason: flaky test

Generated by bin/skip-test.

Release justification: non-production code changes

Release note: None
@jbowens jbowens mentioned this issue Dec 8, 2020
Merged
@irfansharif irfansharif assigned tbg and unassigned bdarnell Dec 8, 2020
craig bot pushed a commit that referenced this issue Dec 8, 2020
@jbowens
Merge #57708
d33024a 
57708: kv/kvserver: skip TestReplicateRange r=irfansharif a=jbowens

Refs: #57560

Reason: flaky test

Generated by bin/skip-test.

Release justification: non-production code changes

Release note: None

Co-authored-by: Jackson Owens <jackson@cockroachlabs.com>
lunevalex added a commit to lunevalex/cockroach that referenced this issue Jan 14, 2021
@lunevalex
kvserver: replace multiTestContext with TestCluster in client_raft_test
fe55907 
Makes progress on cockroachdb#8299
Fixes cockroachdb#40351
Fixes cockroachdb#57560
Fixes cockroachdb#57537

multiTestContext is a legacy construct that is deprecated in favor of running
tests via TestCluster. This is one PR out of many to remove the usage of
multiTestContext in the client_raft test cases. This does not remove all the
uses of mtc, just the simple ones. Leaving the more complex uses cases for a later PR.

With this switch we can also clean up some TestingKnobs and TestServer interfaces.
    - DisablePeriodicGossips flag is removed, it does not work with TestCluster
      and is no longer used
    - DontPreventUseOfOldLeaseOnStart flag is removed, it did not work consistently
      in TestCluster. This flag tries to leave the Lease on the same node after a
      restart, but CRDB makes no such guarantees in the real world and artificially
      testing it does not prove anything. The affected tests were re-worked to
      not rely on this condition and can deal with a lease holder moving on a restart.
    - GetRaftLeader is ported from multiTestContext to TestCluster

Release note: None
@lunevalex lunevalex mentioned this issue Jan 16, 2021
Merged
craig bot pushed a commit that referenced this issue Jan 19, 2021
@lunevalex @rafiss
@irfansharif @RichardJCai @yuzefovich
Merge #56812 #58265 #58902 #59049 #59055
47eb9f3 
56812: kvserver: replace multiTestContext with TestCluster in client_raft_test r=lunevalex a=lunevalex

Makes progress on #8299
Fixes #40351
Fixes #57560
Fixes #57537

multiTestContext is a legacy construct that is deprecated in favor of running
tests via TestCluster. This is one PR out of many to remove the usage of
multiTestContext in the client_raft test cases. This does not remove all the
uses of mtc, just the simple ones. Leaving the more complex uses cases for a later PR.

With this switch we can also clean up some TestingKnobs and TestServer interfaces.
    - DisablePeriodicGossips flag is removed, it does not work with TestCluster
      and is no longer used
    - DontPreventUseOfOldLeaseOnStart flag is removed, it did not work consistently
      in TestCluster. This flag tries to leave the Lease on the same node after a
      restart, but CRDB makes no such guarantees in the real world and artificially
      testing it does not prove anything. The affected tests were re-worked to
      not rely on this condition and can deal with a lease holder moving on a restart.
    - GetRaftLeader is ported from multiTestContext to TestCluster

Release note: None


58265: sql: fix substring(byte[]) to treat input as raw bytes without escaping r=solongordon a=rafiss

fixes #57367 

Release note (bug fix): The substring function on byte arrays would
treat its input as unicode code points, which would cause the wrong
bytes to be returned. Now it only operates on the raw bytes.

Release note (bug fix): The substring(byte[]) functions were not able to
interpret bytes that had the `\` character since it was treating it as
the beginning of an escape sequence. This is now fixed.

58902: tracing,testutils: detect span leaks r=irfansharif a=irfansharif

With always-on tracing, we're maintaining in-memory registry of active
spans (#58490). Spans are added and removed from this registry when
they're Start()-ed and Finish()-ed. Spans that not explicitly finished
(typically using `defer sp.Finish()`) are now a resource-leak, as they
take up room in the registry. We'll want to find instances of this leak
as soon as they crop up.

To that end we add a check in our TestCluster.Stop codepaths that
asserts against the registry being empty. This should give us wide
coverage given it's usage throughout. We expect this change to capture
the cases described in #58721.

---

This check is currently failing, given we're actually leaking spans in
`txnState.resetForNewSQLTxn`. Fixing that doesn't look simple, it ties
into questions around draining SQL and rolling back open txns.

Release note: None

59049: sql: implement alter sequence/view owner r=arulajmani a=RichardJCai

Resolves #57965

Release note (sql change): Add support for ALTER VIEW/SEQUENCE OWNER TO commands.

59055: colexec: fix external aggregator fallback and bool agg functions reset r=yuzefovich a=yuzefovich

Previously, `reset` method of the ordered aggregator would always set
the flag to reset the internal batch to `true`. However, that batch is
only allocated when `Next` is called at least once with a non-zero batch
coming from the input, which is not the case when the fallback to the
disk-backed strategy occurs in the external aggregator (there, we call
`reset` before we use the operator every time). This would lead to
a nil pointer exception, and it is now fixed.

Our unit tests didn't catch it because we forgot to set the forced
number of repartitions which is now also fixed.

This also revealed a bug with resetting of `bool_and` and `bool_or`
aggregates - we forgot to reset whether they have seen a non-null value
or not.

Fixes: #59043.

Release note: None (no stable release with these bugs)

Co-authored-by: Alex Lunev <alexl@cockroachlabs.com>
Co-authored-by: Rafi Shamim <rafi@cockroachlabs.com>
Co-authored-by: irfan sharif <irfanmahmoudsharif@gmail.com>
Co-authored-by: richardjcai <caioftherichard@gmail.com>
Co-authored-by: Yahor Yuzefovich <yahor@cockroachlabs.com>
@craig craig bot closed this as completed in 3876b44 Jan 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@tbg tbg

Labels
branch-master Failures and bugs on the master branch. C-test-failure Broken test (automatically or manually discovered). O-robot Originated from a bot.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

kvserver: replace multiTestContext with TestCluster in client_raft_test lunevalex/cockroach
3 participants
@bdarnell @tbg @cockroach-teamcity