Rename shards only after successfully completing all of them
Before, we could write the first shard, while we would error exit
writing the second shard. The webserver would then see a Frankenstein
version of the corpus.
Fixes #11.
Change-Id: I1bc045c74d0c0e5d46d8e97db5494d8a00748b6c
diff --git a/build/builder.go b/build/builder.go
index 2e0fb01..77ee611 100644
--- a/build/builder.go
+++ b/build/builder.go
@@ -88,6 +88,10 @@
errMu sync.Mutex
buildError error
+
+ // temp name => final name for finished shards. We only rename
+ // them once all shards succeed to avoid Frankstein corpuses.
+ finishedShards map[string]string
}
// SetDefaults sets reasonable default options.
@@ -170,8 +174,9 @@
}
b := &Builder{
- opts: opt,
- throttle: make(chan int, opt.Parallelism),
+ opts: opt,
+ throttle: make(chan int, opt.Parallelism),
+ finishedShards: map[string]string{},
}
if _, err := b.newShardBuilder(); err != nil {
@@ -206,6 +211,20 @@
func (b *Builder) Finish() error {
b.flush()
b.building.Wait()
+
+ if b.buildError != nil {
+ for tmp := range b.finishedShards {
+ os.Remove(tmp)
+ }
+ return b.buildError
+ }
+
+ for tmp, final := range b.finishedShards {
+ if err := os.Rename(tmp, final); err != nil {
+ b.buildError = err
+ }
+ }
+
if b.nextShardNum > 0 {
b.deleteRemainingShards()
}
@@ -319,7 +338,11 @@
for _, t := range todo {
shardBuilder.Add(*t)
}
- return writeShard(name, shardBuilder)
+
+ if err := b.writeShard(name, shardBuilder); err != nil {
+ return err
+ }
+ return nil
}
func (b *Builder) newShardBuilder() (*zoekt.IndexBuilder, error) {
@@ -333,7 +356,7 @@
return shardBuilder, nil
}
-func writeShard(fn string, b *zoekt.IndexBuilder) error {
+func (b *Builder) writeShard(fn string, ib *zoekt.IndexBuilder) error {
dir := filepath.Dir(fn)
if err := os.MkdirAll(dir, 0700); err != nil {
return err
@@ -345,7 +368,7 @@
}
defer f.Close()
- if err := b.Write(f); err != nil {
+ if err := ib.Write(f); err != nil {
return err
}
fi, err := f.Stat()
@@ -355,11 +378,10 @@
if err := f.Close(); err != nil {
return err
}
- if err := os.Rename(f.Name(), fn); err != nil {
- return err
- }
- log.Printf("wrote %s: %d index bytes (overhead %3.1f)", fn, fi.Size(),
- float64(fi.Size())/float64(b.ContentSize()+1))
+ b.finishedShards[f.Name()] = fn
+ log.Printf("finished %s: %d index bytes (overhead %3.1f)", fn, fi.Size(),
+ float64(fi.Size())/float64(ib.ContentSize()+1))
+
return nil
}
diff --git a/build/e2e_test.go b/build/e2e_test.go
index dc7ad0f..46087b7 100644
--- a/build/e2e_test.go
+++ b/build/e2e_test.go
@@ -244,3 +244,48 @@
t.Fatalf("Glob(%s): got %v, want 1 shard", glob, fs)
}
}
+
+func TestPartialSuccess(t *testing.T) {
+ dir, err := ioutil.TempDir("", "")
+ if err != nil {
+ t.Fatalf("TempDir: %v", err)
+ }
+
+ opts := Options{
+ IndexDir: dir,
+ ShardMax: 1024,
+ RepoDir: "/a",
+ SizeMax: 1 << 20,
+ }
+ opts.SetDefaults()
+
+ b, err := NewBuilder(opts)
+ if err != nil {
+ t.Fatalf("NewBuilder: %v", err)
+ }
+
+ for i := 0; i < 4; i++ {
+ nm := fmt.Sprintf("F%d", i)
+
+ // no error checking: the 2nd call will fail
+ b.AddFile(nm, []byte(strings.Repeat("01234567\n", 128)))
+ if i == 1 {
+ // force writes to fail.
+ if err := os.Chmod(dir, 0555); err != nil {
+ t.Fatalf("chmod(%s): %s", dir, err)
+ }
+ }
+ }
+
+ if err := os.Chmod(dir, 0755); err != nil {
+ t.Fatalf("chmod(%s, writable): %s", dir, err)
+ }
+
+ b.Finish()
+
+ if fs, err := filepath.Glob(dir + "/*"); err != nil {
+ t.Errorf("glob(%s): %v", dir, err)
+ } else if len(fs) != 0 {
+ t.Errorf("got shards %v, want []", fs)
+ }
+}
