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

etcdserver: non-mutating requests pass through quotaKVServer when NOS… #13435

Merged
merged 2 commits into from
Oct 25, 2021
Merged

etcdserver: non-mutating requests pass through quotaKVServer when NOS… #13435

merged 2 commits into from
Oct 25, 2021

Conversation

chaochn47
Copy link
Member

Fix #13429

@chaochn47 chaochn47 marked this pull request as ready for review October 22, 2021 20:27
@chaochn47 chaochn47 force-pushed the kube_apiserver_delete_success_when_etcd_NOSPACE branch from 50041f2 to 817e672 Compare October 22, 2021 21:57
@chaochn47
Copy link
Member Author

Just fixed the fmt.

./test.sh -v passed locally.

gyuho
gyuho reviewed Oct 22, 2021
View reviewed changes
server/storage/quota.go Outdated
@@ -127,6 +127,10 @@ func NewBackendQuota(cfg config.ServerConfig, be backend.Backend, name string) Q
}

func (b *BackendQuota) Available(v interface{}) bool {
// if there are no mutating requests, it's safe to pass through
if b.Cost(v) == 0 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

b.Cost(v) is used twice at line 135? Can we assign it to a var?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, good call out. Fixed in the latest commits.

@chaochn47 chaochn47 force-pushed the kube_apiserver_delete_success_when_etcd_NOSPACE branch from 817e672 to 9c6d579 Compare October 22, 2021 23:39
spzala
spzala approved these changes Oct 24, 2021
View reviewed changes
CHANGELOG-3.6.md
@@ -32,6 +32,11 @@ See [code changes](https://github.com/etcd-io/etcd/compare/v3.5.0...v3.6.0).
### etcd server

- Add [`etcd --log-format`](https://github.com/etcd-io/etcd/pull/13339) flag to support log format.
- Fix [non mutating requests pass through quotaKVServer when NOSPACE](https://github.com/etcd-io/etcd/pull/13435)

### tools/benchmark
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wondering why did you change this but looking at the related PR, seems like it was incorrectly added to 3.5 :)
lgtm Thanks @chaochn47

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I made a mistake in the related PR Changelog and corrected it in this PR..

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:) that's fine and thanks for correcting it!

@chaochn47
Copy link
Member Author

chaochn47 commented Oct 25, 2021 •
edited
Loading

Looks like integration tests failed at

# linux-amd64-integration-4-cpu
=== FAIL: integration/clientv3 TestEndpointSwitchResolvesViolation (1.15s)
ordering_util_test.go:77: While speaking to partitioned leader, we should get ErrNoGreaterRev error

# linux-amd64-integration-2-cpu
=== FAIL: integration/clientv3/lease TestLeasingPutGetDeleteConcurrent (1.94s)
    leak.go:118: Test appears to have leaked :
        go.etcd.io/etcd/server/v3/etcdserver/api/v3rpc.(*serverWatchStream).recvLoop(0xc0001a8cc0, 0x0, 0x41611d)
        	/home/runner/work/etcd/etcd/server/etcdserver/api/v3rpc/watch.go:327 +0x214
        go.etcd.io/etcd/server/v3/etcdserver/api/v3rpc.(*watchServer).Watch.func2(0xc0001a8cc0, 0x127a5c0, 0xc000c3b7e0, 0xc003061320)
        	/home/runner/work/etcd/etcd/server/etcdserver/api/v3rpc/watch.go:189 +0x45
        created by go.etcd.io/etcd/server/v3/etcdserver/api/v3rpc.(*watchServer).Watch
        	/home/runner/work/etcd/etcd/server/etcdserver/api/v3rpc/watch.go:188 +0x2ec

ref. https://github.com/etcd-io/etcd/runs/3991284667
ref. https://github.com/etcd-io/etcd/runs/3982567099

Will take a look at the flaky tests. They are not related to the PR though.

@gyuho gyuho merged commit 02cdd19 into etcd-io:main Oct 25, 2021
@chaochn47 chaochn47 deleted the kube_apiserver_delete_success_when_etcd_NOSPACE branch October 25, 2021 06:04
@ahrtr ahrtr mentioned this pull request Feb 20, 2022
Merged
@chaochn47 chaochn47 mentioned this pull request Jul 21, 2022
Closed
25 tasks
ramses pushed a commit to ramses/etcd that referenced this pull request Jul 21, 2022
@chaochn47 @ramses
Backport: non mutating requests pass through quotaKVServer when NOSPACE
a3a60f0 
This is a backport of etcd-io#13435 and is
part of the work for 3.4.20
etcd-io#14232.

The original change had a second commit that modifies a changelog file.
The 3.4 branch does not include any changelog file, so that part was not
cherry-picked.

Local Testing:

- `make build`
- `make test`

Both succeed.
@ramses ramses mentioned this pull request Jul 21, 2022
Merged
ramses pushed a commit to ramses/etcd that referenced this pull request Jul 21, 2022
@chaochn47 @ramses
Backport: non mutating requests pass through quotaKVServer when NOSPACE
96f69de 
This is a backport of etcd-io#13435 and is
part of the work for 3.4.20
etcd-io#14232.

The original change had a second commit that modifies a changelog file.
The 3.4 branch does not include any changelog file, so that part was not
cherry-picked.

Local Testing:

- `make build`
- `make test`

Both succeed.

Signed-off-by: Ramsés Morales <ramses@gmail.com>
tjungblu pushed a commit to tjungblu/etcd that referenced this pull request Sep 8, 2022
@chaochn47 @tjungblu
Backport: non mutating requests pass through quotaKVServer when NOSPACE
36ffdfe 
This is a backport of etcd-io#13435 and is
part of the work for 3.4.20
etcd-io#14232.

The original change had a second commit that modifies a changelog file.
The 3.4 branch does not include any changelog file, so that part was not
cherry-picked.

Local Testing:

- `make build`
- `make test`

Both succeed.

Signed-off-by: Ramsés Morales <ramses@gmail.com>
@mehvaibh mehvaibh mentioned this pull request Dec 2, 2022
Merged
@chaochn47 chaochn47 mentioned this pull request Jul 1, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@spzala spzala spzala approved these changes

@gyuho gyuho gyuho approved these changes

@hexfusion hexfusion hexfusion approved these changes

Assignees
No one assigned
Labels
backport/v3.4 backport/v3.5
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

quotaKVServer blocks Txn-compare-get-delete (kube-apiserver delete) while allowing DeleteRange
5 participants
@chaochn47 @hexfusion @spzala @gyuho @ahrtr