diff --git a/cluster.go b/cluster.go index 54f8795ab..794b8935c 100644 --- a/cluster.go +++ b/cluster.go @@ -1443,7 +1443,7 @@ func (c *cluster) followResizeInstruction(instr *ResizeInstruction) error { return errors.Wrap(err, "merging cluster status") } - c.logger.Printf("done MergeClusterStatus, start goroutine") + c.logger.Printf("done MergeClusterStatus, start goroutine (%s)", c.Node.ID) // The actual resizing runs in a goroutine because we don't want to block // the distribution of other ResizeInstructions to the rest of the cluster. diff --git a/field.go b/field.go index 811786def..899e6f399 100644 --- a/field.go +++ b/field.go @@ -15,11 +15,12 @@ package pilosa import ( - "bufio" + "bytes" "context" "encoding/json" "fmt" "io/ioutil" + "log" "math" "os" "path/filepath" @@ -63,6 +64,26 @@ const ( FieldTypeDecimal = "decimal" ) +type protected struct { + mu sync.Mutex + duration time.Duration +} + +func (p *protected) Set(d time.Duration) { + p.mu.Lock() + defer p.mu.Unlock() + p.duration = d +} +func (p *protected) Get() time.Duration { + p.mu.Lock() + defer p.mu.Unlock() + return p.duration +} + +var availableShardFileFlushDuration = &protected{ + duration: 5 * time.Second, +} + // Field represents a container for views. type Field struct { mu sync.RWMutex @@ -112,6 +133,12 @@ type Field struct { // based on a foreign index; this prevents having to // call holder.index.Keys() every time. usesKeys bool + + // Synchronization primitives needed for async writing of + // the remoteAvailableShards + availableShardChan chan []byte + doneChan chan struct{} + wg sync.WaitGroup } // FieldOption is a functional option type for pilosa.fieldOptions. @@ -348,6 +375,7 @@ func newField(path, index, name string, opts FieldOption) (*Field, error) { OpenTranslateStore: OpenInMemTranslateStore, } + return f, nil } @@ -385,6 +413,13 @@ func (f *Field) AvailableShards() *roaring.Bitmap { return b } +// constainsShard is used for limiting unnecessary CreateShard broadcast +func (f *Field) containsShard(shard uint64) bool { + f.mu.RLock() + defer f.mu.RUnlock() + return f.remoteAvailableShards.Contains(shard) +} + // AddRemoteAvailableShards merges the set of available shards into the current known set // and saves the set to a file. func (f *Field) AddRemoteAvailableShards(b *roaring.Bitmap) error { @@ -441,29 +476,11 @@ func (f *Field) saveAvailableShards() error { } func (f *Field) unprotectedSaveAvailableShards() error { - path := filepath.Join(f.path, ".available.shards") - // Create a temporary file to save to. - tempPath := path + tempExt - - // Open or create file. - file, err := os.OpenFile(tempPath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0666) - if err != nil { - return errors.Wrap(err, "opening temporary available shards file") + var buf bytes.Buffer + if _, err := f.remoteAvailableShards.WriteTo(&buf); err != nil { + return errors.Wrap(err, "rendering available shards ") } - defer file.Close() - - // Write available shards to file. - bw := bufio.NewWriter(file) - if _, err = f.remoteAvailableShards.WriteTo(bw); err != nil { - return errors.Wrap(err, "writing bitmap to buffer") - } - bw.Flush() - - // Move snapshot to data file location. - if err := os.Rename(tempPath, path); err != nil { - return fmt.Errorf("rename snapshot: %s", err) - } - + f.availableShardChan <- buf.Bytes() return nil } @@ -578,7 +595,10 @@ func (f *Field) Open() error { return errors.Wrap(err, "checking foreign index") } } - + f.availableShardChan = make(chan []byte) + f.doneChan = make(chan struct{}) + f.wg.Add(1) + go f.writeAvailableShards() return nil }(); err != nil { f.Close() @@ -588,6 +608,65 @@ func (f *Field) Open() error { f.logger.Debugf("successfully opened field index/field: %s/%s", f.index, f.name) return nil } +func blockingWriteAvailableShards(fieldPath string, availableShardBytes []byte) { + path := filepath.Join(fieldPath, ".available.shards") + // Create a temporary file to save to. + tempPath := path + tempExt + err := ioutil.WriteFile(tempPath, availableShardBytes, 0666) + if err != nil { + log.Println("failed to write ", tempPath) + return + } + + // Move snapshot to data file location. + if err := os.Rename(tempPath, path); err != nil { + log.Printf("rename snapshot: %s", err) + } + +} +func nonBlockingWriteAvailableShards(fieldPath string, availableShardBytes []byte, done chan bool) { + if len(availableShardBytes) == 0 { + return + } + go func() { + blockingWriteAvailableShards(fieldPath, availableShardBytes) + done <- true + }() +} + +func (f *Field) writeAvailableShards() { + defer f.wg.Done() + ticker := time.NewTicker(availableShardFileFlushDuration.Get()) + var data []byte + tracker := make(chan bool) + writing := false + + for alive := true; alive; { + select { + case newdata := <-f.availableShardChan: + data = newdata + case <-ticker.C: + if len(data) > 0 { + if !writing { + writing = true + nonBlockingWriteAvailableShards(f.path, data, tracker) + data = nil + } + } + case <-tracker: + writing = false + case <-f.doneChan: + if writing { //wait to writing is complete + <-tracker + } + if len(data) > 0 { + blockingWriteAvailableShards(f.path, data) + } + alive = false + } + } + ticker.Stop() +} // applyTranslateStore opens the configured translate store. func (f *Field) applyTranslateStore() error { @@ -887,7 +966,15 @@ func (f *Field) applyOptions(opt FieldOptions) error { func (f *Field) Close() error { f.mu.Lock() defer f.mu.Unlock() - + // Shutdown the available shards writer + if f.doneChan != nil { + f.doneChan <- struct{}{} + f.wg.Wait() + close(f.availableShardChan) + close(f.doneChan) + f.availableShardChan = nil + f.doneChan = nil + } // Close the attribute store. if f.rowAttrStore != nil { _ = f.rowAttrStore.Close() @@ -1099,6 +1186,7 @@ func (f *Field) newView(path, name string) *view { view.rowAttrStore = f.rowAttrStore view.stats = f.Stats view.broadcaster = f.broadcaster + view.remoteShardPresent = f.containsShard if f.snapshotQueue != nil { view.snapshotQueue = f.snapshotQueue } diff --git a/field_internal_test.go b/field_internal_test.go index 507feac8b..ee13a269c 100644 --- a/field_internal_test.go +++ b/field_internal_test.go @@ -351,6 +351,7 @@ func TestField_RowTime(t *testing.T) { } func TestField_PersistAvailableShards(t *testing.T) { + availableShardFileFlushDuration.Set(200 * time.Millisecond) //shorten the default time to force a file write f := OpenField(t, OptFieldTypeDefault()) // bm represents remote available shards. @@ -359,6 +360,7 @@ func TestField_PersistAvailableShards(t *testing.T) { if err := f.AddRemoteAvailableShards(bm); err != nil { t.Fatal(err) } + time.Sleep(2 * availableShardFileFlushDuration.Get()) // Reload field and verify that shard data is persisted. if err := f.Reopen(); err != nil { @@ -370,6 +372,7 @@ func TestField_PersistAvailableShards(t *testing.T) { } func TestField_CorruptAvailableShards(t *testing.T) { + availableShardFileFlushDuration.Set(200 * time.Millisecond) //shorten the default time to force a file write f := OpenField(t, OptFieldTypeDefault()) // bm represents remote available shards. @@ -378,6 +381,7 @@ func TestField_CorruptAvailableShards(t *testing.T) { if err := f.AddRemoteAvailableShards(bm); err != nil { t.Fatal(err) } + time.Sleep(2 * availableShardFileFlushDuration.Get()) path := filepath.Join(f.path, ".available.shards") @@ -400,6 +404,7 @@ func TestField_CorruptAvailableShards(t *testing.T) { } func TestField_TruncatedAvailableShards(t *testing.T) { + availableShardFileFlushDuration.Set(200 * time.Millisecond) //shorten the default time to force a file write f := OpenField(t, OptFieldTypeDefault()) // bm represents remote available shards. @@ -408,6 +413,7 @@ func TestField_TruncatedAvailableShards(t *testing.T) { if err := f.AddRemoteAvailableShards(bm); err != nil { t.Fatal(err) } + time.Sleep(2 * availableShardFileFlushDuration.Get()) path := filepath.Join(f.path, ".available.shards") @@ -428,6 +434,7 @@ func TestField_TruncatedAvailableShards(t *testing.T) { // Ensure that persisting available shards having a smaller footprint (for example, // when going from a bitmap to a smaller, RLE representation) succeeds. func TestField_PersistAvailableShardsFootprint(t *testing.T) { + availableShardFileFlushDuration.Set(200 * time.Millisecond) //shorten the default time to force a file write f := OpenField(t, OptFieldTypeDefault()) // bm represents remote available shards. @@ -442,12 +449,14 @@ func TestField_PersistAvailableShardsFootprint(t *testing.T) { if err := f.AddRemoteAvailableShards(bm); err != nil { t.Fatal(err) } + time.Sleep(2 * availableShardFileFlushDuration.Get()) // Reload field and verify that shard data is persisted. if err := f.Reopen(); err != nil { t.Fatal(err) } else if !reflect.DeepEqual(f.remoteAvailableShards.Slice(), bm.Slice()) { - t.Fatalf("unexpected available shards (reopen). expected: %v, but got: %v", bm.Slice(), f.remoteAvailableShards.Slice()) + t.Fatalf("unexpected available shards (reopen). expected: %v, \n but got: %v", bm.Slice(), f.remoteAvailableShards.Slice()) + } bm1 := roaring.NewBitmap() diff --git a/holder.go b/holder.go index 987b8939d..3b9631761 100644 --- a/holder.go +++ b/holder.go @@ -1185,7 +1185,7 @@ func (s *holderSyncer) readFieldTranslateReader(rd TranslateEntryReader) { // Find appropriate store. f := s.Holder.Field(entry.Index, entry.Field) if f == nil { - s.Holder.Logger.Printf("field not found: %q/%q", entry.Index, entry.Field) + s.Holder.Logger.Printf("field not found: %s/%s", entry.Index, entry.Field) return } @@ -1233,6 +1233,15 @@ func (c *holderCleaner) CleanHolder() error { // Get the fragments registered in memory. for _, field := range index.Fields() { + // deletedShards is used to track which shards for the field + // were deleted. Any shards that get deleted from this node + // get added to remoteAvailableShards. This is done because + // the CleanHolder process is cleaning up shards which got + // moved to other nodes. Because those shards still exist + // (just no longer on this particular node), this node still + // needs to consider each of them as an available shard in + // the cluster. + var deletedShards []uint64 for _, view := range field.views() { for _, fragment := range view.allFragments() { fragShard := fragment.shard @@ -1244,6 +1253,12 @@ func (c *holderCleaner) CleanHolder() error { if err := view.deleteFragment(fragShard); err != nil { return errors.Wrap(err, "deleting fragment") } + deletedShards = append(deletedShards, fragShard) + } + } + if len(deletedShards) > 0 { + if err := field.AddRemoteAvailableShards(roaring.NewBitmap(deletedShards...)); err != nil { + return errors.Wrap(err, "adding remote available shards") } } } diff --git a/holder_internal_test.go b/holder_internal_test.go index afee39c6c..ccc52c5b0 100644 --- a/holder_internal_test.go +++ b/holder_internal_test.go @@ -21,6 +21,7 @@ import ( "reflect" "strings" "testing" + "time" "github.com/pilosa/pilosa/v2/roaring" ) @@ -98,6 +99,7 @@ func TestHolder_Optn(t *testing.T) { if os.Geteuid() == 0 { t.Skip("Skipping permissions test since user is root.") } + availableShardFileFlushDuration.Set(100 * time.Millisecond) h := newHolder() defer h.Close() @@ -182,6 +184,7 @@ func TestHolder_Optn(t *testing.T) { // Ensure holder can clean up orphaned fragments. func TestHolderCleaner_CleanHolder(t *testing.T) { + availableShardFileFlushDuration.Set(100 * time.Millisecond) //shorten the default time to force a file write cluster := NewTestCluster(2) // Create a local holder. @@ -223,6 +226,7 @@ func TestHolderCleaner_CleanHolder(t *testing.T) { if err != nil { t.Fatalf("adding remote shards: %v", err) } + time.Sleep(2 * availableShardFileFlushDuration.Get()) // Keep replication the same and ensure we get the expected results. cluster.ReplicaN = 2 diff --git a/http/client.go b/http/client.go index c688e5565..a8a5106d6 100644 --- a/http/client.go +++ b/http/client.go @@ -1115,6 +1115,7 @@ func (c *InternalClient) SendMessage(ctx context.Context, uri *pilosa.URI, msg [ req.Header.Set("Content-Type", "application/x-protobuf") req.Header.Set("User-Agent", "pilosa/"+pilosa.Version) req.Header.Set("Accept", "application/json") + req.Header.Set("Connection", "keep-alive") // Execute request. resp, err := c.executeRequest(req.WithContext(ctx)) @@ -1232,6 +1233,7 @@ func (c *InternalClient) TranslateIDsNode(ctx context.Context, uri *pilosa.URI, // is closed. func (c *InternalClient) executeRequest(req *http.Request) (*http.Response, error) { tracing.GlobalTracer.InjectHTTPHeaders(req) + req.Close = false resp, err := c.httpClient.Do(req) if err != nil { if resp != nil { diff --git a/http/handler.go b/http/handler.go index 584791d75..68bb384ce 100644 --- a/http/handler.go +++ b/http/handler.go @@ -1590,7 +1590,7 @@ func (h *Handler) handlePostClusterMessage(w http.ResponseWriter, r *http.Reques http.Error(w, "Unsupported media type", http.StatusUnsupportedMediaType) return } - + defer r.Body.Close() err := h.api.ClusterMessage(r.Context(), r.Body) if err != nil { // TODO this was the previous behavior, but perhaps not everything is a bad request diff --git a/server/cluster_test.go b/server/cluster_test.go index a991c9f7a..0899b8e97 100644 --- a/server/cluster_test.go +++ b/server/cluster_test.go @@ -206,7 +206,6 @@ func TestClusterResize_AddNode(t *testing.T) { `); err != nil { t.Fatal(err) } - // exp is the expected result for the Row queries that follow. exp := `{"results":[{"attrs":{},"columns":[1,1300000]}]}` + "\n" diff --git a/utils_internal_test.go b/utils_internal_test.go index 8abfdeb91..83751f4bf 100644 --- a/utils_internal_test.go +++ b/utils_internal_test.go @@ -34,6 +34,7 @@ func NewTestCluster(n int) *cluster { panic(err) } + availableShardFileFlushDuration.Set(100 * time.Millisecond) c := newCluster() c.ReplicaN = 1 c.Hasher = NewTestModHasher() diff --git a/view.go b/view.go index 815e2a71e..06d38dce8 100644 --- a/view.go +++ b/view.go @@ -55,11 +55,12 @@ type view struct { // Fragments by shard. fragments map[uint64]*fragment - broadcaster broadcaster - stats stats.StatsClient - rowAttrStore AttrStore - logger logger.Logger - snapshotQueue snapshotQueue + broadcaster broadcaster + stats stats.StatsClient + rowAttrStore AttrStore + logger logger.Logger + snapshotQueue snapshotQueue + remoteShardPresent func(uint64) bool } // newView returns a new instance of View. @@ -76,9 +77,10 @@ func newView(path, index, field, name string, fieldOptions FieldOptions) *view { fragments: make(map[uint64]*fragment), - broadcaster: NopBroadcaster, - stats: stats.NopStatsClient, - logger: logger.NopLogger, + broadcaster: NopBroadcaster, + stats: stats.NopStatsClient, + logger: logger.NopLogger, + remoteShardPresent: func(uint64) bool { return false }, } } @@ -276,6 +278,14 @@ func (v *view) CreateFragmentIfNotExists(shard uint64) (*fragment, error) { frag.RowAttrStore = v.rowAttrStore v.fragments[shard] = frag + v.notifyIfNewShard(shard) + return frag, nil +} + +func (v *view) notifyIfNewShard(shard uint64) { + if v.remoteShardPresent(shard) { //checks the fields remoteShards bitmap to see if broadcast needed + return + } broadcastChan := make(chan struct{}) go func() { @@ -299,8 +309,6 @@ func (v *view) CreateFragmentIfNotExists(shard uint64) (*fragment, error) { case <-time.After(50 * time.Millisecond): v.logger.Debugf("broadcasting create shard took >50ms") } - - return frag, nil } func (v *view) newFragment(path string, shard uint64) *fragment {