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

Linearizability raft before leader send #14752

Merged
merged 2 commits into from
Nov 18, 2022

Conversation

serathius
Copy link
Member

Part of #14726
cc @ahrtr

@ahrtr
Copy link
Member

ahrtr commented Nov 14, 2022

Please resolve the pipeline failures.

@serathius serathius force-pushed the linearizability-raftBeforeLeaderSend branch 3 times, most recently from 194a010 to 20e19df Compare November 14, 2022 18:26
@serathius
Copy link
Member Author

@ahrtr done

tests/framework/e2e/cluster.go Outdated Show resolved Hide resolved
tests/framework/e2e/cluster.go Outdated Show resolved Hide resolved
tests/framework/e2e/cluster.go Outdated Show resolved Hide resolved
tests/framework/e2e/cluster.go Show resolved Hide resolved
@serathius serathius force-pushed the linearizability-raftBeforeLeaderSend branch from 20e19df to 22ee2e6 Compare November 15, 2022 13:34
@codecov-commenter
Copy link

Codecov Report

Merging #14752 (22ee2e6) into main (a699764) will increase coverage by 0.06%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main   #14752      +/-   ##
==========================================
+ Coverage   75.45%   75.52%   +0.06%     
==========================================
  Files         457      457              
  Lines       37372    37372              
==========================================
+ Hits        28200    28225      +25     
+ Misses       7401     7368      -33     
- Partials     1771     1779       +8     
Flag Coverage Δ
all 75.52% <ø> (+0.06%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
client/v3/leasing/util.go 91.66% <0.00%> (-6.67%) ⬇️
client/v3/concurrency/session.go 88.63% <0.00%> (-4.55%) ⬇️
server/etcdserver/txn/util.go 75.47% <0.00%> (-3.78%) ⬇️
server/etcdserver/cluster_util.go 70.35% <0.00%> (-3.17%) ⬇️
server/etcdserver/api/v3rpc/interceptor.go 74.47% <0.00%> (-3.13%) ⬇️
client/v3/experimental/recipes/double_barrier.go 68.83% <0.00%> (-2.60%) ⬇️
server/etcdserver/api/rafthttp/msgappv2_codec.go 67.82% <0.00%> (-1.74%) ⬇️
server/etcdserver/api/v3rpc/key.go 82.19% <0.00%> (-1.37%) ⬇️
server/proxy/grpcproxy/lease.go 88.23% <0.00%> (-0.91%) ⬇️
client/v3/leasing/cache.go 87.22% <0.00%> (-0.56%) ⬇️
... and 13 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Signed-off-by: Marek Siarkowicz <siarkowicz@google.com>
@serathius serathius force-pushed the linearizability-raftBeforeLeaderSend branch from 22ee2e6 to f002c49 Compare November 17, 2022 09:50
Signed-off-by: Marek Siarkowicz <siarkowicz@google.com>
@serathius serathius force-pushed the linearizability-raftBeforeLeaderSend branch from f002c49 to 371179e Compare November 17, 2022 13:17
@serathius
Copy link
Member Author

ping @ahrtr @ptabor

Comment on lines +36 to +54
DefragBeforeCopyPanic Failpoint = goFailpoint{"backend/defragBeforeCopy", "panic", triggerDefrag, AnyMember}
DefragBeforeRenamePanic Failpoint = goFailpoint{"backend/defragBeforeRename", "panic", triggerDefrag, AnyMember}
BeforeCommitPanic Failpoint = goFailpoint{"backend/beforeCommit", "panic", nil, AnyMember}
AfterCommitPanic Failpoint = goFailpoint{"backend/afterCommit", "panic", nil, AnyMember}
RaftBeforeSavePanic Failpoint = goFailpoint{"etcdserver/raftBeforeSave", "panic", nil, AnyMember}
RaftAfterSavePanic Failpoint = goFailpoint{"etcdserver/raftAfterSave", "panic", nil, AnyMember}
BackendBeforePreCommitHookPanic Failpoint = goFailpoint{"backend/commitBeforePreCommitHook", "panic", nil, AnyMember}
BackendAfterPreCommitHookPanic Failpoint = goFailpoint{"backend/commitAfterPreCommitHook", "panic", nil, AnyMember}
BackendBeforeStartDBTxnPanic Failpoint = goFailpoint{"backend/beforeStartDBTxn", "panic", nil, AnyMember}
BackendAfterStartDBTxnPanic Failpoint = goFailpoint{"backend/afterStartDBTxn", "panic", nil, AnyMember}
BackendBeforeWritebackBufPanic Failpoint = goFailpoint{"backend/beforeWritebackBuf", "panic", nil, AnyMember}
BackendAfterWritebackBufPanic Failpoint = goFailpoint{"backend/afterWritebackBuf", "panic", nil, AnyMember}
CompactBeforeCommitScheduledCompactPanic Failpoint = goFailpoint{"mvcc/compactBeforeCommitScheduledCompact", "panic", triggerCompact, AnyMember}
CompactAfterCommitScheduledCompactPanic Failpoint = goFailpoint{"mvcc/compactAfterCommitScheduledCompact", "panic", triggerCompact, AnyMember}
CompactBeforeSetFinishedCompactPanic Failpoint = goFailpoint{"mvcc/compactBeforeSetFinishedCompact", "panic", triggerCompact, AnyMember}
CompactAfterSetFinishedCompactPanic Failpoint = goFailpoint{"mvcc/compactAfterSetFinishedCompact", "panic", triggerCompact, AnyMember}
CompactBeforeCommitBatchPanic Failpoint = goFailpoint{"mvcc/compactBeforeCommitBatch", "panic", triggerCompact, AnyMember}
CompactAfterCommitBatchPanic Failpoint = goFailpoint{"mvcc/compactAfterCommitBatch", "panic", triggerCompact, AnyMember}
RaftBeforeLeaderSendPanic Failpoint = goFailpoint{"etcdserver/raftBeforeLeaderSend", "panic", nil, Leader}
Copy link
Member

Choose a reason for hiding this comment

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

There are too many duplicated parameters, e.g. nil, AnyMember. We should only provide a value when it isn't nil or AnyMember. Let's consider to refactor it later in separate PRs.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agree, I'm thinking about how to combine this with allowing non-panic failpoints

Copy link
Member

@ahrtr ahrtr left a comment

Choose a reason for hiding this comment

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

Overall looks good to me.

@ahrtr ahrtr merged commit 2742bdc into etcd-io:main Nov 18, 2022
@serathius serathius deleted the linearizability-raftBeforeLeaderSend branch June 15, 2023 20:37
ivanvc added a commit to ivanvc/etcd that referenced this pull request Feb 20, 2024
Fixes the incorrect backport of WaitLeader, as spawnJsonCmd expects an
expected output string as argument, which wasn't provided in the first
backport implementation.

Original backport of commit 371179e from
PR etcd-io#14752.

Signed-off-by: Ivan Valdes <ivan@vald.es>
ivanvc added a commit to ivanvc/etcd that referenced this pull request Feb 20, 2024
Fixes the incorrect backport of WaitLeader, as spawnJsonCmd expects an
expected output string as argument, which wasn't provided in the first
backport implementation.

Original backport of commit 371179e from
PR etcd-io#14752.

Signed-off-by: Ivan Valdes <ivan@vald.es>
ivanvc added a commit to ivanvc/etcd that referenced this pull request Feb 20, 2024
Fixes the incorrect backport of WaitLeader, as spawnJsonCmd receives an
expected output string as an argument, which wasn't provided in the
first backport implementation.

Original backport of commit 371179e from
PR etcd-io#14752.

Signed-off-by: Ivan Valdes <ivan@vald.es>
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 participants