Skip to content

Commit e15ce28

Browse files
gyuhogyuho
committed
wal: add missing logs, improve pipeline test coverage
Signed-off-by: Gyuho Lee <[email protected]>
1 parent 610dd09 commit e15ce28

7 files changed

+273
-49
lines changed

‎wal/file_pipeline.go

+5-1
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,11 @@ func (fp *filePipeline) alloc() (f *fileutil.LockedFile, err error) {
7575
return nil, err
7676
}
7777
if err = fileutil.Preallocate(f.File, fp.size, true); err != nil {
78-
plog.Errorf("failed to allocate space when creating new wal file (%v)", err)
78+
if fp.lg != nil {
79+
fp.lg.Warn("failed to preallocate space when creating a new WAL", zap.Int64("size", fp.size), zap.Error(err))
80+
} else {
81+
plog.Errorf("failed to allocate space when creating new wal file (%v)", err)
82+
}
7983
f.Close()
8084
return nil, err
8185
}

‎wal/file_pipeline_test.go

+73
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
// Copyright 2018 The etcd Authors
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package wal
16+
17+
import (
18+
"io/ioutil"
19+
"math"
20+
"os"
21+
"testing"
22+
23+
"go.uber.org/zap"
24+
)
25+
26+
func TestFilePipeline(t *testing.T) {
27+
tdir, err := ioutil.TempDir(os.TempDir(), "wal-test")
28+
if err != nil {
29+
t.Fatal(err)
30+
}
31+
defer os.RemoveAll(tdir)
32+
33+
fp := newFilePipeline(zap.NewExample(), tdir, SegmentSizeBytes)
34+
defer fp.Close()
35+
36+
f, ferr := fp.Open()
37+
if ferr != nil {
38+
t.Fatal(ferr)
39+
}
40+
f.Close()
41+
}
42+
43+
func TestFilePipelineFailPreallocate(t *testing.T) {
44+
tdir, err := ioutil.TempDir(os.TempDir(), "wal-test")
45+
if err != nil {
46+
t.Fatal(err)
47+
}
48+
defer os.RemoveAll(tdir)
49+
50+
fp := newFilePipeline(zap.NewExample(), tdir, math.MaxInt64)
51+
defer fp.Close()
52+
53+
f, ferr := fp.Open()
54+
if f != nil || ferr == nil { // no space left on device
55+
t.Fatal("expected error on invalid pre-allocate size, but no error")
56+
}
57+
}
58+
59+
func TestFilePipelineFailLockFile(t *testing.T) {
60+
tdir, err := ioutil.TempDir(os.TempDir(), "wal-test")
61+
if err != nil {
62+
t.Fatal(err)
63+
}
64+
os.RemoveAll(tdir)
65+
66+
fp := newFilePipeline(zap.NewExample(), tdir, math.MaxInt64)
67+
defer fp.Close()
68+
69+
f, ferr := fp.Open()
70+
if f != nil || ferr == nil { // no such file or directory
71+
t.Fatal("expected error on invalid pre-allocate size, but no error")
72+
}
73+
}

‎wal/repair.go

+21-12
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,12 @@ func Repair(lg *zap.Logger, dirpath string) bool {
3434
}
3535
defer f.Close()
3636

37+
if lg != nil {
38+
lg.Info("repairing", zap.String("path", f.Name()))
39+
} else {
40+
plog.Noticef("repairing %v", f.Name())
41+
}
42+
3743
rec := &walpb.Record{}
3844
decoder := newDecoder(f)
3945
for {
@@ -55,18 +61,16 @@ func Repair(lg *zap.Logger, dirpath string) bool {
5561
continue
5662

5763
case io.EOF:
64+
if lg != nil {
65+
lg.Info("repaired", zap.String("path", f.Name()), zap.Error(io.EOF))
66+
}
5867
return true
5968

6069
case io.ErrUnexpectedEOF:
61-
if lg != nil {
62-
lg.Info("repairing", zap.String("path", f.Name()))
63-
} else {
64-
plog.Noticef("repairing %v", f.Name())
65-
}
6670
bf, bferr := os.Create(f.Name() + ".broken")
6771
if bferr != nil {
6872
if lg != nil {
69-
lg.Warn("failed to create backup file", zap.String("path", f.Name()+".broken"))
73+
lg.Warn("failed to create backup file", zap.String("path", f.Name()+".broken"), zap.Error(bferr))
7074
} else {
7175
plog.Errorf("could not repair %v, failed to create backup file", f.Name())
7276
}
@@ -76,7 +80,7 @@ func Repair(lg *zap.Logger, dirpath string) bool {
7680

7781
if _, err = f.Seek(0, io.SeekStart); err != nil {
7882
if lg != nil {
79-
lg.Warn("failed to read file", zap.String("path", f.Name()))
83+
lg.Warn("failed to read file", zap.String("path", f.Name()), zap.Error(err))
8084
} else {
8185
plog.Errorf("could not repair %v, failed to read file", f.Name())
8286
}
@@ -85,7 +89,7 @@ func Repair(lg *zap.Logger, dirpath string) bool {
8589

8690
if _, err = io.Copy(bf, f); err != nil {
8791
if lg != nil {
88-
lg.Warn("failed to copy", zap.String("from", f.Name()+".broken"), zap.String("to", f.Name()))
92+
lg.Warn("failed to copy", zap.String("from", f.Name()+".broken"), zap.String("to", f.Name()), zap.Error(err))
8993
} else {
9094
plog.Errorf("could not repair %v, failed to copy file", f.Name())
9195
}
@@ -94,25 +98,30 @@ func Repair(lg *zap.Logger, dirpath string) bool {
9498

9599
if err = f.Truncate(lastOffset); err != nil {
96100
if lg != nil {
97-
lg.Warn("failed to truncate", zap.String("path", f.Name()))
101+
lg.Warn("failed to truncate", zap.String("path", f.Name()), zap.Error(err))
98102
} else {
99103
plog.Errorf("could not repair %v, failed to truncate file", f.Name())
100104
}
101105
return false
102106
}
107+
103108
if err = fileutil.Fsync(f.File); err != nil {
104109
if lg != nil {
105-
lg.Warn("failed to fsync", zap.String("path", f.Name()))
110+
lg.Warn("failed to fsync", zap.String("path", f.Name()), zap.Error(err))
106111
} else {
107112
plog.Errorf("could not repair %v, failed to sync file", f.Name())
108113
}
109114
return false
110115
}
116+
117+
if lg != nil {
118+
lg.Info("repaired", zap.String("path", f.Name()), zap.Error(io.ErrUnexpectedEOF))
119+
}
111120
return true
112121

113122
default:
114123
if lg != nil {
115-
lg.Warn("failed to repair", zap.Error(err))
124+
lg.Warn("failed to repair", zap.String("path", f.Name()), zap.Error(err))
116125
} else {
117126
plog.Errorf("could not repair error (%v)", err)
118127
}
@@ -123,7 +132,7 @@ func Repair(lg *zap.Logger, dirpath string) bool {
123132

124133
// openLast opens the last wal file for read and write.
125134
func openLast(lg *zap.Logger, dirpath string) (*fileutil.LockedFile, error) {
126-
names, err := readWalNames(lg, dirpath)
135+
names, err := readWALNames(lg, dirpath)
127136
if err != nil {
128137
return nil, err
129138
}

‎wal/repair_test.go

+56-1
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ func testRepair(t *testing.T, ents [][]raftpb.Entry, corrupt corruptFunc, expect
4949
t.Fatal(err)
5050
}
5151
defer os.RemoveAll(p)
52+
5253
// create WAL
5354
w, err := Create(zap.NewExample(), p, nil)
5455
defer func() {
@@ -90,7 +91,7 @@ func testRepair(t *testing.T, ents [][]raftpb.Entry, corrupt corruptFunc, expect
9091

9192
// repair the wal
9293
if ok := Repair(zap.NewExample(), p); !ok {
93-
t.Fatalf("fix = %t, want %t", ok, true)
94+
t.Fatalf("'Repair' returned '%v', want 'true'", ok)
9495
}
9596

9697
// read it back
@@ -184,3 +185,57 @@ func TestRepairWriteTearMiddle(t *testing.T) {
184185
}
185186
testRepair(t, ents, corruptf, 1)
186187
}
188+
189+
func TestRepairFailDeleteDir(t *testing.T) {
190+
p, err := ioutil.TempDir(os.TempDir(), "waltest")
191+
if err != nil {
192+
t.Fatal(err)
193+
}
194+
defer os.RemoveAll(p)
195+
196+
w, err := Create(zap.NewExample(), p, nil)
197+
if err != nil {
198+
t.Fatal(err)
199+
}
200+
201+
oldSegmentSizeBytes := SegmentSizeBytes
202+
SegmentSizeBytes = 64
203+
defer func() {
204+
SegmentSizeBytes = oldSegmentSizeBytes
205+
}()
206+
for _, es := range makeEnts(50) {
207+
if err = w.Save(raftpb.HardState{}, es); err != nil {
208+
t.Fatal(err)
209+
}
210+
}
211+
212+
_, serr := w.tail().Seek(0, io.SeekCurrent)
213+
if serr != nil {
214+
t.Fatal(serr)
215+
}
216+
w.Close()
217+
218+
f, err := openLast(zap.NewExample(), p)
219+
if err != nil {
220+
t.Fatal(err)
221+
}
222+
if terr := f.Truncate(20); terr != nil {
223+
t.Fatal(err)
224+
}
225+
f.Close()
226+
227+
w, err = Open(zap.NewExample(), p, walpb.Snapshot{})
228+
if err != nil {
229+
t.Fatal(err)
230+
}
231+
_, _, _, err = w.ReadAll()
232+
if err != io.ErrUnexpectedEOF {
233+
t.Fatalf("err = %v, want error %v", err, io.ErrUnexpectedEOF)
234+
}
235+
w.Close()
236+
237+
os.RemoveAll(p)
238+
if Repair(zap.NewExample(), p) {
239+
t.Fatal("expect 'Repair' fail on unexpected directory deletion")
240+
}
241+
}

‎wal/util.go

+23-15
Original file line numberDiff line numberDiff line change
@@ -20,15 +20,15 @@ import (
2020
"strings"
2121

2222
"github.com/coreos/etcd/pkg/fileutil"
23+
2324
"go.uber.org/zap"
2425
)
2526

26-
var (
27-
badWalName = errors.New("bad wal name")
28-
)
27+
var errBadWALName = errors.New("bad wal name")
2928

30-
func Exist(dirpath string) bool {
31-
names, err := fileutil.ReadDir(dirpath)
29+
// Exist returns true if there are any files in a given directory.
30+
func Exist(dir string) bool {
31+
names, err := fileutil.ReadDir(dir)
3232
if err != nil {
3333
return false
3434
}
@@ -38,12 +38,16 @@ func Exist(dirpath string) bool {
3838
// searchIndex returns the last array index of names whose raft index section is
3939
// equal to or smaller than the given index.
4040
// The given names MUST be sorted.
41-
func searchIndex(names []string, index uint64) (int, bool) {
41+
func searchIndex(lg *zap.Logger, names []string, index uint64) (int, bool) {
4242
for i := len(names) - 1; i >= 0; i-- {
4343
name := names[i]
44-
_, curIndex, err := parseWalName(name)
44+
_, curIndex, err := parseWALName(name)
4545
if err != nil {
46-
plog.Panicf("parse correct name should never fail: %v", err)
46+
if lg != nil {
47+
lg.Panic("failed to parse WAL file name", zap.String("path", name), zap.Error(err))
48+
} else {
49+
plog.Panicf("parse correct name should never fail: %v", err)
50+
}
4751
}
4852
if index >= curIndex {
4953
return i, true
@@ -54,12 +58,16 @@ func searchIndex(names []string, index uint64) (int, bool) {
5458

5559
// names should have been sorted based on sequence number.
5660
// isValidSeq checks whether seq increases continuously.
57-
func isValidSeq(names []string) bool {
61+
func isValidSeq(lg *zap.Logger, names []string) bool {
5862
var lastSeq uint64
5963
for _, name := range names {
60-
curSeq, _, err := parseWalName(name)
64+
curSeq, _, err := parseWALName(name)
6165
if err != nil {
62-
plog.Panicf("parse correct name should never fail: %v", err)
66+
if lg != nil {
67+
lg.Panic("failed to parse WAL file name", zap.String("path", name), zap.Error(err))
68+
} else {
69+
plog.Panicf("parse correct name should never fail: %v", err)
70+
}
6371
}
6472
if lastSeq != 0 && lastSeq != curSeq-1 {
6573
return false
@@ -69,7 +77,7 @@ func isValidSeq(names []string) bool {
6977
return true
7078
}
7179

72-
func readWalNames(lg *zap.Logger, dirpath string) ([]string, error) {
80+
func readWALNames(lg *zap.Logger, dirpath string) ([]string, error) {
7381
names, err := fileutil.ReadDir(dirpath)
7482
if err != nil {
7583
return nil, err
@@ -84,7 +92,7 @@ func readWalNames(lg *zap.Logger, dirpath string) ([]string, error) {
8492
func checkWalNames(lg *zap.Logger, names []string) []string {
8593
wnames := make([]string, 0)
8694
for _, name := range names {
87-
if _, _, err := parseWalName(name); err != nil {
95+
if _, _, err := parseWALName(name); err != nil {
8896
// don't complain about left over tmp files
8997
if !strings.HasSuffix(name, ".tmp") {
9098
if lg != nil {
@@ -103,9 +111,9 @@ func checkWalNames(lg *zap.Logger, names []string) []string {
103111
return wnames
104112
}
105113

106-
func parseWalName(str string) (seq, index uint64, err error) {
114+
func parseWALName(str string) (seq, index uint64, err error) {
107115
if !strings.HasSuffix(str, ".wal") {
108-
return 0, 0, badWalName
116+
return 0, 0, errBadWALName
109117
}
110118
_, err = fmt.Sscanf(str, "%016x-%016x.wal", &seq, &index)
111119
return seq, index, err

0 commit comments

Comments
 (0)