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

etcdserver: keep a min number of entries in memory #2403

Merged
merged 1 commit into from
Mar 1, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 9 additions & 0 deletions etcdserver/raft.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,15 @@ import (
"github.com/coreos/etcd/wal/walpb"
)

const (
// Number of entries for slow follower to catch-up after compacting
// the raft storage entries.
// We expect the follower has a millisecond level latency with the leader.
// The max throughput is around 10K. Keep a 5K entries is enough for helping
// follower to catch up.
numberOfCatchUpEntries = 5000
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did we pick 5000 here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

add a comment for why we choose 5000.

)

var (
// indirection for expvar func interface
// expvar panics when publishing duplicate name
Expand Down
10 changes: 8 additions & 2 deletions etcdserver/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -836,8 +836,14 @@ func (s *EtcdServer) snapshot(snapi uint64, confState raftpb.ConfState) {
if err := s.r.storage.SaveSnap(snap); err != nil {
log.Fatalf("etcdserver: save snapshot error: %v", err)
}
log.Printf("etcdserver: saved snapshot at index %d", snap.Metadata.Index)

err = s.r.raftStorage.Compact(snapi)
// keep some in memory log entries for slow followers.
compacti := uint64(1)
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this mean we will always keep 1 log entry in memory? Why 1 and not say 10?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

compact 1 means do not nothing rather than keeping 1.

if the snapshot index is smaller than what we should keep, we should keep all of them.

Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we can express that fact in the comment, it's not clear what's going on here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i think the code is clear enough. and we do not comment code path in general.
you need to check the compact comment to see what is going on.

// Compact discards all log entries prior to i.
// It is the application's responsibility to not attempt to compact an index
// greater than raftLog.applied.

so it is clear that compact(1) = discard nothing.

if snapi > numberOfCatchUpEntries {
compacti = snapi - numberOfCatchUpEntries
}
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.
Expand All @@ -846,7 +852,7 @@ func (s *EtcdServer) snapshot(snapi uint64, confState raftpb.ConfState) {
}
log.Panicf("etcdserver: unexpected compaction error %v", err)
}
log.Printf("etcdserver: saved snapshot at index %d", snap.Metadata.Index)
log.Printf("etcdserver: compacted raft log at %d", compacti)
}()
}

Expand Down