diff --git a/server/server.go b/server/server.go index b4ac595fa..010eb2f8a 100644 --- a/server/server.go +++ b/server/server.go @@ -364,7 +364,10 @@ func (m *Command) Close() error { eg.Go(m.gossipMemberSet.Close) } if closer, ok := m.logOutput.(io.Closer); ok { - eg.Go(closer.Close) + // If closer is os.Stdout or os.Stderr, don't close it. + if closer != os.Stdout && closer != os.Stderr { + eg.Go(closer.Close) + } } err := eg.Wait() return errors.Wrap(err, "closing everything") diff --git a/view.go b/view.go index 5fc0f5eb8..a0abb0e9c 100644 --- a/view.go +++ b/view.go @@ -228,6 +228,7 @@ func (v *view) createFragmentIfNotExists(shard uint64) (*fragment, error) { Field: v.field, Shard: shard, }); err != nil { + frag.close() return nil, errors.Wrap(err, "sending createshard message") } diff --git a/view_internal_test.go b/view_internal_test.go index b71696128..4ca7667ad 100644 --- a/view_internal_test.go +++ b/view_internal_test.go @@ -16,7 +16,10 @@ package pilosa import ( "io/ioutil" + "strings" "testing" + + "github.com/pkg/errors" ) // mustOpenView returns a new instance of View with a temporary path. @@ -73,3 +76,45 @@ func TestView_DeleteFragment(t *testing.T) { t.Fatal("failed to create new fragment") } } + +// Ensure view closes fragment after failed shard broadcast. +func TestView_CreateFragmentError(t *testing.T) { + v := mustOpenView("i", "f", "v") + defer v.close() + + // Use a broadcaster which intentionally fails. + v.broadcaster = errorBroadcaster{} + + shard := uint64(0) + + // Create fragment (with error on broadcast). + fragment, err := v.CreateFragmentIfNotExists(shard) + if !strings.Contains(err.Error(), "intentional error") { + if err != nil { + t.Fatal(err) + } else if fragment == nil { + t.Fatal("expected fragment") + } else { + t.Fatal("expected intentional error") + } + } + + // Set the broadcaster back to no-op. + v.broadcaster = nopBroadcaster{} + + // Try to create the fragment again. + _, err = v.CreateFragmentIfNotExists(shard) + if err != nil { + t.Fatal(err) + } +} + +// errorBroadcaster is a broadcaster which always returns an error. +type errorBroadcaster struct { + nopBroadcaster +} + +// SendSync is an implementation of Broadcaster SendSync which always returns an error. +func (errorBroadcaster) SendSync(Message) error { + return errors.New("intentional error") +}