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.

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

etcdserver: Fix invalid count returned on Range with Limit #13060

Merged
merged 1 commit into from
Jun 1, 2021

Conversation

serathius
Copy link
Member

@serathius serathius commented May 31, 2021

Fixes #13056

PR #11990 introduced a breaking change where Count would no longer count all keys under range, but only up to the Limit. This change is a breaking change in behavior and is blocking Kubernetes from using 3.5 release as this breaks RemainingItemCount in K8s API.

@serathius serathius changed the title etcdserver: WIP fix invalid count returned on Range with Limit etcdserver: Fix invalid count returned on Range with Limit May 31, 2021
@serathius
Copy link
Member Author

cc @ptabor @tangcong

@serathius serathius force-pushed the limit branch 3 times, most recently from 6593262 to a8a738a Compare May 31, 2021 16:32
@ptabor
Copy link
Contributor

ptabor commented May 31, 2021

There are some flakes

    leak.go:118: Test appears to have leaked a Transport:
        github.com/soheilhy/cmux.muxListener.Accept(0xce1b50, 0x4005c1d680, 0x400036d440, 0x4000814e40, 0x40003439a0, 0x4000343a28, 0x4000fde601, 0x40003439a0)
        	/home/ec2-user/go/pkg/mod/github.com/soheilhy/cmux@v0.1.5/cmux.go:262 +0x68
        google.golang.org/grpc.(*Server).Serve(0x400078a1c0, 0xce2e40, 0x4009efd020, 0x0, 0x0)
        	/home/ec2-user/go/pkg/mod/google.golang.org/grpc@v1.37.0/server.go:739 +0x290
        created by go.etcd.io/etcd/tests/v3/integration.(*member).Launch
        	/home/ec2-user/actions-runner/_work/etcd/etcd/tests/integration/cluster.go:887 +0x1490
        
        github.com/soheilhy/cmux.muxListener.Accept(0xce1b50, 0x4005c1d680, 0x400036d500, 0x4000814ea0, 0x4000908df8, 0x515f4, 0x4000fdbe48, 0x2ee4f8)
        	/home/ec2-user/go/pkg/mod/github.com/soheilhy/cmux@v0.1.5/cmux.go:262 +0x68
        net/http.(*Server).Serve(0x40008a2380, 0xce2e40, 0x4009efd040, 0x0, 0x0)

I don't know why we don't close the cluster. One of the options is that we hit:

t.Fatalf("#%d: couldn't put key (%v)", i, err)

so we don't execute: clus.Terminate(t) that is not defered.

Maybe it would be easier if the body of the method into:

func restV3RangeRequestFor(t *testing.T, tt struct {
	putKeys []string
	reqs    []pb.RangeRequest
	wresps  [][]string
	wmores  []bool
}) {

@serathius
Copy link
Member Author

There was small error in test cases I added, it should pass now. Still it's worrying that test failures are so unreadable, I would be happy to improve this.

@ptabor
Copy link
Contributor

ptabor commented Jun 1, 2021

Thank you for fixing it. Please prepare a backport PR (for branch release-3.5)

@serathius
Copy link
Member Author

Backport PR created #13064

ptabor added a commit that referenced this pull request Jun 1, 2021
@wilsonwang371
Copy link
Contributor

Most likely this is the reason for a sudden performance drop in my test script. #13068

Do we have any other solution to this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3.5: k8s API chunking tests flakes
3 participants