Skip to content

Commit b3b688e

Browse files
corecodeSimon Schubert
corecode
authored and
Simon Schubert
committed
sbft: get rid of null requests + deliver when necessary
Change-Id: I127556199bf44ec095b10d42334b7811d3dfdc68 Signed-off-by: Simon Schubert <[email protected]>
1 parent 273ec21 commit b3b688e

File tree

5 files changed

+116
-109
lines changed

5 files changed

+116
-109
lines changed

‎orderer/sbft/simplebft/connection.go

+2
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,8 @@ func (s *SBFT) handleHello(h *Hello, src uint64) {
9797
s.view = h.NewView.View
9898
s.activeView = true
9999
}
100+
101+
s.maybeDeliverUsingXset(h.NewView)
100102
}
101103

102104
s.replicaState[src].hello = h

‎orderer/sbft/simplebft/newview.go

+57-25
Original file line numberDiff line numberDiff line change
@@ -37,15 +37,15 @@ func (s *SBFT) maybeSendNewView() {
3737
}
3838
}
3939

40-
xset, newBatch, ok := s.makeXset(vcs)
40+
xset, _, ok := s.makeXset(vcs)
4141
if !ok {
4242
log.Debug("xset not yet sufficient")
4343
return
4444
}
4545

4646
var batch *Batch
47-
if newBatch != nil {
48-
batch = newBatch
47+
if xset == nil {
48+
// no need for batch, it is contained in the vset
4949
} else if reflect.DeepEqual(s.cur.subject.Digest, xset.Digest) {
5050
batch = s.cur.preprep.Batch
5151
} else {
@@ -103,13 +103,7 @@ func (s *SBFT) handleNewView(nv *NewView, src uint64) {
103103
return
104104
}
105105

106-
if nv.Batch == nil {
107-
log.Warningf("invalid new view from %d: no batch attached", src)
108-
s.sendViewChange()
109-
return
110-
}
111-
112-
xset, newBatch, ok := s.makeXset(vcs)
106+
xset, _, ok := s.makeXset(vcs)
113107

114108
if !ok || !reflect.DeepEqual(nv.Xset, xset) {
115109
log.Warningf("invalid new view from %d: xset incorrect: %v, %v", src, nv.Xset, xset)
@@ -118,25 +112,26 @@ func (s *SBFT) handleNewView(nv *NewView, src uint64) {
118112
}
119113

120114
if nv.Xset == nil {
121-
if !bytes.Equal(nv.Batch.Hash(), newBatch.Hash()) {
122-
log.Warningf("invalid new view from %d: new batch for null request does not match: %x, %x, %v",
123-
src, nv.Batch.Hash(), newBatch.Hash(), nv)
115+
if nv.Batch != nil {
116+
log.Warningf("invalid new view from %d: null request should come with null batch", src)
124117
s.sendViewChange()
125118
return
126119
}
127-
} else if !bytes.Equal(nv.Batch.Hash(), nv.Xset.Digest) {
120+
} else if nv.Batch == nil || !bytes.Equal(nv.Batch.Hash(), nv.Xset.Digest) {
128121
log.Warningf("invalid new view from %d: batch head hash does not match xset: %x, %x, %v",
129122
src, hash(nv.Batch.Header), nv.Xset.Digest, nv)
130123
s.sendViewChange()
131124
return
132125
}
133126

134-
_, err = s.checkBatch(nv.Batch, true, false)
135-
if err != nil {
136-
log.Warningf("invalid new view from %d: invalid batch, %s",
137-
src, err)
138-
s.sendViewChange()
139-
return
127+
if nv.Batch != nil {
128+
_, err = s.checkBatch(nv.Batch, true, false)
129+
if err != nil {
130+
log.Warningf("invalid new view from %d: invalid batch, %s",
131+
src, err)
132+
s.sendViewChange()
133+
return
134+
}
140135
}
141136

142137
s.replicaState[s.primaryIDView(nv.View)].newview = nv
@@ -162,12 +157,49 @@ func (s *SBFT) processNewView() {
162157
s.activeView = true
163158
s.discardBacklog(s.primaryID())
164159

165-
pp := &Preprepare{
166-
Seq: &SeqView{Seq: nv.Batch.DecodeHeader().Seq, View: s.view},
167-
Batch: nv.Batch,
168-
}
160+
s.maybeDeliverUsingXset(nv)
161+
162+
// By now we cannot be waiting for any more outstanding
163+
// messages. after a new-view message, by definition all
164+
// activity has acquiesced. Prepare to accept a new request.
165+
s.cur.checkpointDone = true
166+
s.cur.subject.Seq.Seq = 0
167+
168+
log.Infof("replica %d now active in view %d; primary: %v", s.id, s.view, s.isPrimary())
169169

170-
s.handleCheckedPreprepare(pp)
170+
if nv.Batch != nil {
171+
pp := &Preprepare{
172+
Seq: &SeqView{Seq: nv.Batch.DecodeHeader().Seq, View: s.view},
173+
Batch: nv.Batch,
174+
}
175+
176+
s.handleCheckedPreprepare(pp)
177+
} else {
178+
log.Debugf("%+v", s)
179+
s.cancelViewChangeTimer()
180+
s.maybeSendNextBatch()
181+
}
171182

172183
s.processBacklog()
173184
}
185+
186+
func (s *SBFT) maybeDeliverUsingXset(nv *NewView) {
187+
// TODO we could cache vcs in replicaState
188+
vcs, err := s.checkNewViewSignatures(nv)
189+
if err != nil {
190+
panic(err)
191+
}
192+
193+
_, prevBatch, ok := s.makeXset(vcs)
194+
if !ok {
195+
panic("invalid newview")
196+
}
197+
if s.sys.LastBatch().DecodeHeader().Seq < prevBatch.DecodeHeader().Seq {
198+
if prevBatch.DecodeHeader().Seq == s.cur.subject.Seq.Seq {
199+
// we just received a signature set for a request which we preprepared, but never delivered.
200+
prevBatch.Payloads = s.cur.preprep.Batch.Payloads
201+
}
202+
s.cur.checkpointDone = true
203+
s.sys.Deliver(prevBatch)
204+
}
205+
}

‎orderer/sbft/simplebft/newview_test.go

+10-7
Original file line numberDiff line numberDiff line change
@@ -80,14 +80,17 @@ func TestXsetNoNew(t *testing.T) {
8080
},
8181
}
8282

83-
_, newBatch, ok := s.makeXset(vcs)
83+
xset, prevBatch, ok := s.makeXset(vcs)
8484
if !ok {
8585
t.Fatal("no xset")
8686
}
8787

88-
expect := s.makeBatch(3, prev.Hash(), nil)
89-
if !reflect.DeepEqual(newBatch, expect) {
90-
t.Errorf("batches don't match: %v, %v", newBatch.DecodeHeader(), expect.DecodeHeader())
88+
if xset != nil {
89+
t.Errorf("should have null request")
90+
}
91+
92+
if !reflect.DeepEqual(prevBatch, prev) {
93+
t.Errorf("batches don't match: %v, %v", prevBatch.DecodeHeader(), prev.DecodeHeader())
9194
}
9295
}
9396

@@ -132,8 +135,8 @@ func TestXsetByz0(t *testing.T) {
132135
if !ok {
133136
t.Error("no xset")
134137
}
135-
if !reflect.DeepEqual(xset, &Subject{&SeqView{3, 2}, []byte("val2")}) {
136-
t.Error(xset)
138+
if xset != nil {
139+
t.Error("expected null request")
137140
}
138141
}
139142

@@ -170,7 +173,7 @@ func TestXsetByz2(t *testing.T) {
170173
View: 3,
171174
Pset: []*Subject{&Subject{&SeqView{1, 2}, []byte("val1")}},
172175
Qset: []*Subject{&Subject{&SeqView{1, 2}, []byte("val1")}},
173-
Checkpoint: s.makeBatch(2, []byte("prev"), nil),
176+
Checkpoint: s.makeBatch(1, []byte("prev"), nil),
174177
})
175178

176179
xset, _, ok = s.makeXset(vcs)

‎orderer/sbft/simplebft/simplebft_test.go

+30-42
Original file line numberDiff line numberDiff line change
@@ -320,13 +320,10 @@ func TestViewChangeXset(t *testing.T) {
320320
if i == 3 {
321321
continue
322322
}
323-
if len(a.batches) != 2 {
324-
t.Fatal("expected execution of 1 null request + 1 batch")
325-
}
326-
if len(a.batches[0].Payloads) != 0 {
327-
t.Error("not a null request")
323+
if len(a.batches) != 1 {
324+
t.Fatalf("expected execution of 1 batch: %v", a.batches)
328325
}
329-
if !reflect.DeepEqual([][]byte{r2}, a.batches[1].Payloads) {
326+
if !reflect.DeepEqual([][]byte{r2}, a.batches[0].Payloads) {
330327
t.Error("wrong request executed")
331328
}
332329
}
@@ -373,13 +370,13 @@ func TestRestart(t *testing.T) {
373370
repls[1].Request(r3)
374371
sys.Run()
375372
for _, a := range adapters {
376-
if len(a.batches) != 3 {
377-
t.Fatalf("expected execution of 3 batches, %d got %v", a.id, a.batches)
373+
if len(a.batches) != 2 {
374+
t.Fatalf("expected execution of 2 batches, %d got %v", a.id, a.batches)
378375
}
379-
if !reflect.DeepEqual([][]byte{r1}, a.batches[1].Payloads) {
376+
if !reflect.DeepEqual([][]byte{r1}, a.batches[0].Payloads) {
380377
t.Error("wrong request executed (1)")
381378
}
382-
if !reflect.DeepEqual([][]byte{r2, r3}, a.batches[2].Payloads) {
379+
if !reflect.DeepEqual([][]byte{r2, r3}, a.batches[1].Payloads) {
383380
t.Error("wrong request executed (2)")
384381
}
385382
}
@@ -442,13 +439,13 @@ func TestRestartAfterPrepare(t *testing.T) {
442439
repls[1].Request(r3)
443440
sys.Run()
444441
for _, a := range adapters {
445-
if len(a.batches) != 3 {
446-
t.Fatal("expected execution of 3 batches")
442+
if len(a.batches) != 2 {
443+
t.Fatal("expected execution of 2 batches")
447444
}
448-
if !reflect.DeepEqual([][]byte{r1}, a.batches[1].Payloads) {
445+
if !reflect.DeepEqual([][]byte{r1}, a.batches[0].Payloads) {
449446
t.Error("wrong request executed (1)")
450447
}
451-
if !reflect.DeepEqual([][]byte{r2, r3}, a.batches[2].Payloads) {
448+
if !reflect.DeepEqual([][]byte{r2, r3}, a.batches[1].Payloads) {
452449
t.Error("wrong request executed (2)")
453450
}
454451
}
@@ -511,13 +508,13 @@ func TestRestartAfterCommit(t *testing.T) {
511508
repls[1].Request(r3)
512509
sys.Run()
513510
for _, a := range adapters {
514-
if len(a.batches) != 3 {
515-
t.Fatal("expected execution of 3 batches")
511+
if len(a.batches) != 2 {
512+
t.Fatal("expected execution of 2 batches")
516513
}
517-
if !reflect.DeepEqual([][]byte{r1}, a.batches[1].Payloads) {
514+
if !reflect.DeepEqual([][]byte{r1}, a.batches[0].Payloads) {
518515
t.Error("wrong request executed (1)")
519516
}
520-
if !reflect.DeepEqual([][]byte{r2, r3}, a.batches[2].Payloads) {
517+
if !reflect.DeepEqual([][]byte{r2, r3}, a.batches[1].Payloads) {
521518
t.Error("wrong request executed (2)")
522519
}
523520
}
@@ -580,13 +577,13 @@ func TestRestartAfterCheckpoint(t *testing.T) {
580577
repls[1].Request(r3)
581578
sys.Run()
582579
for _, a := range adapters {
583-
if len(a.batches) != 3 {
584-
t.Fatal("expected execution of 3 batches")
580+
if len(a.batches) != 2 {
581+
t.Fatal("expected execution of 2 batches")
585582
}
586-
if !reflect.DeepEqual([][]byte{r1}, a.batches[1].Payloads) {
583+
if !reflect.DeepEqual([][]byte{r1}, a.batches[0].Payloads) {
587584
t.Error("wrong request executed (1)")
588585
}
589-
if !reflect.DeepEqual([][]byte{r2, r3}, a.batches[2].Payloads) {
586+
if !reflect.DeepEqual([][]byte{r2, r3}, a.batches[1].Payloads) {
590587
t.Error("wrong request executed (2)")
591588
}
592589
}
@@ -669,13 +666,13 @@ func TestErroneousViewChange(t *testing.T) {
669666
repls[1].Request(r3)
670667
sys.Run()
671668
for _, a := range adapters {
672-
if len(a.batches) != 3 {
673-
t.Fatal("expected execution of 3 batches")
669+
if len(a.batches) != 2 {
670+
t.Fatal("expected execution of 2 batches")
674671
}
675-
if !reflect.DeepEqual([][]byte{r1}, a.batches[1].Payloads) {
672+
if !reflect.DeepEqual([][]byte{r1}, a.batches[0].Payloads) {
676673
t.Error("wrong request executed (1)")
677674
}
678-
if !reflect.DeepEqual([][]byte{r2, r3}, a.batches[2].Payloads) {
675+
if !reflect.DeepEqual([][]byte{r2, r3}, a.batches[1].Payloads) {
679676
t.Error("wrong request executed (2)")
680677
}
681678
}
@@ -863,24 +860,15 @@ func TestViewChangeTimer(t *testing.T) {
863860
repls[2].Request(r3)
864861
sys.Run()
865862
for _, a := range adapters {
866-
offs := 0
863+
if len(a.batches) != 2 {
864+
t.Fatalf("%d: expected execution of 2 batches: %v", a.id, a.batches)
865+
}
867866
if a.id != 3 {
868-
if len(a.batches) != 3 {
869-
t.Fatalf("%d: expected execution of 3 batches: %v", a.id, a.batches)
870-
}
871867
if !reflect.DeepEqual([][]byte{r1}, a.batches[0].Payloads) {
872868
t.Errorf("%d: wrong request executed (1): %v", a.id, a.batches)
873869
}
874-
} else {
875-
offs = -1
876-
if len(a.batches) != 2 {
877-
t.Fatalf("%d: expected execution of 3 batches: %v", a.id, a.batches)
878-
}
879-
}
880-
if len(a.batches[1+offs].Payloads) != 0 {
881-
t.Errorf("%d: not a null request: %v", a.id, a.batches[1])
882870
}
883-
if !reflect.DeepEqual([][]byte{r2, r3}, a.batches[2+offs].Payloads) {
871+
if !reflect.DeepEqual([][]byte{r2, r3}, a.batches[1].Payloads) {
884872
t.Errorf("%d: wrong request executed (2): %v", a.id, a.batches)
885873
}
886874
}
@@ -933,13 +921,13 @@ func TestResendViewChange(t *testing.T) {
933921
repls[1].Request(r3)
934922
sys.Run()
935923
for _, a := range adapters {
936-
if len(a.batches) != 3 {
924+
if len(a.batches) != 2 {
937925
t.Fatal("expected execution of 2 batches")
938926
}
939-
if !reflect.DeepEqual([][]byte{r1}, a.batches[1].Payloads) {
927+
if !reflect.DeepEqual([][]byte{r1}, a.batches[0].Payloads) {
940928
t.Error("wrong request executed (1)")
941929
}
942-
if !reflect.DeepEqual([][]byte{r2, r3}, a.batches[2].Payloads) {
930+
if !reflect.DeepEqual([][]byte{r2, r3}, a.batches[1].Payloads) {
943931
t.Error("wrong request executed (2)")
944932
}
945933
}

0 commit comments

Comments
 (0)