Skip to content

Commit ef99f6d

Browse files
zzy96jpmsam
authored andcommitted
Fix raft applied index out of range (#880)
1 parent b7edc0b commit ef99f6d

File tree

5 files changed

+186
-21
lines changed

5 files changed

+186
-21
lines changed

raft/backend.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,9 @@ import (
55
"sync"
66
"time"
77

8-
"github.com/ethereum/go-ethereum/core/types"
9-
108
"github.com/ethereum/go-ethereum/accounts"
119
"github.com/ethereum/go-ethereum/core"
10+
"github.com/ethereum/go-ethereum/core/types"
1211
"github.com/ethereum/go-ethereum/eth"
1312
"github.com/ethereum/go-ethereum/eth/downloader"
1413
"github.com/ethereum/go-ethereum/ethdb"

raft/handler.go

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -11,29 +11,27 @@ import (
1111
"sync"
1212
"time"
1313

14-
"golang.org/x/net/context"
15-
14+
"github.com/coreos/etcd/etcdserver/stats"
1615
"github.com/coreos/etcd/pkg/fileutil"
16+
raftTypes "github.com/coreos/etcd/pkg/types"
17+
etcdRaft "github.com/coreos/etcd/raft"
18+
"github.com/coreos/etcd/raft/raftpb"
19+
"github.com/coreos/etcd/rafthttp"
1720
"github.com/coreos/etcd/snap"
1821
"github.com/coreos/etcd/wal"
22+
mapset "github.com/deckarep/golang-set"
23+
"github.com/syndtr/goleveldb/leveldb"
24+
"golang.org/x/net/context"
25+
1926
"github.com/ethereum/go-ethereum/core"
2027
"github.com/ethereum/go-ethereum/core/types"
2128
"github.com/ethereum/go-ethereum/eth/downloader"
2229
"github.com/ethereum/go-ethereum/event"
2330
"github.com/ethereum/go-ethereum/log"
2431
"github.com/ethereum/go-ethereum/p2p"
25-
"github.com/ethereum/go-ethereum/rlp"
26-
27-
"github.com/coreos/etcd/etcdserver/stats"
28-
raftTypes "github.com/coreos/etcd/pkg/types"
29-
etcdRaft "github.com/coreos/etcd/raft"
30-
"github.com/coreos/etcd/raft/raftpb"
31-
"github.com/coreos/etcd/rafthttp"
3232
"github.com/ethereum/go-ethereum/p2p/enode"
3333
"github.com/ethereum/go-ethereum/p2p/enr"
34-
"github.com/syndtr/goleveldb/leveldb"
35-
36-
mapset "github.com/deckarep/golang-set"
34+
"github.com/ethereum/go-ethereum/rlp"
3735
)
3836

3937
type ProtocolManager struct {
@@ -446,6 +444,19 @@ func (pm *ProtocolManager) startRaft() {
446444
// the single call to `pm.applyNewChainHead` for more context.
447445
lastAppliedIndex = lastPersistedCommittedIndex
448446
}
447+
448+
// fix raft applied index out of range
449+
firstIndex, err := pm.raftStorage.FirstIndex()
450+
if err != nil {
451+
panic(fmt.Sprintf("failed to read last persisted applied index from raft while restarting: %v", err))
452+
}
453+
lastPersistedAppliedIndex := firstIndex - 1
454+
if lastPersistedAppliedIndex > lastAppliedIndex {
455+
log.Debug("set lastAppliedIndex to lastPersistedAppliedIndex", "last applied index", lastAppliedIndex, "last persisted applied index", lastPersistedAppliedIndex)
456+
457+
lastAppliedIndex = lastPersistedAppliedIndex
458+
pm.advanceAppliedIndex(lastAppliedIndex)
459+
}
449460
}
450461
}
451462

raft/handler_test.go

Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,155 @@
1+
package raft
2+
3+
import (
4+
"crypto/ecdsa"
5+
"fmt"
6+
"io/ioutil"
7+
"net"
8+
"os"
9+
"reflect"
10+
"testing"
11+
"time"
12+
"unsafe"
13+
14+
"github.com/ethereum/go-ethereum/core"
15+
"github.com/ethereum/go-ethereum/crypto"
16+
"github.com/ethereum/go-ethereum/eth"
17+
"github.com/ethereum/go-ethereum/event"
18+
"github.com/ethereum/go-ethereum/log"
19+
"github.com/ethereum/go-ethereum/node"
20+
"github.com/ethereum/go-ethereum/p2p"
21+
"github.com/ethereum/go-ethereum/p2p/enode"
22+
"github.com/ethereum/go-ethereum/params"
23+
)
24+
25+
// pm.advanceAppliedIndex() and state updates are in different
26+
// transaction boundaries hence there's a probablity that they are
27+
// out of sync due to premature shutdown
28+
func TestProtocolManager_whenAppliedIndexOutOfSync(t *testing.T) {
29+
tmpWorkingDir, err := ioutil.TempDir("", "")
30+
if err != nil {
31+
t.Fatal(err)
32+
}
33+
defer func() {
34+
_ = os.RemoveAll(tmpWorkingDir)
35+
}()
36+
count := 3
37+
ports := make([]uint16, count)
38+
nodeKeys := make([]*ecdsa.PrivateKey, count)
39+
peers := make([]*enode.Node, count)
40+
for i := 0; i < count; i++ {
41+
ports[i] = nextPort(t)
42+
nodeKeys[i] = mustNewNodeKey(t)
43+
peers[i] = enode.NewV4(&(nodeKeys[i].PublicKey), net.IPv4(127, 0, 0, 1), 0, 0, int(ports[i]))
44+
}
45+
raftNodes := make([]*RaftService, count)
46+
for i := 0; i < count; i++ {
47+
if s, err := startRaftNode(uint16(i+1), ports[i], tmpWorkingDir, nodeKeys[i], peers); err != nil {
48+
t.Fatal(err)
49+
} else {
50+
raftNodes[i] = s
51+
}
52+
}
53+
waitFunc := func() {
54+
for {
55+
time.Sleep(200 * time.Millisecond)
56+
for i := 0; i < count; i++ {
57+
if raftNodes[i].raftProtocolManager.role == minterRole {
58+
return
59+
}
60+
}
61+
}
62+
}
63+
waitFunc()
64+
// update the index to mimic the issue (set applied index behind for node 0)
65+
raftNodes[0].raftProtocolManager.advanceAppliedIndex(1)
66+
// now stop and restart the nodes
67+
for i := 0; i < count; i++ {
68+
if err := raftNodes[i].Stop(); err != nil {
69+
t.Fatal(err)
70+
}
71+
}
72+
log.Debug("restart raft cluster")
73+
for i := 0; i < count; i++ {
74+
if s, err := startRaftNode(uint16(i+1), ports[i], tmpWorkingDir, nodeKeys[i], peers); err != nil {
75+
t.Fatal(err)
76+
} else {
77+
raftNodes[i] = s
78+
}
79+
}
80+
waitFunc()
81+
}
82+
83+
func mustNewNodeKey(t *testing.T) *ecdsa.PrivateKey {
84+
k, err := crypto.GenerateKey()
85+
if err != nil {
86+
t.Fatal(err)
87+
}
88+
return k
89+
}
90+
91+
func nextPort(t *testing.T) uint16 {
92+
listener, err := net.Listen("tcp", ":0")
93+
if err != nil {
94+
t.Fatal(err)
95+
}
96+
return uint16(listener.Addr().(*net.TCPAddr).Port)
97+
}
98+
99+
func prepareServiceContext(key *ecdsa.PrivateKey) (ctx *node.ServiceContext, cfg *node.Config, err error) {
100+
defer func() {
101+
if r := recover(); r != nil {
102+
err = fmt.Errorf("%s", r)
103+
ctx = nil
104+
cfg = nil
105+
}
106+
}()
107+
cfg = &node.Config{
108+
P2P: p2p.Config{
109+
PrivateKey: key,
110+
},
111+
}
112+
ctx = &node.ServiceContext{
113+
EventMux: new(event.TypeMux),
114+
}
115+
// config is private field so we need some workaround to set the value
116+
configField := reflect.ValueOf(ctx).Elem().FieldByName("config")
117+
configField = reflect.NewAt(configField.Type(), unsafe.Pointer(configField.UnsafeAddr())).Elem()
118+
configField.Set(reflect.ValueOf(cfg))
119+
return
120+
}
121+
122+
func startRaftNode(id, port uint16, tmpWorkingDir string, key *ecdsa.PrivateKey, nodes []*enode.Node) (*RaftService, error) {
123+
datadir := fmt.Sprintf("%s/node%d", tmpWorkingDir, id)
124+
125+
ctx, _, err := prepareServiceContext(key)
126+
if err != nil {
127+
return nil, err
128+
}
129+
130+
e, err := eth.New(ctx, &eth.Config{
131+
Genesis: &core.Genesis{Config: params.QuorumTestChainConfig},
132+
})
133+
if err != nil {
134+
return nil, err
135+
}
136+
137+
s, err := New(ctx, params.QuorumTestChainConfig, id, port, false, 100*time.Millisecond, e, nodes, datadir, false)
138+
if err != nil {
139+
return nil, err
140+
}
141+
142+
srv := &p2p.Server{
143+
Config: p2p.Config{
144+
PrivateKey: key,
145+
},
146+
}
147+
if err := srv.Start(); err != nil {
148+
return nil, fmt.Errorf("could not start: %v", err)
149+
}
150+
if err := s.Start(srv); err != nil {
151+
return nil, err
152+
}
153+
154+
return s, nil
155+
}

raft/snapshot.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,17 +9,17 @@ import (
99
"sort"
1010
"time"
1111

12-
mapset "github.com/deckarep/golang-set"
13-
"github.com/ethereum/go-ethereum/p2p/enode"
14-
"github.com/ethereum/go-ethereum/p2p/enr"
15-
1612
"github.com/coreos/etcd/raft/raftpb"
1713
"github.com/coreos/etcd/snap"
1814
"github.com/coreos/etcd/wal/walpb"
15+
mapset "github.com/deckarep/golang-set"
16+
1917
"github.com/ethereum/go-ethereum/common"
2018
"github.com/ethereum/go-ethereum/core/types"
2119
"github.com/ethereum/go-ethereum/eth/downloader"
2220
"github.com/ethereum/go-ethereum/log"
21+
"github.com/ethereum/go-ethereum/p2p/enode"
22+
"github.com/ethereum/go-ethereum/p2p/enr"
2323
"github.com/ethereum/go-ethereum/rlp"
2424
)
2525

raft/speculative_chain.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
package raft
22

33
import (
4+
mapset "github.com/deckarep/golang-set"
5+
"gopkg.in/oleiade/lane.v1"
6+
47
"github.com/ethereum/go-ethereum/common"
58
"github.com/ethereum/go-ethereum/core/types"
69
"github.com/ethereum/go-ethereum/log"
7-
8-
"github.com/deckarep/golang-set"
9-
"gopkg.in/oleiade/lane.v1"
1010
)
1111

1212
// The speculative chain represents blocks that we have minted which haven't been accepted into the chain yet, building

0 commit comments

Comments
 (0)