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

receipt: dequeue not returning watch err #17198

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

wxsms
Copy link

@wxsms wxsms commented Jan 4, 2024

the error from watch is not returned, if the client token expired, the ev return by watch will be nil, therefore the Deque method will panic in such case.

Please read https://github.com/etcd-io/etcd/blob/main/CONTRIBUTING.md#contribution-flow.

@k8s-ci-robot
Copy link

Hi @wxsms. Thanks for your PR.

I'm waiting for a etcd-io member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@serathius
Copy link
Member

Can you add a test?

@wxsms
Copy link
Author

wxsms commented Jan 10, 2024

Hi there, Thanks for reply, I just added a test to simulate an error during watch by shutting down a cluster, since I'm very new to etcd, not sure if I make it correctly. The main propose of this PR is to make sure watch return a non-nil value or error. The testcase will failed as followed if queue code stay untouched:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x8 pc=0xad0bb6]

goroutine 169 [running]:
go.etcd.io/etcd/client/v3/experimental/recipes.(*Queue).Dequeue(0xc00045b290)
        /workspaces/etcd/client/v3/experimental/recipes/queue.go:70 +0x1f6
go.etcd.io/etcd/tests/v3/integration/clientv3/experimental/recipes_test.TestQueueWatchError.func1()
        /workspaces/etcd/tests/integration/clientv3/experimental/recipes/v3_queue_test.go:92 +0x25
created by go.etcd.io/etcd/tests/v3/integration/clientv3/experimental/recipes_test.TestQueueWatchError in goroutine 64
        /workspaces/etcd/tests/integration/clientv3/experimental/recipes/v3_queue_test.go:91 +0x153
FAIL    go.etcd.io/etcd/tests/v3/integration/clientv3/experimental/recipes      1.107s

the error from watch is not returned, if the client token expired, the `ev` return by watch will be nil, therefore the Deque method will panic in such case.

Signed-off-by: guokairui <wxsms@foxmail.com>
Signed-off-by: guokairui <wxsms@foxmail.com>
@wxsms
Copy link
Author

wxsms commented Jan 15, 2024

Sry but I have no idea why integration/clientv3 TestMaintenanceSnapshotCancel failing, every thing's fine while running tests inside my codespace, including this one. Can anyone help to take a look?

image

@jmhbnz
Copy link
Member

jmhbnz commented Feb 21, 2024

/retest

@serathius
Copy link
Member

/retest

@siyuanfoundation
Copy link
Contributor

cc @moficodes

@serathius
Copy link
Member

/retest

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

Successfully merging this pull request may close these issues.

5 participants