From 92f013393c11826de8a937c72b50caa47fbb875f Mon Sep 17 00:00:00 2001 From: Yicheng Qin Date: Thu, 8 Jan 2015 14:46:13 -0800 Subject: [PATCH 1/7] test: remove no-test directory etcdserverpb --- test | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test b/test index 27e192eb8..149aa758f 100755 --- a/test +++ b/test @@ -15,7 +15,7 @@ COVER=${COVER:-"-cover"} source ./build # Hack: gofmt ./ will recursively check the .git directory. So use *.go for gofmt. -TESTABLE_AND_FORMATTABLE="client discovery error etcdctl/command etcdmain etcdserver etcdserver/etcdhttp etcdserver/etcdhttp/httptypes etcdserver/etcdserverpb etcdserver/idutil integration migrate pkg/fileutil pkg/flags pkg/ioutils pkg/netutil pkg/pbutil pkg/types pkg/transport pkg/wait proxy raft rafthttp snap store wal" +TESTABLE_AND_FORMATTABLE="client discovery error etcdctl/command etcdmain etcdserver etcdserver/etcdhttp etcdserver/etcdhttp/httptypes etcdserver/idutil integration migrate pkg/fileutil pkg/flags pkg/ioutils pkg/netutil pkg/pbutil pkg/types pkg/transport pkg/wait proxy raft rafthttp snap store wal" FORMATTABLE="$TESTABLE_AND_FORMATTABLE *.go etcdctl/" # user has not provided PKG override From 9532810f7635b6dbb9aeaa53496277e19f742e8f Mon Sep 17 00:00:00 2001 From: Yicheng Qin Date: Thu, 8 Jan 2015 14:49:14 -0800 Subject: [PATCH 2/7] wal: remove unused max function --- wal/util.go | 7 ------- 1 file changed, 7 deletions(-) diff --git a/wal/util.go b/wal/util.go index 62af664e0..44a74cad2 100644 --- a/wal/util.go +++ b/wal/util.go @@ -129,10 +129,3 @@ func parseWalName(str string) (seq, index uint64, err error) { func walName(seq, index uint64) string { return fmt.Sprintf("%016x-%016x.wal", seq, index) } - -func max(a, b int64) int64 { - if a > b { - return a - } - return b -} From f08d1090d0d90d984a839d06823f85ca68395b54 Mon Sep 17 00:00:00 2001 From: Yicheng Qin Date: Thu, 8 Jan 2015 14:56:21 -0800 Subject: [PATCH 3/7] wal: refine parseWalName function According to http://godoc.org/fmt#Scan, if scan number is less than the number of arguments, err will report why. So we don't need to handle this error case. --- wal/util.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/wal/util.go b/wal/util.go index 44a74cad2..1d57d4b0e 100644 --- a/wal/util.go +++ b/wal/util.go @@ -118,11 +118,7 @@ func checkWalNames(names []string) []string { } func parseWalName(str string) (seq, index uint64, err error) { - var num int - num, err = fmt.Sscanf(str, "%016x-%016x.wal", &seq, &index) - if num != 2 && err == nil { - err = fmt.Errorf("bad wal name: %s", str) - } + _, err = fmt.Sscanf(str, "%016x-%016x.wal", &seq, &index) return } From 50c179ec1cfb9fe4aa0b063e5eca453d2634ec85 Mon Sep 17 00:00:00 2001 From: Yicheng Qin Date: Thu, 8 Jan 2015 21:54:05 -0800 Subject: [PATCH 4/7] wal: add DetectVersion test --- wal/util.go | 6 ++-- wal/util_test.go | 79 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+), 3 deletions(-) create mode 100644 wal/util_test.go diff --git a/wal/util.go b/wal/util.go index 1d57d4b0e..cf132e42e 100644 --- a/wal/util.go +++ b/wal/util.go @@ -37,11 +37,11 @@ const ( ) func DetectVersion(dirpath string) (WalVersion, error) { - if _, err := os.Stat(dirpath); os.IsNotExist(err) { - return WALNotExist, nil - } names, err := fileutil.ReadDir(dirpath) if err != nil { + if os.IsNotExist(err) { + err = nil + } // Error reading the directory return WALNotExist, err } diff --git a/wal/util_test.go b/wal/util_test.go new file mode 100644 index 000000000..dbfa58106 --- /dev/null +++ b/wal/util_test.go @@ -0,0 +1,79 @@ +/* + Copyright 2014 CoreOS, Inc. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package wal + +import ( + "io/ioutil" + "os" + "path" + "strings" + "testing" +) + +func TestDetectVersion(t *testing.T) { + tests := []struct { + names []string + wver WalVersion + }{ + {[]string{}, WALNotExist}, + {[]string{"snap/", "wal/", "wal/1"}, WALv0_5}, + {[]string{"snapshot/", "conf", "log"}, WALv0_4}, + {[]string{"weird"}, WALUnknown}, + {[]string{"snap/", "wal/"}, WALUnknown}, + } + for i, tt := range tests { + p := mustMakeDir(t, tt.names...) + ver, err := DetectVersion(p) + if ver != tt.wver { + t.Errorf("#%d: version = %s, want %s", i, ver, tt.wver) + } + if err != nil { + t.Errorf("#%d: err = %s, want nil", i, err) + } + os.RemoveAll(p) + } + + // detect on non-exist directory + v, err := DetectVersion(path.Join(os.TempDir(), "waltest", "not-exist")) + if v != WALNotExist { + t.Errorf("#non-exist: version = %s, want %s", v, WALNotExist) + } + if err != nil { + t.Errorf("#non-exist: err = %s, want %s", v, WALNotExist) + } +} + +// mustMakeDir builds the directory that contains files with the given +// names. If the name ends with '/', it is created as a directory. +func mustMakeDir(t *testing.T, names ...string) string { + p, err := ioutil.TempDir(os.TempDir(), "waltest") + if err != nil { + t.Fatal(err) + } + for _, n := range names { + if strings.HasSuffix(n, "/") { + if err := os.MkdirAll(path.Join(p, n), 0700); err != nil { + t.Fatal(err) + } + } else { + if _, err := os.Create(path.Join(p, n)); err != nil { + t.Fatal(err) + } + } + } + return p +} From 270e67db84e88a2b8f6dfbdb368557efae15f0f1 Mon Sep 17 00:00:00 2001 From: Yicheng Qin Date: Fri, 9 Jan 2015 12:01:22 -0800 Subject: [PATCH 5/7] wal: not export unnecessary public functions --- etcdserver/force_cluster.go | 8 +++----- wal/wal.go | 10 +++++----- wal/wal_bench_test.go | 2 +- wal/wal_test.go | 27 +++++++++++---------------- 4 files changed, 20 insertions(+), 27 deletions(-) diff --git a/etcdserver/force_cluster.go b/etcdserver/force_cluster.go index 810ed1c31..22693e686 100644 --- a/etcdserver/force_cluster.go +++ b/etcdserver/force_cluster.go @@ -51,11 +51,9 @@ func restartAsStandaloneNode(cfg *ServerConfig, snapshot *raftpb.Snapshot) (type ents = append(ents, toAppEnts...) // force commit newly appended entries - for _, e := range toAppEnts { - err := w.SaveEntry(&e) - if err != nil { - log.Fatalf("etcdserver: %v", err) - } + err := w.Save(raftpb.HardState{}, toAppEnts) + if err != nil { + log.Fatalf("etcdserver: %v", err) } if len(ents) != 0 { st.Commit = ents[len(ents)-1].Index diff --git a/wal/wal.go b/wal/wal.go index 0eea6e89b..4794cf7ed 100644 --- a/wal/wal.go +++ b/wal/wal.go @@ -307,7 +307,7 @@ func (w *WAL) Cut() error { if err := w.encoder.encode(&walpb.Record{Type: metadataType, Data: w.metadata}); err != nil { return err } - if err := w.SaveState(&w.state); err != nil { + if err := w.saveState(&w.state); err != nil { return err } return w.sync() @@ -363,7 +363,7 @@ func (w *WAL) Close() error { return nil } -func (w *WAL) SaveEntry(e *raftpb.Entry) error { +func (w *WAL) saveEntry(e *raftpb.Entry) error { b := pbutil.MustMarshal(e) rec := &walpb.Record{Type: entryType, Data: b} if err := w.encoder.encode(rec); err != nil { @@ -373,7 +373,7 @@ func (w *WAL) SaveEntry(e *raftpb.Entry) error { return nil } -func (w *WAL) SaveState(s *raftpb.HardState) error { +func (w *WAL) saveState(s *raftpb.HardState) error { if raft.IsEmptyHardState(*s) { return nil } @@ -385,11 +385,11 @@ func (w *WAL) SaveState(s *raftpb.HardState) error { func (w *WAL) Save(st raftpb.HardState, ents []raftpb.Entry) error { // TODO(xiangli): no more reference operator - if err := w.SaveState(&st); err != nil { + if err := w.saveState(&st); err != nil { return err } for i := range ents { - if err := w.SaveEntry(&ents[i]); err != nil { + if err := w.saveEntry(&ents[i]); err != nil { return err } } diff --git a/wal/wal_bench_test.go b/wal/wal_bench_test.go index 40f4711d5..b4631ac72 100644 --- a/wal/wal_bench_test.go +++ b/wal/wal_bench_test.go @@ -40,7 +40,7 @@ func benchmarkWriteEntry(b *testing.B, size int, batch int) { b.ResetTimer() n := 0 for i := 0; i < b.N; i++ { - err := w.SaveEntry(e) + err := w.saveEntry(e) if err != nil { b.Fatal(err) } diff --git a/wal/wal_test.go b/wal/wal_test.go index 9460f0fb3..a29be040c 100644 --- a/wal/wal_test.go +++ b/wal/wal_test.go @@ -153,12 +153,9 @@ func TestCut(t *testing.T) { } defer w.Close() - // TODO(unihorn): remove this when cut can operate on an empty file - if err := w.SaveEntry(&raftpb.Entry{}); err != nil { - t.Fatal(err) - } state := raftpb.HardState{Term: 1} - if err := w.SaveState(&state); err != nil { + // TODO(unihorn): remove this when cut can operate on an empty file + if err := w.Save(state, []raftpb.Entry{{}}); err != nil { t.Fatal(err) } if err := w.Cut(); err != nil { @@ -169,8 +166,8 @@ func TestCut(t *testing.T) { t.Errorf("name = %s, want %s", g, wname) } - e := &raftpb.Entry{Index: 1, Term: 1, Data: []byte{1}} - if err := w.SaveEntry(e); err != nil { + es := []raftpb.Entry{{Index: 1, Term: 1, Data: []byte{1}}} + if err := w.Save(raftpb.HardState{}, es); err != nil { t.Fatal(err) } if err := w.Cut(); err != nil { @@ -221,14 +218,12 @@ func TestRecover(t *testing.T) { t.Fatal(err) } ents := []raftpb.Entry{{Index: 1, Term: 1, Data: []byte{1}}, {Index: 2, Term: 2, Data: []byte{2}}} - for _, e := range ents { - if err = w.SaveEntry(&e); err != nil { - t.Fatal(err) - } + if err = w.Save(raftpb.HardState{}, ents); err != nil { + t.Fatal(err) } sts := []raftpb.HardState{{Term: 1, Vote: 1, Commit: 1}, {Term: 2, Vote: 2, Commit: 2}} for _, s := range sts { - if err = w.SaveState(&s); err != nil { + if err = w.Save(s, nil); err != nil { t.Fatal(err) } } @@ -338,8 +333,8 @@ func TestRecoverAfterCut(t *testing.T) { if err = w.SaveSnapshot(walpb.Snapshot{Index: uint64(i)}); err != nil { t.Fatal(err) } - e := raftpb.Entry{Index: uint64(i)} - if err = w.SaveEntry(&e); err != nil { + es := []raftpb.Entry{{Index: uint64(i)}} + if err = w.Save(raftpb.HardState{}, es); err != nil { t.Fatal(err) } if err = w.Cut(); err != nil { @@ -395,7 +390,7 @@ func TestOpenAtUncommittedIndex(t *testing.T) { if err := w.SaveSnapshot(walpb.Snapshot{}); err != nil { t.Fatal(err) } - if err := w.SaveEntry(&raftpb.Entry{Index: 0}); err != nil { + if err := w.Save(raftpb.HardState{}, []raftpb.Entry{{Index: 0}}); err != nil { t.Fatal(err) } w.Close() @@ -417,7 +412,7 @@ func TestSaveEmpty(t *testing.T) { w := WAL{ encoder: newEncoder(&buf, 0), } - if err := w.SaveState(&est); err != nil { + if err := w.saveState(&est); err != nil { t.Errorf("err = %v, want nil", err) } if len(buf.Bytes()) != 0 { From 9bdc343b7c75fb2d2b4c5836b83f3ec2186a041f Mon Sep 17 00:00:00 2001 From: Yicheng Qin Date: Fri, 9 Jan 2015 12:47:27 -0800 Subject: [PATCH 6/7] wal: add ReleaseLockTo test --- wal/wal_test.go | 45 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/wal/wal_test.go b/wal/wal_test.go index a29be040c..f4590b780 100644 --- a/wal/wal_test.go +++ b/wal/wal_test.go @@ -406,6 +406,51 @@ func TestOpenAtUncommittedIndex(t *testing.T) { w.Close() } +// TestOpenNotInUse tests that OpenNotInUse can load all files that are +// not in use at that point. +// The tests creates WAL directory, and cut out multiple WAL files. Then +// it releases the lock of part of data, and excepts that OpenNotInUse +// can read out all unlocked data. +func TestOpenNotInUse(t *testing.T) { + p, err := ioutil.TempDir(os.TempDir(), "waltest") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(p) + // create WAL + w, err := Create(p, nil) + defer w.Close() + if err != nil { + t.Fatal(err) + } + // make 10 seperate files + for i := 0; i < 10; i++ { + es := []raftpb.Entry{{Index: uint64(i)}} + if err = w.Save(raftpb.HardState{}, es); err != nil { + t.Fatal(err) + } + if err = w.Cut(); err != nil { + t.Fatal(err) + } + } + // release the lock to 5 + unlockIndex := uint64(5) + w.ReleaseLockTo(unlockIndex) + + w2, err := OpenNotInUse(p, walpb.Snapshot{}) + defer w2.Close() + if err != nil { + t.Fatal(err) + } + _, _, ents, err := w2.ReadAll() + if err != nil { + t.Fatalf("err = %v, want nil", err) + } + if g := ents[len(ents)-1].Index; g != unlockIndex { + t.Errorf("last index read = %d, want %d", g, unlockIndex) + } +} + func TestSaveEmpty(t *testing.T) { var buf bytes.Buffer var est raftpb.HardState From 05e591f80509ac24fe793432972e9b46a0580714 Mon Sep 17 00:00:00 2001 From: Yicheng Qin Date: Fri, 9 Jan 2015 12:48:01 -0800 Subject: [PATCH 7/7] wal: remove unused encoder.buffered func --- wal/encoder.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/wal/encoder.go b/wal/encoder.go index e0cd86228..0f2f0be02 100644 --- a/wal/encoder.go +++ b/wal/encoder.go @@ -56,10 +56,6 @@ func (e *encoder) flush() error { return e.bw.Flush() } -func (e *encoder) buffered() int { - return e.bw.Buffered() -} - func writeInt64(w io.Writer, n int64) error { return binary.Write(w, binary.LittleEndian, n) }