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

[release-3.6] Auto sync members in v3store is IsLearner differs between v2 and v3 store #19636

Merged
merged 1 commit into from
Mar 24, 2025

Conversation

ahrtr
Copy link
Member

@ahrtr ahrtr commented Mar 21, 2025

release-3.6 isn't able to automatically fix the issues which have already been affected by #19557.

Also adding the validation has flaws as mentioned in #19557 (comment)

Eventually I tried to add similar auto sync in 3.6 as what we did for 3.5 in #19606

This is the only way to avoid all confusion and reduce of user experience.

cc @fuweid @serathius @siyuanfoundation

Copy link

codecov bot commented Mar 21, 2025

Codecov Report

Attention: Patch coverage is 54.90196% with 23 lines in your changes missing coverage. Please review.

Project coverage is 68.98%. Comparing base (f9df7c6) to head (de774e7).
Report is 4 commits behind head on release-3.6.

Files with missing lines Patch % Lines
server/etcdserver/api/membership/cluster.go 69.23% 11 Missing and 1 partial ⚠️
server/storage/schema/membership.go 0.00% 9 Missing ⚠️
server/storage/backend/verify.go 0.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
Files with missing lines Coverage Δ
server/etcdserver/api/membership/store.go 60.00% <ø> (ø)
server/etcdserver/bootstrap.go 63.67% <100.00%> (+0.08%) ⬆️
server/storage/backend/verify.go 84.61% <0.00%> (-3.39%) ⬇️
server/storage/schema/membership.go 56.16% <0.00%> (-3.69%) ⬇️
server/etcdserver/api/membership/cluster.go 87.29% <69.23%> (-1.31%) ⬇️

... and 25 files with indirect coverage changes

@@               Coverage Diff               @@
##           release-3.6   #19636      +/-   ##
===============================================
+ Coverage        68.87%   68.98%   +0.11%     
===============================================
  Files              420      420              
  Lines            35787    35827      +40     
===============================================
+ Hits             24648    24717      +69     
+ Misses            9705     9689      -16     
+ Partials          1434     1421      -13     

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 f9df7c6...de774e7. Read the comment docs.

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

@ahrtr ahrtr force-pushed the 3.6_learner_sync_20250321 branch from d01d8f2 to c120a3b Compare March 21, 2025 16:13
@ahrtr ahrtr force-pushed the 3.6_learner_sync_20250321 branch from 546e38c to 507f6d6 Compare March 21, 2025 20:18
@siyuanfoundation
Copy link
Contributor

/lgtm

Copy link
Member

@fuweid fuweid left a comment

Choose a reason for hiding this comment

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

LGTM

@ahrtr ahrtr force-pushed the 3.6_learner_sync_20250321 branch from 507f6d6 to e7a2bd7 Compare March 21, 2025 21:47
@k8s-ci-robot k8s-ci-robot removed the lgtm label Mar 21, 2025
@k8s-ci-robot
Copy link

New changes are detected. LGTM label has been removed.

@ahrtr ahrtr force-pushed the 3.6_learner_sync_20250321 branch from e7a2bd7 to 1a03d4d Compare March 21, 2025 21:50
@ahrtr
Copy link
Member Author

ahrtr commented Mar 21, 2025

cc @siyuanfoundation @serathius

@serathius
Copy link
Member

release-3.6 isn't able to automatically fix the issues which have already been affected by #19557.

What do you mean by that?

@serathius
Copy link
Member

Left couple of comments apart of that LGTM.

@ahrtr ahrtr force-pushed the 3.6_learner_sync_20250321 branch from 1a03d4d to de774e7 Compare March 24, 2025 10:26
@ahrtr
Copy link
Member Author

ahrtr commented Mar 24, 2025

release-3.6 isn't able to automatically fix the issues which have already been affected by #19557.

What do you mean by that?

release-3.5 is able to automatically fix the already affected issues after #19602. But release-3.6 isn't able to automatically fix it, see #19557 (comment)

It's exactly the reason why we need this PR.

@k8s-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ahrtr, fuweid, serathius

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

The pull request process is described 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

@ahrtr ahrtr merged commit b4d383f into etcd-io:release-3.6 Mar 24, 2025
29 checks passed
@ahrtr ahrtr deleted the 3.6_learner_sync_20250321 branch March 24, 2025 12:49
@ivanvc ivanvc mentioned this pull request Mar 24, 2025
1 task
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