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

Add e2e test to reproduce issue #19406 #19608

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

miancheng7
Copy link

@miancheng7 miancheng7 commented Mar 15, 2025

PR Description

This is a follow-up to issue #19406.

The fix in #19405 has been merged, and this PR introduces an e2e test to reproduce the issue. The goal is to ensure that the test will catch the problem if it resurfaces in the future.

Testing

I have tested this locally and confirmed that the test:
Passes consistently with the fix applied.
Fails reliably if the fix in #19405 is rolled back.

% go test -v -run TestReproduce19406
=== RUN   TestReproduce19406
    testing.go:25: Changing working directory to: /tmp/TestReproduce194063444741838/001
    logger.go:146: 2025-03-14T23:56:18.083Z	INFO	starting server...	{"name": "TestReproduce19406-test-0"}
......
    logger.go:146: 2025-03-14T23:56:29.739Z	INFO	stopping server...	{"name": "TestReproduce19406-test-0"}
    logger.go:146: 2025-03-14T23:56:29.777Z	INFO	stopped server.	{"name": "TestReproduce19406-test-0"}
--- PASS: TestReproduce19406 (11.70s)
PASS
ok  	go.etcd.io/etcd/tests/v3/e2e	11.714s


# rollback the fix and run it again
% go test -v -run TestReproduce19406
=== RUN   TestReproduce19406
    testing.go:25: Changing working directory to: /tmp/TestReproduce194064120080253/001
......
    logger.go:146: 2025-03-14T23:54:38.079Z	INFO	started server.	{"name": "TestReproduce19406-test-0", "pid": 30796}
    reproduce_19406_test.go:57: start compaction...
    reproduce_19406_test.go:99: Test failed: put latency is larger than 100ms
    reproduce_19406_test.go:99: Test failed: put latency is larger than 100ms
......
    logger.go:146: 2025-03-14T23:54:43.592Z	INFO	stopping server...	{"name": "TestReproduce19406-test-0"}
    logger.go:146: 2025-03-14T23:54:43.605Z	INFO	stopped server.	{"name": "TestReproduce19406-test-0"}
--- FAIL: TestReproduce19406 (6.56s)
FAIL
exit status 1
FAIL	go.etcd.io/etcd/tests/v3/e2e	6.579s

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

@k8s-ci-robot
Copy link

Hi @miancheng7. 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-sigs/prow repository.

Copy link

codecov bot commented Mar 15, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 68.85%. Comparing base (5122d43) to head (9507b5d).
Report is 4 commits behind head on main.

Additional details and impacted files
Files with missing lines Coverage Δ
server/storage/mvcc/kvstore_compaction.go 100.00% <ø> (ø)

... and 26 files with indirect coverage changes

@@            Coverage Diff             @@
##             main   #19608      +/-   ##
==========================================
+ Coverage   68.76%   68.85%   +0.08%     
==========================================
  Files         421      421              
  Lines       35897    35897              
==========================================
+ Hits        24686    24717      +31     
+ Misses       9783     9752      -31     
  Partials     1428     1428              

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5122d43...9507b5d. Read the comment docs.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@chaochn47
Copy link
Member

/ok-to-test

t.Cleanup(func() { require.NoError(t, clus.Stop()) })

// produce some data
cli := newClient(t, clus.EndpointsGRPC(), e2e.ClientConfig{})
Copy link
Member

Choose a reason for hiding this comment

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

Can reuse the v3 client in generateTrafficAndVerifyPutLatency?

@miancheng7 miancheng7 force-pushed the e2etestforissue19406 branch from 320b1b0 to 16dc8de Compare March 15, 2025 16:41
@miancheng7 miancheng7 force-pushed the e2etestforissue19406 branch 2 times, most recently from 8c4d3fc to 8563cb0 Compare March 16, 2025 00:38
@miancheng7 miancheng7 force-pushed the e2etestforissue19406 branch from 8563cb0 to 9507b5d Compare March 16, 2025 00:51
@k8s-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: chaochn47, miancheng7
Once this PR has been reviewed and has the lgtm label, please assign fuweid for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@chaochn47
Copy link
Member

cc @ahrtr @serathius @fuweid for a stamp of approval, thanks!

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.

3 participants