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

etcdserver: separate "raft log compact" from snapshot #18235

Closed
wants to merge 8 commits into from
Closed

etcdserver: separate "raft log compact" from snapshot #18235

Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
1998a71
etcdserver: separate compact from snapshot
clement2026 Jun 26, 2024
8dc2a42
etcdserver: skip compaction if there is an inflight snapshot
clement2026 Jun 26, 2024
641eb41
etcdserver: skip compaction if there are snapshots being created
clement2026 Jun 27, 2024
93d0484
etcdserver: compact the raft log up to the minimum snapshot index of …
clement2026 Jun 28, 2024
302254b
fix lint errorgs
clement2026 Jun 28, 2024
fcb154f
fix vet errors
clement2026 Jun 28, 2024
99b3806
fix test cases
clement2026 Jun 28, 2024
5745418
add license header
clement2026 Jun 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 13 additions & 5 deletions server/etcdserver/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package etcdserver
import (
"context"
"encoding/json"
goerrors "errors"
"expvar"
"fmt"
"math"
Expand Down Expand Up @@ -963,6 +964,7 @@ func (s *EtcdServer) applyAll(ep *etcdProgress, apply *toApply) {
<-apply.notifyc

s.triggerSnapshot(ep)
s.compactRaftLog(ep.appliedi)
select {
// snapshot requested via send()
case m := <-s.r.msgSnapC:
Expand Down Expand Up @@ -2164,18 +2166,24 @@ func (s *EtcdServer) snapshot(snapi uint64, confState raftpb.ConfState) {
lg.Info("skip compaction since there is an inflight snapshot")
return
}
})
}

func (s *EtcdServer) compactRaftLog(appliedi uint64) {
s.GoAttach(func() {
lg := s.Logger()

// keep some in memory log entries for slow followers.
compacti := uint64(1)
if snapi > s.Cfg.SnapshotCatchUpEntries {
compacti = snapi - s.Cfg.SnapshotCatchUpEntries
compacti := uint64(0)
Copy link
Member

Choose a reason for hiding this comment

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

Why change from 1 to 0?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://github.com/etcd-io/raft/blob/ee19bbe7f9bb16fd7d874a6c08dfac319596d2ad/storage.go#L258-L260

	if compactIndex > ms.lastIndex() {
		getLogger().Panicf("compact %d is out of bound lastindex(%d)", compactIndex, ms.lastIndex())
	}

A panic occurred at this line during local test runs. It appears the initial value of ms.lastIndex() is 0, so compacti should default to zero.

In the main branch, a panic doesn't occur because ms always contains more than one element when compaction starts, which isn't the case in this patch.

if appliedi > s.Cfg.SnapshotCatchUpEntries {
compacti = appliedi - s.Cfg.SnapshotCatchUpEntries
}

err = s.r.raftStorage.Compact(compacti)
err := s.r.raftStorage.Compact(compacti)
if err != nil {
// the compaction was done asynchronously with the progress of raft.
// raft log might already been compact.
if err == raft.ErrCompacted {
if goerrors.Is(err, raft.ErrCompacted) {
return
}
lg.Panic("failed to compact", zap.Error(err))
Expand Down
Loading