From 61af15d097e8abc8e8d7b4c427c098b984ec9738 Mon Sep 17 00:00:00 2001 From: Richard Artoul Date: Mon, 10 Sep 2018 12:50:41 -0400 Subject: [PATCH 01/28] Refactor commit log bootstrapper to only return it can satisfy everything if node is Available or Leaving for that shard and add uninitialized source --- .../bootstrapper/commitlog/source.go | 76 ++++- .../commitlog/source_data_test.go | 16 +- .../bootstrapper/commitlog/source_test.go | 170 +++++++++++ .../bootstrap/bootstrapper/peers/source.go | 2 + .../bootstrapper/peers/source_test.go | 185 +++++------- .../bootstrapper/uninitialized/options.go | 57 ++++ .../bootstrapper/uninitialized/source.go | 142 +++++++++ .../bootstrapper/uninitialized/source_test.go | 282 ++++++++++++++++++ .../bootstrapper/uninitialized/types.go | 35 +++ src/dbnode/topology/testutil/topology.go | 42 +++ 10 files changed, 875 insertions(+), 132 deletions(-) create mode 100644 src/dbnode/storage/bootstrap/bootstrapper/commitlog/source_test.go create mode 100644 src/dbnode/storage/bootstrap/bootstrapper/uninitialized/options.go create mode 100644 src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source.go create mode 100644 src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source_test.go create mode 100644 src/dbnode/storage/bootstrap/bootstrapper/uninitialized/types.go diff --git a/src/dbnode/storage/bootstrap/bootstrapper/commitlog/source.go b/src/dbnode/storage/bootstrap/bootstrapper/commitlog/source.go index 28dbfbe849..952ada2d38 100644 --- a/src/dbnode/storage/bootstrap/bootstrapper/commitlog/source.go +++ b/src/dbnode/storage/bootstrap/bootstrapper/commitlog/source.go @@ -36,8 +36,10 @@ import ( "github.com/m3db/m3/src/dbnode/storage/bootstrap/result" "github.com/m3db/m3/src/dbnode/storage/index/convert" "github.com/m3db/m3/src/dbnode/storage/namespace" + "github.com/m3db/m3/src/dbnode/topology" "github.com/m3db/m3/src/dbnode/ts" "github.com/m3db/m3/src/dbnode/x/xio" + "github.com/m3db/m3cluster/shard" "github.com/m3db/m3x/checked" "github.com/m3db/m3x/ident" "github.com/m3db/m3x/instrument" @@ -104,10 +106,7 @@ func (s *commitLogSource) AvailableData( shardsTimeRanges result.ShardTimeRanges, runOpts bootstrap.RunOptions, ) result.ShardTimeRanges { - // Commit log bootstrapper is a last ditch effort, so fulfill all - // time ranges requested even if not enough data, just to succeed - // the bootstrap - return shardsTimeRanges + return s.availability(ns, shardsTimeRanges, runOpts) } // ReadData will read a combination of the available snapshot files and commit log files to @@ -1249,10 +1248,7 @@ func (s *commitLogSource) AvailableIndex( shardsTimeRanges result.ShardTimeRanges, runOpts bootstrap.RunOptions, ) result.ShardTimeRanges { - // Commit log bootstrapper is a last ditch effort, so fulfill all - // time ranges requested even if not enough data, just to succeed - // the bootstrap - return shardsTimeRanges + return s.availability(ns, shardsTimeRanges, runOpts) } func (s *commitLogSource) ReadIndex( @@ -1425,6 +1421,70 @@ func (s commitLogSource) maybeAddToIndex( return err } +func (s *commitLogSource) availability( + ns namespace.Metadata, + shardsTimeRanges result.ShardTimeRanges, + runOpts bootstrap.RunOptions, +) result.ShardTimeRanges { + var ( + topoState = runOpts.InitialTopologyState() + availableShardTimeRanges = result.ShardTimeRanges{} + ) + + for shardIDUint := range shardsTimeRanges { + shardID := topology.ShardID(shardIDUint) + hostShardStates, ok := topoState.ShardStates[shardID] + if !ok { + // This shard was not part of the topology when the bootstrapping + // process began. + continue + } + + originHostShardState, ok := hostShardStates[topology.HostID(topoState.Origin.ID())] + if !ok { + // TODO(rartoul): Make this a hard error once we refactor the interface to support + // returning errors. + iOpts := s.opts.CommitLogOptions().InstrumentOptions() + invariantLogger := instrument.EmitInvariantViolationAndGetLogger(iOpts) + invariantLogger.Errorf( + "Initial topolgoy state does not contain shard state for origin node and shard: %d", shardIDUint) + continue + } + + originShardState := originHostShardState.ShardState + switch originShardState { + // In the Initializing and Unknown states we have to assume that the commit log + // is missing data and can't satisfy the bootstrap request. + case shard.Initializing: + case shard.Unknown: + // In the Leaving and Available case, we assume that the commit log contains + // all the data required to satisfy the bootstrap request because the node + // had (at some point) been completely bootstrapped for the requested shard. + // This doesn't mean that the node can't be missing any data or wasn't down + // for some period of time and missing writes in a multi-node deployment, it + // only means that technically the node has successfully taken ownership of + // the data for this shard and made it to a "bootstrapped" state which is + // all that is required to maintain our cluster-level consistency guarantees. + case shard.Leaving: + fallthrough + case shard.Available: + // Assume that we can bootstrap any requested time range, which is valid as + // long as the FS bootstrapper precedes the commit log bootstrapper. + // TODO(rartoul): Once we make changes to the bootstrapping interfaces + // to distinguish between "unfulfilled" data and "corrupt" data, then + // modify this to only say the commit log bootstrapper can fullfil + // "unfulfilled" data, but not corrupt data. + availableShardTimeRanges[shardIDUint] = shardsTimeRanges[shardIDUint] + default: + // TODO(rartoul): Make this a hard error once we refactor the interface to support + // returning errors. + panic(fmt.Sprintf("unknown shard state: %v", originShardState)) + } + } + + return availableShardTimeRanges +} + func newReadSeriesPredicate(ns namespace.Metadata) commitlog.SeriesFilterPredicate { nsID := ns.ID() return func(id ident.ID, namespace ident.ID) bool { diff --git a/src/dbnode/storage/bootstrap/bootstrapper/commitlog/source_data_test.go b/src/dbnode/storage/bootstrap/bootstrapper/commitlog/source_data_test.go index 5964e46407..f47cb335f4 100644 --- a/src/dbnode/storage/bootstrap/bootstrapper/commitlog/source_data_test.go +++ b/src/dbnode/storage/bootstrap/bootstrapper/commitlog/source_data_test.go @@ -86,14 +86,14 @@ func testOptions() Options { } func TestAvailableEmptyRangeError(t *testing.T) { - opts := testOptions() + opts := testDefaultOpts src := newCommitLogSource(opts, fs.Inspection{}) res := src.AvailableData(testNsMetadata(t), result.ShardTimeRanges{}, testDefaultRunOpts) require.True(t, result.ShardTimeRanges{}.Equal(res)) } func TestReadEmpty(t *testing.T) { - opts := testOptions() + opts := testDefaultOpts src := newCommitLogSource(opts, fs.Inspection{}) @@ -105,7 +105,7 @@ func TestReadEmpty(t *testing.T) { } func TestReadErrorOnNewIteratorError(t *testing.T) { - opts := testOptions() + opts := testDefaultOpts src := newCommitLogSource(opts, fs.Inspection{}).(*commitLogSource) src.newIteratorFn = func(_ commitlog.IteratorOpts) (commitlog.Iterator, error) { @@ -124,7 +124,7 @@ func TestReadErrorOnNewIteratorError(t *testing.T) { } func TestReadOrderedValues(t *testing.T) { - opts := testOptions() + opts := testDefaultOpts md := testNsMetadata(t) src := newCommitLogSource(opts, fs.Inspection{}).(*commitLogSource) @@ -169,7 +169,7 @@ func TestReadOrderedValues(t *testing.T) { } func TestReadUnorderedValues(t *testing.T) { - opts := testOptions() + opts := testDefaultOpts md := testNsMetadata(t) src := newCommitLogSource(opts, fs.Inspection{}).(*commitLogSource) @@ -216,7 +216,7 @@ func TestReadUnorderedValues(t *testing.T) { // files can span multiple M3DB processes which means that unique indexes could be re-used for multiple // different series. func TestReadHandlesDifferentSeriesWithIdenticalUniqueIndex(t *testing.T) { - opts := testOptions() + opts := testDefaultOpts md := testNsMetadata(t) src := newCommitLogSource(opts, fs.Inspection{}).(*commitLogSource) @@ -257,7 +257,7 @@ func TestReadHandlesDifferentSeriesWithIdenticalUniqueIndex(t *testing.T) { } func TestReadTrimsToRanges(t *testing.T) { - opts := testOptions() + opts := testDefaultOpts md := testNsMetadata(t) src := newCommitLogSource(opts, fs.Inspection{}).(*commitLogSource) @@ -302,7 +302,7 @@ func TestItMergesSnapshotsAndCommitLogs(t *testing.T) { defer ctrl.Finish() var ( - opts = testOptions() + opts = testDefaultOpts md = testNsMetadata(t) src = newCommitLogSource(opts, fs.Inspection{}).(*commitLogSource) blockSize = md.Options().RetentionOptions().BlockSize() diff --git a/src/dbnode/storage/bootstrap/bootstrapper/commitlog/source_test.go b/src/dbnode/storage/bootstrap/bootstrapper/commitlog/source_test.go new file mode 100644 index 0000000000..4eea17de4a --- /dev/null +++ b/src/dbnode/storage/bootstrap/bootstrapper/commitlog/source_test.go @@ -0,0 +1,170 @@ +// Copyright (c) 2018 Uber Technologies, Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +package commitlog + +import ( + "testing" + "time" + + "github.com/m3db/m3/src/dbnode/persist/fs" + "github.com/m3db/m3/src/dbnode/storage/bootstrap/result" + topotestutils "github.com/m3db/m3/src/dbnode/topology/testutil" + "github.com/m3db/m3cluster/shard" + xtime "github.com/m3db/m3x/time" + "github.com/stretchr/testify/require" +) + +var ( + testDefaultOpts = testOptions() + notSelfID = "not-self" +) + +func TestAvailableData(t *testing.T) { + var ( + nsMetadata = testNsMetadata(t) + blockSize = 2 * time.Hour + shards = []uint32{0, 1, 2, 3} + blockStart = time.Now().Truncate(blockSize) + shardTimeRangesToBootstrap = result.ShardTimeRanges{} + bootstrapRanges = xtime.Ranges{}.AddRange(xtime.Range{ + Start: blockStart, + End: blockStart.Add(blockSize), + }) + ) + + for _, shard := range shards { + shardTimeRangesToBootstrap[shard] = bootstrapRanges + } + + testCases := []struct { + title string + hosts topotestutils.SourceAvailableHosts + majorityReplicas int + shardsTimeRangesToBootstrap result.ShardTimeRanges + expectedAvailableShardsTimeRanges result.ShardTimeRanges + }{ + { + title: "Single node - Shard initializing", + hosts: []topotestutils.SourceAvailableHost{ + topotestutils.SourceAvailableHost{ + Name: topotestutils.SelfID, + Shards: shards, + ShardStates: shard.Initializing, + }, + }, + majorityReplicas: 1, + shardsTimeRangesToBootstrap: shardTimeRangesToBootstrap, + expectedAvailableShardsTimeRanges: result.ShardTimeRanges{}, + }, + { + title: "Single node - Shard unknown", + hosts: []topotestutils.SourceAvailableHost{ + topotestutils.SourceAvailableHost{ + Name: topotestutils.SelfID, + Shards: shards, + ShardStates: shard.Unknown, + }, + }, + majorityReplicas: 1, + shardsTimeRangesToBootstrap: shardTimeRangesToBootstrap, + expectedAvailableShardsTimeRanges: result.ShardTimeRanges{}, + }, + { + title: "Single node - Shard leaving", + hosts: []topotestutils.SourceAvailableHost{ + topotestutils.SourceAvailableHost{ + Name: topotestutils.SelfID, + Shards: shards, + ShardStates: shard.Leaving, + }, + }, + majorityReplicas: 1, + shardsTimeRangesToBootstrap: shardTimeRangesToBootstrap, + expectedAvailableShardsTimeRanges: shardTimeRangesToBootstrap, + }, + { + title: "Single node - Shard available", + hosts: []topotestutils.SourceAvailableHost{ + topotestutils.SourceAvailableHost{ + Name: topotestutils.SelfID, + Shards: shards, + ShardStates: shard.Available, + }, + }, + majorityReplicas: 1, + shardsTimeRangesToBootstrap: shardTimeRangesToBootstrap, + expectedAvailableShardsTimeRanges: shardTimeRangesToBootstrap, + }, + { + title: "Multi node - Origin available", + hosts: []topotestutils.SourceAvailableHost{ + topotestutils.SourceAvailableHost{ + Name: topotestutils.SelfID, + Shards: shards, + ShardStates: shard.Available, + }, + topotestutils.SourceAvailableHost{ + Name: notSelfID, + Shards: shards, + ShardStates: shard.Initializing, + }, + }, + majorityReplicas: 1, + shardsTimeRangesToBootstrap: shardTimeRangesToBootstrap, + expectedAvailableShardsTimeRanges: shardTimeRangesToBootstrap, + }, + { + title: "Multi node - Origin not available", + hosts: []topotestutils.SourceAvailableHost{ + topotestutils.SourceAvailableHost{ + Name: topotestutils.SelfID, + Shards: shards, + ShardStates: shard.Initializing, + }, + topotestutils.SourceAvailableHost{ + Name: notSelfID, + Shards: shards, + ShardStates: shard.Available, + }, + }, + majorityReplicas: 1, + shardsTimeRangesToBootstrap: shardTimeRangesToBootstrap, + expectedAvailableShardsTimeRanges: result.ShardTimeRanges{}, + }, + } + + for _, tc := range testCases { + t.Run(tc.title, func(t *testing.T) { + + var ( + src = newCommitLogSource(testOptions(), fs.Inspection{}) + topoState = tc.hosts.TopologyState(tc.majorityReplicas) + runOpts = testDefaultRunOpts.SetInitialTopologyState(topoState) + dataRes = src.AvailableData(nsMetadata, tc.shardsTimeRangesToBootstrap, runOpts) + ) + + require.Equal(t, tc.expectedAvailableShardsTimeRanges, dataRes) + + indexRes := src.AvailableIndex(nsMetadata, tc.shardsTimeRangesToBootstrap, runOpts) + require.Equal(t, tc.expectedAvailableShardsTimeRanges, indexRes) + }) + } +} diff --git a/src/dbnode/storage/bootstrap/bootstrapper/peers/source.go b/src/dbnode/storage/bootstrap/bootstrapper/peers/source.go index cfba16e48a..814dac2b9c 100644 --- a/src/dbnode/storage/bootstrap/bootstrapper/peers/source.go +++ b/src/dbnode/storage/bootstrap/bootstrapper/peers/source.go @@ -722,6 +722,8 @@ func (s *peersSource) peerAvailability( case shard.Available: shardPeers.numAvailablePeers++ default: + // TODO(rartoul): Make this a hard error once we refactor the interface to support + // returning errors. panic( fmt.Sprintf("encountered unknown shard state: %s", shardState.String())) } diff --git a/src/dbnode/storage/bootstrap/bootstrapper/peers/source_test.go b/src/dbnode/storage/bootstrap/bootstrapper/peers/source_test.go index dd88225650..eb0e9f3c26 100644 --- a/src/dbnode/storage/bootstrap/bootstrapper/peers/source_test.go +++ b/src/dbnode/storage/bootstrap/bootstrapper/peers/source_test.go @@ -25,9 +25,9 @@ import ( "time" m3dbruntime "github.com/m3db/m3/src/dbnode/runtime" - "github.com/m3db/m3/src/dbnode/sharding" "github.com/m3db/m3/src/dbnode/storage/bootstrap/result" "github.com/m3db/m3/src/dbnode/topology" + topotestutils "github.com/m3db/m3/src/dbnode/topology/testutil" "github.com/m3db/m3cluster/shard" xtime "github.com/m3db/m3x/time" @@ -35,49 +35,11 @@ import ( "github.com/stretchr/testify/require" ) -const ( - selfID = "self" -) - -type sourceAvailableHost struct { - name string - shards []uint32 - shardStates shard.State -} - -type sourceAvailableHosts []sourceAvailableHost - -func (s sourceAvailableHosts) topologyState() *topology.StateSnapshot { - topoState := &topology.StateSnapshot{ - Origin: topology.NewHost(selfID, "127.0.0.1"), - MajorityReplicas: 2, - ShardStates: make(map[topology.ShardID]map[topology.HostID]topology.HostShardState), - } - - for _, host := range s { - for _, shard := range host.shards { - hostShardStates, ok := topoState.ShardStates[topology.ShardID(shard)] - if !ok { - hostShardStates = make(map[topology.HostID]topology.HostShardState) - } - - hostShardStates[topology.HostID(host.name)] = topology.HostShardState{ - Host: topology.NewHost(host.name, host.name+"address"), - ShardState: host.shardStates, - } - topoState.ShardStates[topology.ShardID(shard)] = hostShardStates - } - } - - return topoState -} - func TestPeersSourceAvailableDataAndIndex(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() var ( - replicaMajority = 2 blockSize = 2 * time.Hour nsMetadata = testNamespaceMetadata(t) shards = []uint32{0, 1, 2, 3} @@ -98,64 +60,67 @@ func TestPeersSourceAvailableDataAndIndex(t *testing.T) { testCases := []struct { title string - hosts sourceAvailableHosts + hosts topotestutils.SourceAvailableHosts + majorityReplicas int bootstrapReadConsistency topology.ReadConsistencyLevel shardsTimeRangesToBootstrap result.ShardTimeRanges expectedAvailableShardsTimeRanges result.ShardTimeRanges }{ { title: "Returns empty if only self is available", - hosts: []sourceAvailableHost{ - sourceAvailableHost{ - name: selfID, - shards: shards, - shardStates: shard.Available, + hosts: []topotestutils.SourceAvailableHost{ + topotestutils.SourceAvailableHost{ + Name: topotestutils.SelfID, + Shards: shards, + ShardStates: shard.Available, }, }, + majorityReplicas: 1, bootstrapReadConsistency: topology.ReadConsistencyLevelMajority, shardsTimeRangesToBootstrap: shardTimeRangesToBootstrap, expectedAvailableShardsTimeRanges: result.ShardTimeRanges{}, }, { title: "Returns empty if all other peers initializing/unknown", - hosts: []sourceAvailableHost{ - sourceAvailableHost{ - name: selfID, - shards: shards, - shardStates: shard.Available, + hosts: []topotestutils.SourceAvailableHost{ + topotestutils.SourceAvailableHost{ + Name: topotestutils.SelfID, + Shards: shards, + ShardStates: shard.Available, }, - sourceAvailableHost{ - name: "other1", - shards: shards, - shardStates: shard.Initializing, + topotestutils.SourceAvailableHost{ + Name: "other1", + Shards: shards, + ShardStates: shard.Initializing, }, - sourceAvailableHost{ - name: "other2", - shards: shards, - shardStates: shard.Unknown, + topotestutils.SourceAvailableHost{ + Name: "other2", + Shards: shards, + ShardStates: shard.Unknown, }, }, + majorityReplicas: 2, bootstrapReadConsistency: topology.ReadConsistencyLevelMajority, shardsTimeRangesToBootstrap: shardTimeRangesToBootstrap, expectedAvailableShardsTimeRanges: result.ShardTimeRanges{}, }, { title: "Returns success if consistency can be met (available/leaving)", - hosts: []sourceAvailableHost{ - sourceAvailableHost{ - name: selfID, - shards: shards, - shardStates: shard.Initializing, + hosts: []topotestutils.SourceAvailableHost{ + topotestutils.SourceAvailableHost{ + Name: topotestutils.SelfID, + Shards: shards, + ShardStates: shard.Initializing, }, - sourceAvailableHost{ - name: "other1", - shards: shards, - shardStates: shard.Available, + topotestutils.SourceAvailableHost{ + Name: "other1", + Shards: shards, + ShardStates: shard.Available, }, - sourceAvailableHost{ - name: "other2", - shards: shards, - shardStates: shard.Leaving, + topotestutils.SourceAvailableHost{ + Name: "other2", + Shards: shards, + ShardStates: shard.Leaving, }, }, bootstrapReadConsistency: topology.ReadConsistencyLevelMajority, @@ -164,46 +129,48 @@ func TestPeersSourceAvailableDataAndIndex(t *testing.T) { }, { title: "Skips shards that were not in the topology at start", - hosts: []sourceAvailableHost{ - sourceAvailableHost{ - name: selfID, - shards: shards, - shardStates: shard.Initializing, + hosts: []topotestutils.SourceAvailableHost{ + topotestutils.SourceAvailableHost{ + Name: topotestutils.SelfID, + Shards: shards, + ShardStates: shard.Initializing, }, - sourceAvailableHost{ - name: "other1", - shards: shards, - shardStates: shard.Available, + topotestutils.SourceAvailableHost{ + Name: "other1", + Shards: shards, + ShardStates: shard.Available, }, - sourceAvailableHost{ - name: "other2", - shards: shards, - shardStates: shard.Available, + topotestutils.SourceAvailableHost{ + Name: "other2", + Shards: shards, + ShardStates: shard.Available, }, }, + majorityReplicas: 2, bootstrapReadConsistency: topology.ReadConsistencyLevelMajority, shardsTimeRangesToBootstrap: shardTimeRangesToBootstrapOneExtra, expectedAvailableShardsTimeRanges: shardTimeRangesToBootstrap, }, { title: "Returns empty if consistency can not be met", - hosts: []sourceAvailableHost{ - sourceAvailableHost{ - name: selfID, - shards: shards, - shardStates: shard.Initializing, + hosts: []topotestutils.SourceAvailableHost{ + topotestutils.SourceAvailableHost{ + Name: topotestutils.SelfID, + Shards: shards, + ShardStates: shard.Initializing, }, - sourceAvailableHost{ - name: "other1", - shards: shards, - shardStates: shard.Available, + topotestutils.SourceAvailableHost{ + Name: "other1", + Shards: shards, + ShardStates: shard.Available, }, - sourceAvailableHost{ - name: "other2", - shards: shards, - shardStates: shard.Available, + topotestutils.SourceAvailableHost{ + Name: "other2", + Shards: shards, + ShardStates: shard.Available, }, }, + majorityReplicas: 2, bootstrapReadConsistency: topology.ReadConsistencyLevelAll, shardsTimeRangesToBootstrap: shardTimeRangesToBootstrap, expectedAvailableShardsTimeRanges: result.ShardTimeRanges{}, @@ -212,22 +179,6 @@ func TestPeersSourceAvailableDataAndIndex(t *testing.T) { for _, tc := range testCases { t.Run(tc.title, func(t *testing.T) { - hostShardSets := []topology.HostShardSet{} - - for _, host := range tc.hosts { - shards := sharding.NewShards(host.shards, host.shardStates) - shardSet, err := sharding.NewShardSet(shards, sharding.DefaultHashFn(0)) - require.NoError(t, err) - - hostShardSet := topology.NewHostShardSet( - topology.NewHost(host.name, host.name+"address"), shardSet) - hostShardSets = append(hostShardSets, hostShardSet) - } - - mockMap := topology.NewMockMap(ctrl) - mockMap.EXPECT().HostShardSets().Return(hostShardSets).AnyTimes() - mockMap.EXPECT().MajorityReplicas().Return(replicaMajority).AnyTimes() - mockRuntimeOpts := m3dbruntime.NewMockOptions(ctrl) mockRuntimeOpts. EXPECT(). @@ -243,14 +194,16 @@ func TestPeersSourceAvailableDataAndIndex(t *testing.T) { AnyTimes() opts := testDefaultOpts. - // SetAdminClient(mockClient). SetRuntimeOptionsManager(mockRuntimeOptsMgr) src, err := newPeersSource(opts) require.NoError(t, err) - runOpts := testDefaultRunOpts.SetInitialTopologyState(tc.hosts.topologyState()) - dataRes := src.AvailableData(nsMetadata, tc.shardsTimeRangesToBootstrap, runOpts) + var ( + topoState = tc.hosts.TopologyState(tc.majorityReplicas) + runOpts = testDefaultRunOpts.SetInitialTopologyState(topoState) + dataRes = src.AvailableData(nsMetadata, tc.shardsTimeRangesToBootstrap, runOpts) + ) require.Equal(t, tc.expectedAvailableShardsTimeRanges, dataRes) indexRes := src.AvailableIndex(nsMetadata, tc.shardsTimeRangesToBootstrap, runOpts) diff --git a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/options.go b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/options.go new file mode 100644 index 0000000000..555a8fd464 --- /dev/null +++ b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/options.go @@ -0,0 +1,57 @@ +// Copyright (c) 2018 Uber Technologies, Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +package uninitialized + +import ( + "errors" + + "github.com/m3db/m3x/instrument" +) + +var ( + errInstrumentOptsNotSet = errors.New("instrument options not set") +) + +type options struct { + iOpts instrument.Options +} + +func NewOptions() Options { + return &options{} +} + +func (o *options) Validate() error { + if o.iOpts == nil { + return errInstrumentOptsNotSet + } + + return nil +} + +func (o *options) SetInstrumentOptions(value instrument.Options) Options { + opts := *o + opts.iOpts = value + return &opts +} + +func (o *options) InstrumentOptions() instrument.Options { + return o.iOpts +} diff --git a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source.go b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source.go new file mode 100644 index 0000000000..87b953e78e --- /dev/null +++ b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source.go @@ -0,0 +1,142 @@ +// Copyright (c) 2018 Uber Technologies, Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +package uninitialized + +import ( + "fmt" + + "github.com/m3db/m3/src/dbnode/storage/bootstrap" + "github.com/m3db/m3/src/dbnode/storage/bootstrap/result" + "github.com/m3db/m3/src/dbnode/storage/namespace" + "github.com/m3db/m3/src/dbnode/topology" + "github.com/m3db/m3cluster/shard" +) + +// The purpose of the unitializedSource is succeed bootstraps for any +// shard/time-ranges if the given shard/namespace combination has never +// been completely initialized (is a new namespace). This is required for +// allowing us to configure the bootstrappers such that the commitlog +// bootstrapper can precede the peers bootstrapper and still suceed bootstraps +// for brand new namespaces without permitting unintentional data loss by +// putting the noop-all or noop-none bootstrappers at the end of the process. +type uninitializedSource struct { + opts Options +} + +// NewUninitializedSource creates a new uninitialized source. +func NewUninitializedSource(opts Options) (bootstrap.Source, error) { + if err := opts.Validate(); err != nil { + return nil, err + } + + return &uninitializedSource{ + opts: opts, + }, nil +} + +func (s *uninitializedSource) Can(strategy bootstrap.Strategy) bool { + switch strategy { + case bootstrap.BootstrapSequential: + return true + } + + return false +} + +func (s *uninitializedSource) AvailableData( + ns namespace.Metadata, + shardsTimeRanges result.ShardTimeRanges, + runOpts bootstrap.RunOptions, +) result.ShardTimeRanges { + return s.availability(ns, shardsTimeRanges, runOpts) +} + +func (s *uninitializedSource) AvailableIndex( + ns namespace.Metadata, + shardsTimeRanges result.ShardTimeRanges, + runOpts bootstrap.RunOptions, +) result.ShardTimeRanges { + return s.availability(ns, shardsTimeRanges, runOpts) +} + +func (s *uninitializedSource) availability( + ns namespace.Metadata, + shardsTimeRanges result.ShardTimeRanges, + runOpts bootstrap.RunOptions, +) result.ShardTimeRanges { + var ( + topoState = runOpts.InitialTopologyState() + availableShardTimeRanges = result.ShardTimeRanges{} + ) + + for shardIDUint := range shardsTimeRanges { + shardID := topology.ShardID(shardIDUint) + hostShardStates, ok := topoState.ShardStates[shardID] + if !ok { + // This shard was not part of the topology when the bootstrapping + // process began. + continue + } + + var ( + numInitializing = 0 + numLeaving = 0 + ) + for _, hostState := range hostShardStates { + shardState := hostState.ShardState + switch shardState { + case shard.Initializing: + numInitializing++ + case shard.Unknown: + case shard.Leaving: + numLeaving++ + case shard.Available: + default: + // TODO(rartoul): Make this a hard error once we refactor the interface to support + // returning errors. + panic(fmt.Sprintf("unknown shard state: %v", shardState)) + } + } + + isNewNamespace := numInitializing-numLeaving > 0 + if isNewNamespace { + availableShardTimeRanges[shardIDUint] = shardsTimeRanges[shardIDUint] + } + } + + return availableShardTimeRanges +} + +func (s *uninitializedSource) ReadData( + ns namespace.Metadata, + shardsTimeRanges result.ShardTimeRanges, + runOpts bootstrap.RunOptions, +) (result.DataBootstrapResult, error) { + return result.NewDataBootstrapResult(), nil +} + +func (s *uninitializedSource) ReadIndex( + ns namespace.Metadata, + shardsTimeRanges result.ShardTimeRanges, + runOpts bootstrap.RunOptions, +) (result.IndexBootstrapResult, error) { + return result.NewIndexBootstrapResult(), nil +} diff --git a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source_test.go b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source_test.go new file mode 100644 index 0000000000..cac036804e --- /dev/null +++ b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source_test.go @@ -0,0 +1,282 @@ +// Copyright (c) 2018 Uber Technologies, Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +package uninitialized + +import ( + "testing" + "time" + + "github.com/m3db/m3/src/dbnode/storage/bootstrap" + "github.com/m3db/m3/src/dbnode/storage/bootstrap/result" + "github.com/m3db/m3/src/dbnode/storage/namespace" + "github.com/m3db/m3/src/dbnode/topology" + topotestutils "github.com/m3db/m3/src/dbnode/topology/testutil" + "github.com/m3db/m3cluster/shard" + "github.com/m3db/m3x/ident" + "github.com/m3db/m3x/instrument" + xtime "github.com/m3db/m3x/time" + "github.com/stretchr/testify/require" +) + +var ( + testNamespaceID = ident.StringID("testnamespace") + testDefaultRunOpts = bootstrap.NewRunOptions() + notSelfID1 = "not-self-1" + notSelfID2 = "not-self-2" + notSelfID3 = "not-self-3" +) + +func TestUnitializedSourceAvailableDataAndAvailableIndex(t *testing.T) { + var ( + blockSize = 2 * time.Hour + shards = []uint32{0, 1, 2, 3} + blockStart = time.Now().Truncate(blockSize) + shardTimeRangesToBootstrap = result.ShardTimeRanges{} + bootstrapRanges = xtime.Ranges{}.AddRange(xtime.Range{ + Start: blockStart, + End: blockStart.Add(blockSize), + }) + ) + nsMetadata, err := namespace.NewMetadata(testNamespaceID, namespace.NewOptions()) + require.NoError(t, err) + + for _, shard := range shards { + shardTimeRangesToBootstrap[shard] = bootstrapRanges + } + + testCases := []struct { + title string + majorityReplicas int + hosts topotestutils.SourceAvailableHosts + bootstrapReadConsistency topology.ReadConsistencyLevel + shardsTimeRangesToBootstrap result.ShardTimeRanges + expectedAvailableShardsTimeRanges result.ShardTimeRanges + }{ + // Snould return that it can bootstrap everything because + // it's a new namespace. + { + title: "Single node - Shard initializing", + majorityReplicas: 1, + hosts: []topotestutils.SourceAvailableHost{ + topotestutils.SourceAvailableHost{ + Name: topotestutils.SelfID, + Shards: shards, + ShardStates: shard.Initializing, + }, + }, + shardsTimeRangesToBootstrap: shardTimeRangesToBootstrap, + expectedAvailableShardsTimeRanges: shardTimeRangesToBootstrap, + }, + // Snould return that it can't bootstrap anything because we don't + // know how to handle unknown shard states. + { + title: "Single node - Shard unknown", + majorityReplicas: 1, + hosts: []topotestutils.SourceAvailableHost{ + topotestutils.SourceAvailableHost{ + Name: topotestutils.SelfID, + Shards: shards, + ShardStates: shard.Unknown, + }, + }, + shardsTimeRangesToBootstrap: shardTimeRangesToBootstrap, + expectedAvailableShardsTimeRanges: result.ShardTimeRanges{}, + }, + // Snould return that it can't bootstrap anything because it's not + // a new namespace. + { + title: "Single node - Shard leaving", + majorityReplicas: 1, + hosts: []topotestutils.SourceAvailableHost{ + topotestutils.SourceAvailableHost{ + Name: topotestutils.SelfID, + Shards: shards, + ShardStates: shard.Leaving, + }, + }, + shardsTimeRangesToBootstrap: shardTimeRangesToBootstrap, + expectedAvailableShardsTimeRanges: result.ShardTimeRanges{}, + }, + // Snould return that it can't bootstrap anything because it's not + // a new namespace. + { + title: "Single node - Shard available", + majorityReplicas: 1, + hosts: []topotestutils.SourceAvailableHost{ + topotestutils.SourceAvailableHost{ + Name: topotestutils.SelfID, + Shards: shards, + ShardStates: shard.Available, + }, + }, + shardsTimeRangesToBootstrap: shardTimeRangesToBootstrap, + expectedAvailableShardsTimeRanges: result.ShardTimeRanges{}, + }, + // Snould return that it can bootstrap everything because + // it's a new namespace. + { + title: "Multi node - Brand new namespace (all nodes initializing)", + majorityReplicas: 2, + hosts: []topotestutils.SourceAvailableHost{ + topotestutils.SourceAvailableHost{ + Name: topotestutils.SelfID, + Shards: shards, + ShardStates: shard.Initializing, + }, + topotestutils.SourceAvailableHost{ + Name: notSelfID1, + Shards: shards, + ShardStates: shard.Initializing, + }, + topotestutils.SourceAvailableHost{ + Name: notSelfID2, + Shards: shards, + ShardStates: shard.Initializing, + }, + }, + shardsTimeRangesToBootstrap: shardTimeRangesToBootstrap, + expectedAvailableShardsTimeRanges: shardTimeRangesToBootstrap, + }, + // Snould return that it can bootstrap everything because + // it's a new namespace (one of the nodes hasn't completed + // initializing yet.) + { + title: "Multi node - Recently created namespace (one node still initializing)", + majorityReplicas: 2, + hosts: []topotestutils.SourceAvailableHost{ + topotestutils.SourceAvailableHost{ + Name: topotestutils.SelfID, + Shards: shards, + ShardStates: shard.Initializing, + }, + topotestutils.SourceAvailableHost{ + Name: notSelfID1, + Shards: shards, + ShardStates: shard.Available, + }, + topotestutils.SourceAvailableHost{ + Name: notSelfID2, + Shards: shards, + ShardStates: shard.Available, + }, + }, + shardsTimeRangesToBootstrap: shardTimeRangesToBootstrap, + expectedAvailableShardsTimeRanges: shardTimeRangesToBootstrap, + }, + // Snould return that it can't bootstrap anything because it's not + // a new namespace. + { + title: "Multi node - Initialized namespace (no nodes initializing)", + majorityReplicas: 2, + hosts: []topotestutils.SourceAvailableHost{ + topotestutils.SourceAvailableHost{ + Name: topotestutils.SelfID, + Shards: shards, + ShardStates: shard.Available, + }, + topotestutils.SourceAvailableHost{ + Name: notSelfID1, + Shards: shards, + ShardStates: shard.Available, + }, + topotestutils.SourceAvailableHost{ + Name: notSelfID2, + Shards: shards, + ShardStates: shard.Available, + }, + }, + shardsTimeRangesToBootstrap: shardTimeRangesToBootstrap, + expectedAvailableShardsTimeRanges: result.ShardTimeRanges{}, + }, + // Snould return that it can't bootstrap anything because it's not + // a new namespace, we're just doing a node replace. + { + title: "Multi node - Node replace (one node leaving, one initializing)", + majorityReplicas: 2, + hosts: []topotestutils.SourceAvailableHost{ + topotestutils.SourceAvailableHost{ + Name: topotestutils.SelfID, + Shards: shards, + ShardStates: shard.Available, + }, + topotestutils.SourceAvailableHost{ + Name: notSelfID1, + Shards: shards, + ShardStates: shard.Leaving, + }, + topotestutils.SourceAvailableHost{ + Name: notSelfID2, + Shards: shards, + ShardStates: shard.Available, + }, + topotestutils.SourceAvailableHost{ + Name: notSelfID3, + Shards: shards, + ShardStates: shard.Initializing, + }, + }, + shardsTimeRangesToBootstrap: shardTimeRangesToBootstrap, + expectedAvailableShardsTimeRanges: result.ShardTimeRanges{}, + }, + // Snould return that it can't bootstrap anything because we don't + // know how to interpret the unknown host. + { + title: "Multi node - One node unknown", + majorityReplicas: 2, + hosts: []topotestutils.SourceAvailableHost{ + topotestutils.SourceAvailableHost{ + Name: topotestutils.SelfID, + Shards: shards, + ShardStates: shard.Available, + }, + topotestutils.SourceAvailableHost{ + Name: notSelfID1, + Shards: shards, + ShardStates: shard.Available, + }, + topotestutils.SourceAvailableHost{ + Name: notSelfID2, + Shards: shards, + ShardStates: shard.Unknown, + }, + }, + shardsTimeRangesToBootstrap: shardTimeRangesToBootstrap, + expectedAvailableShardsTimeRanges: result.ShardTimeRanges{}, + }, + } + + for _, tc := range testCases { + t.Run(tc.title, func(t *testing.T) { + + srcOpts := NewOptions().SetInstrumentOptions(instrument.NewOptions()) + src, err := NewUninitializedSource(srcOpts) + require.NoError(t, err) + + runOpts := testDefaultRunOpts.SetInitialTopologyState(tc.hosts.TopologyState(tc.majorityReplicas)) + dataRes := src.AvailableData(nsMetadata, tc.shardsTimeRangesToBootstrap, runOpts) + + require.Equal(t, tc.expectedAvailableShardsTimeRanges, dataRes) + + indexRes := src.AvailableIndex(nsMetadata, tc.shardsTimeRangesToBootstrap, runOpts) + require.Equal(t, tc.expectedAvailableShardsTimeRanges, indexRes) + }) + } +} diff --git a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/types.go b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/types.go new file mode 100644 index 0000000000..6598501f56 --- /dev/null +++ b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/types.go @@ -0,0 +1,35 @@ +// Copyright (c) 2018 Uber Technologies, Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +package uninitialized + +import "github.com/m3db/m3x/instrument" + +// Options is the options interface for the uninitialized source. +type Options interface { + // Return the instrument options. + InstrumentOptions() instrument.Options + + // Set the instrument options. + SetInstrumentOptions(value instrument.Options) Options + + // Validate validates the options are correct. + Validate() error +} diff --git a/src/dbnode/topology/testutil/topology.go b/src/dbnode/topology/testutil/topology.go index 446b0c6c3b..169b569620 100644 --- a/src/dbnode/topology/testutil/topology.go +++ b/src/dbnode/topology/testutil/topology.go @@ -28,6 +28,10 @@ import ( "github.com/m3db/m3cluster/shard" ) +const ( + SelfID = "self" +) + // MustNewTopologyMap returns a new topology.Map with provided parameters. // It's a utility method to make tests easier to write. func MustNewTopologyMap( @@ -101,3 +105,41 @@ func (v TopologyView) Map() (topology.Map, error) { return topology.NewStaticMap(opts), nil } + +// SourceAvailableHost is a human-friendly way of constructing +// TopologyStates for test cases. +type SourceAvailableHost struct { + Name string + Shards []uint32 + ShardStates shard.State +} + +// SourceAvailableHosts is a slice of SourceAvailableHost. +type SourceAvailableHosts []SourceAvailableHost + +// TopologyState returns a topology.StateSnapshot that is equivalent to +// the slice of SourceAvailableHosts. +func (s SourceAvailableHosts) TopologyState(numMajorityReplicas int) *topology.StateSnapshot { + topoState := &topology.StateSnapshot{ + Origin: topology.NewHost(SelfID, "127.0.0.1"), + MajorityReplicas: numMajorityReplicas, + ShardStates: make(map[topology.ShardID]map[topology.HostID]topology.HostShardState), + } + + for _, host := range s { + for _, shard := range host.Shards { + hostShardStates, ok := topoState.ShardStates[topology.ShardID(shard)] + if !ok { + hostShardStates = make(map[topology.HostID]topology.HostShardState) + } + + hostShardStates[topology.HostID(host.Name)] = topology.HostShardState{ + Host: topology.NewHost(host.Name, host.Name+"address"), + ShardState: host.ShardStates, + } + topoState.ShardStates[topology.ShardID(shard)] = hostShardStates + } + } + + return topoState +} From 77fe9888ce30d8b560835e149a57fba0d996e0a8 Mon Sep 17 00:00:00 2001 From: Richard Artoul Date: Mon, 10 Sep 2018 12:56:56 -0400 Subject: [PATCH 02/28] Improve comments --- .../bootstrapper/peers/source_test.go | 21 ++++++++++++------- .../bootstrapper/uninitialized/source.go | 4 +++- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/src/dbnode/storage/bootstrap/bootstrapper/peers/source_test.go b/src/dbnode/storage/bootstrap/bootstrapper/peers/source_test.go index eb0e9f3c26..84c084498f 100644 --- a/src/dbnode/storage/bootstrap/bootstrapper/peers/source_test.go +++ b/src/dbnode/storage/bootstrap/bootstrapper/peers/source_test.go @@ -35,6 +35,11 @@ import ( "github.com/stretchr/testify/require" ) +const ( + notSelfID1 = "not-self1" + notSelfID2 = "not-self2" +) + func TestPeersSourceAvailableDataAndIndex(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() @@ -89,12 +94,12 @@ func TestPeersSourceAvailableDataAndIndex(t *testing.T) { ShardStates: shard.Available, }, topotestutils.SourceAvailableHost{ - Name: "other1", + Name: notSelfID1, Shards: shards, ShardStates: shard.Initializing, }, topotestutils.SourceAvailableHost{ - Name: "other2", + Name: notSelfID2, Shards: shards, ShardStates: shard.Unknown, }, @@ -113,12 +118,12 @@ func TestPeersSourceAvailableDataAndIndex(t *testing.T) { ShardStates: shard.Initializing, }, topotestutils.SourceAvailableHost{ - Name: "other1", + Name: notSelfID1, Shards: shards, ShardStates: shard.Available, }, topotestutils.SourceAvailableHost{ - Name: "other2", + Name: notSelfID2, Shards: shards, ShardStates: shard.Leaving, }, @@ -136,12 +141,12 @@ func TestPeersSourceAvailableDataAndIndex(t *testing.T) { ShardStates: shard.Initializing, }, topotestutils.SourceAvailableHost{ - Name: "other1", + Name: notSelfID1, Shards: shards, ShardStates: shard.Available, }, topotestutils.SourceAvailableHost{ - Name: "other2", + Name: notSelfID2, Shards: shards, ShardStates: shard.Available, }, @@ -160,12 +165,12 @@ func TestPeersSourceAvailableDataAndIndex(t *testing.T) { ShardStates: shard.Initializing, }, topotestutils.SourceAvailableHost{ - Name: "other1", + Name: notSelfID1, Shards: shards, ShardStates: shard.Available, }, topotestutils.SourceAvailableHost{ - Name: "other2", + Name: notSelfID2, Shards: shards, ShardStates: shard.Available, }, diff --git a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source.go b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source.go index 87b953e78e..052c69c714 100644 --- a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source.go +++ b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source.go @@ -30,13 +30,15 @@ import ( "github.com/m3db/m3cluster/shard" ) -// The purpose of the unitializedSource is succeed bootstraps for any +// The purpose of the unitializedSource is to succeed bootstraps for any // shard/time-ranges if the given shard/namespace combination has never // been completely initialized (is a new namespace). This is required for // allowing us to configure the bootstrappers such that the commitlog // bootstrapper can precede the peers bootstrapper and still suceed bootstraps // for brand new namespaces without permitting unintentional data loss by // putting the noop-all or noop-none bootstrappers at the end of the process. +// Behavior is best understood by reading the test cases for the test: +// TestUnitializedSourceAvailableDataAndAvailableIndex type uninitializedSource struct { opts Options } From 7b1646e7b83f3dd24864f2f71336737d150e9898 Mon Sep 17 00:00:00 2001 From: Richard Artoul Date: Mon, 10 Sep 2018 13:04:44 -0400 Subject: [PATCH 03/28] Improve comments --- .../bootstrapper/uninitialized/source.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source.go b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source.go index 052c69c714..7e401e0d1d 100644 --- a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source.go +++ b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source.go @@ -98,6 +98,17 @@ func (s *uninitializedSource) availability( continue } + // The basic idea for the algorithm is that on a shard-by-shard basis we + // need to determine if the namespace is "new" in the sense that it has + // never been completely initialized (reached a state where all the hosts + // in the topology are "available"). + // In order to determine this, we simply count the number of hosts in the + // "initializing" state. If this number is larger than zero, than the + // namespace is "new". + // The one exception to this case is when we perform topology changes and + // we end up with one extra node that is initializing which should be offset + // by the corresponding node that is leaving. I.E if numInitializing > 0 + // BUT numLeaving >= numInitializing then it is still not a new namespace. var ( numInitializing = 0 numLeaving = 0 @@ -108,6 +119,10 @@ func (s *uninitializedSource) availability( case shard.Initializing: numInitializing++ case shard.Unknown: + // TODO: Right now we ignore unknown shards (which biases us towards + // failing the bootstrap). Once this interface supports returning errors, + // we should return an error in this case because the cluster is in a bad + // state and the operator should make a decision on how they want to proceed. case shard.Leaving: numLeaving++ case shard.Available: From bfad99fcee08dd0293aaad2384a2a50c9b27f054 Mon Sep 17 00:00:00 2001 From: Richard Artoul Date: Mon, 10 Sep 2018 13:05:29 -0400 Subject: [PATCH 04/28] Rename var --- .../storage/bootstrap/bootstrapper/uninitialized/source.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source.go b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source.go index 7e401e0d1d..8d732a5bd0 100644 --- a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source.go +++ b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source.go @@ -133,8 +133,8 @@ func (s *uninitializedSource) availability( } } - isNewNamespace := numInitializing-numLeaving > 0 - if isNewNamespace { + shardHasEverBeenCompletelyInitialized := numInitializing-numLeaving > 0 + if shardHasEverBeenCompletelyInitialized { availableShardTimeRanges[shardIDUint] = shardsTimeRanges[shardIDUint] } } From 11367e10a8d6f441f2eef7f415cbfa5295c33874 Mon Sep 17 00:00:00 2001 From: Richard Artoul Date: Mon, 10 Sep 2018 13:05:56 -0400 Subject: [PATCH 05/28] Improve comment --- .../storage/bootstrap/bootstrapper/uninitialized/source.go | 1 + 1 file changed, 1 insertion(+) diff --git a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source.go b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source.go index 8d732a5bd0..dbe86d258b 100644 --- a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source.go +++ b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source.go @@ -109,6 +109,7 @@ func (s *uninitializedSource) availability( // we end up with one extra node that is initializing which should be offset // by the corresponding node that is leaving. I.E if numInitializing > 0 // BUT numLeaving >= numInitializing then it is still not a new namespace. + // See the TestUnitializedSourceAvailableDataAndAvailableIndex test for more details. var ( numInitializing = 0 numLeaving = 0 From a8ef19e3ce09e3c94f7c4e6d38d7f4ec90003961 Mon Sep 17 00:00:00 2001 From: Richard Artoul Date: Mon, 10 Sep 2018 13:28:07 -0400 Subject: [PATCH 06/28] Refactor and add uninitialized bootstrapper provider --- .../bootstrap/bootstrapper/commitlog/types.go | 4 +- .../bootstrapper/uninitialized/options.go | 19 ++++- .../bootstrapper/uninitialized/provider.go | 83 +++++++++++++++++++ .../bootstrapper/uninitialized/source.go | 8 +- .../bootstrapper/uninitialized/source_test.go | 15 ++-- .../bootstrapper/uninitialized/types.go | 11 ++- 6 files changed, 122 insertions(+), 18 deletions(-) create mode 100644 src/dbnode/storage/bootstrap/bootstrapper/uninitialized/provider.go diff --git a/src/dbnode/storage/bootstrap/bootstrapper/commitlog/types.go b/src/dbnode/storage/bootstrap/bootstrapper/commitlog/types.go index 4407f1e8fd..047a100369 100644 --- a/src/dbnode/storage/bootstrap/bootstrapper/commitlog/types.go +++ b/src/dbnode/storage/bootstrap/bootstrapper/commitlog/types.go @@ -30,10 +30,10 @@ type Options interface { // Validate validates the options Validate() error - // SetResultOptions sets the instrumentation options + // SetResultOptions sets the result options SetResultOptions(value result.Options) Options - // ResultOptions returns the instrumentation options + // ResultOptions returns the result options ResultOptions() result.Options // SetCommitLogOptions sets the commit log options diff --git a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/options.go b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/options.go index 555a8fd464..d98b1e9995 100644 --- a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/options.go +++ b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/options.go @@ -23,15 +23,18 @@ package uninitialized import ( "errors" + "github.com/m3db/m3/src/dbnode/storage/bootstrap/result" "github.com/m3db/m3x/instrument" ) var ( errInstrumentOptsNotSet = errors.New("instrument options not set") + errResultOptsNotSet = errors.New("result options not set") ) type options struct { - iOpts instrument.Options + resultOpts result.Options + iOpts instrument.Options } func NewOptions() Options { @@ -43,9 +46,23 @@ func (o *options) Validate() error { return errInstrumentOptsNotSet } + if o.resultOpts == nil { + return errResultOptsNotSet + } + return nil } +func (o *options) SetResultOptions(value result.Options) Options { + opts := *o + opts.resultOpts = value + return &opts +} + +func (o *options) ResultOptions() result.Options { + return o.resultOpts +} + func (o *options) SetInstrumentOptions(value instrument.Options) Options { opts := *o opts.iOpts = value diff --git a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/provider.go b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/provider.go new file mode 100644 index 0000000000..803ee32f42 --- /dev/null +++ b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/provider.go @@ -0,0 +1,83 @@ +// Copyright (c) 2018 Uber Technologies, Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +package uninitialized + +import ( + "github.com/m3db/m3/src/dbnode/storage/bootstrap" + "github.com/m3db/m3/src/dbnode/storage/bootstrap/bootstrapper" +) + +const ( + // UninitializedBootstrapperName is the name of the uninitialized bootstrapper. + UninitializedBootstrapperName = "uninitialized" +) + +type uninitializedBootstrapperProvider struct { + opts Options + next bootstrap.BootstrapperProvider +} + +// NewUninitializedBootstrapperProvider creates a new uninitialized bootstrapper +// provider. +func NewUninitializedBootstrapperProvider( + opts Options, + next bootstrap.BootstrapperProvider, +) (bootstrap.BootstrapperProvider, error) { + if err := opts.Validate(); err != nil { + return nil, err + } + + return uninitializedBootstrapperProvider{ + opts: opts, + next: next, + }, nil +} + +func (p uninitializedBootstrapperProvider) Provide() (bootstrap.Bootstrapper, error) { + var ( + src = newUninitializedSource(p.opts) + b = &uninitializedBootstrapper{} + next bootstrap.Bootstrapper + err error + ) + + if p.next != nil { + next, err = p.next.Provide() + if err != nil { + return nil, err + } + } + + return bootstrapper.NewBaseBootstrapper( + b.String(), src, p.opts.ResultOptions(), next) +} + +func (p uninitializedBootstrapperProvider) String() string { + return UninitializedBootstrapperName +} + +type uninitializedBootstrapper struct { + bootstrap.Bootstrapper +} + +func (*uninitializedBootstrapper) String() string { + return UninitializedBootstrapperName +} diff --git a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source.go b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source.go index dbe86d258b..b7243e73ca 100644 --- a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source.go +++ b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source.go @@ -44,14 +44,10 @@ type uninitializedSource struct { } // NewUninitializedSource creates a new uninitialized source. -func NewUninitializedSource(opts Options) (bootstrap.Source, error) { - if err := opts.Validate(); err != nil { - return nil, err - } - +func newUninitializedSource(opts Options) bootstrap.Source { return &uninitializedSource{ opts: opts, - }, nil + } } func (s *uninitializedSource) Can(strategy bootstrap.Strategy) bool { diff --git a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source_test.go b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source_test.go index cac036804e..e697956bf4 100644 --- a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source_test.go +++ b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source_test.go @@ -266,16 +266,15 @@ func TestUnitializedSourceAvailableDataAndAvailableIndex(t *testing.T) { for _, tc := range testCases { t.Run(tc.title, func(t *testing.T) { - srcOpts := NewOptions().SetInstrumentOptions(instrument.NewOptions()) - src, err := NewUninitializedSource(srcOpts) - require.NoError(t, err) - - runOpts := testDefaultRunOpts.SetInitialTopologyState(tc.hosts.TopologyState(tc.majorityReplicas)) - dataRes := src.AvailableData(nsMetadata, tc.shardsTimeRangesToBootstrap, runOpts) + var ( + srcOpts = NewOptions().SetInstrumentOptions(instrument.NewOptions()) + src = newUninitializedSource(srcOpts) + runOpts = testDefaultRunOpts.SetInitialTopologyState(tc.hosts.TopologyState(tc.majorityReplicas)) + dataRes = src.AvailableData(nsMetadata, tc.shardsTimeRangesToBootstrap, runOpts) + indexRes = src.AvailableIndex(nsMetadata, tc.shardsTimeRangesToBootstrap, runOpts) + ) require.Equal(t, tc.expectedAvailableShardsTimeRanges, dataRes) - - indexRes := src.AvailableIndex(nsMetadata, tc.shardsTimeRangesToBootstrap, runOpts) require.Equal(t, tc.expectedAvailableShardsTimeRanges, indexRes) }) } diff --git a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/types.go b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/types.go index 6598501f56..719ba87c1c 100644 --- a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/types.go +++ b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/types.go @@ -20,10 +20,19 @@ package uninitialized -import "github.com/m3db/m3x/instrument" +import ( + "github.com/m3db/m3/src/dbnode/storage/bootstrap/result" + "github.com/m3db/m3x/instrument" +) // Options is the options interface for the uninitialized source. type Options interface { + // SetResultOptions sets the result options + SetResultOptions(value result.Options) Options + + // ResultOptions returns the result options + ResultOptions() result.Options + // Return the instrument options. InstrumentOptions() instrument.Options From 232fce9e407fd1bd095fb23491983e8e90eee369 Mon Sep 17 00:00:00 2001 From: Richard Artoul Date: Mon, 10 Sep 2018 15:35:01 -0400 Subject: [PATCH 07/28] Fix topology initialization logic --- .../bootstrap_after_buffer_rotation_regression_test.go | 1 + src/dbnode/storage/bootstrap/bootstrapper/commitlog/source.go | 2 +- src/dbnode/storage/bootstrap/process.go | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/dbnode/integration/bootstrap_after_buffer_rotation_regression_test.go b/src/dbnode/integration/bootstrap_after_buffer_rotation_regression_test.go index 9f1c604ee4..05303b9c5a 100644 --- a/src/dbnode/integration/bootstrap_after_buffer_rotation_regression_test.go +++ b/src/dbnode/integration/bootstrap_after_buffer_rotation_regression_test.go @@ -118,6 +118,7 @@ func TestBootstrapAfterBufferRotation(t *testing.T) { signalCh := make(chan struct{}) bootstrapper, err := commitlogBootstrapperProvider.Provide() require.NoError(t, err) + test := newTestBootstrapperSource(testBootstrapperSourceOptions{ readData: func( _ namespace.Metadata, diff --git a/src/dbnode/storage/bootstrap/bootstrapper/commitlog/source.go b/src/dbnode/storage/bootstrap/bootstrapper/commitlog/source.go index 952ada2d38..8f01d8e653 100644 --- a/src/dbnode/storage/bootstrap/bootstrapper/commitlog/source.go +++ b/src/dbnode/storage/bootstrap/bootstrapper/commitlog/source.go @@ -1447,7 +1447,7 @@ func (s *commitLogSource) availability( iOpts := s.opts.CommitLogOptions().InstrumentOptions() invariantLogger := instrument.EmitInvariantViolationAndGetLogger(iOpts) invariantLogger.Errorf( - "Initial topolgoy state does not contain shard state for origin node and shard: %d", shardIDUint) + "Initial topology state does not contain shard state for origin node and shard: %d", shardIDUint) continue } diff --git a/src/dbnode/storage/bootstrap/process.go b/src/dbnode/storage/bootstrap/process.go index 09e391fa4d..9f184a11f6 100644 --- a/src/dbnode/storage/bootstrap/process.go +++ b/src/dbnode/storage/bootstrap/process.go @@ -132,7 +132,7 @@ func (b *bootstrapProcessProvider) newInitialTopologyState() (*topology.StateSna topologyState.ShardStates[shardID] = existing } - hostID := topology.HostID(hostShardSet.Host().String()) + hostID := topology.HostID(hostShardSet.Host().ID()) existing[hostID] = topology.HostShardState{ Host: hostShardSet.Host(), ShardState: currShard.State(), From 47c14739b73c5eb91ded50285e7d723d143ba470 Mon Sep 17 00:00:00 2001 From: Richard Artoul Date: Mon, 10 Sep 2018 15:42:02 -0400 Subject: [PATCH 08/28] Fix lint issues --- .../storage/bootstrap/bootstrapper/uninitialized/options.go | 1 + .../storage/bootstrap/bootstrapper/uninitialized/source.go | 2 +- .../bootstrap/bootstrapper/uninitialized/source_test.go | 3 +-- src/dbnode/topology/testutil/topology.go | 1 + 4 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/options.go b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/options.go index d98b1e9995..32b10f333b 100644 --- a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/options.go +++ b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/options.go @@ -37,6 +37,7 @@ type options struct { iOpts instrument.Options } +// NewOptions creates a new Options. func NewOptions() Options { return &options{} } diff --git a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source.go b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source.go index b7243e73ca..a053f4076f 100644 --- a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source.go +++ b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source.go @@ -34,7 +34,7 @@ import ( // shard/time-ranges if the given shard/namespace combination has never // been completely initialized (is a new namespace). This is required for // allowing us to configure the bootstrappers such that the commitlog -// bootstrapper can precede the peers bootstrapper and still suceed bootstraps +// bootstrapper can precede the peers bootstrapper and still succeed bootstraps // for brand new namespaces without permitting unintentional data loss by // putting the noop-all or noop-none bootstrappers at the end of the process. // Behavior is best understood by reading the test cases for the test: diff --git a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source_test.go b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source_test.go index e697956bf4..fb2b9d1e4f 100644 --- a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source_test.go +++ b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source_test.go @@ -27,12 +27,12 @@ import ( "github.com/m3db/m3/src/dbnode/storage/bootstrap" "github.com/m3db/m3/src/dbnode/storage/bootstrap/result" "github.com/m3db/m3/src/dbnode/storage/namespace" - "github.com/m3db/m3/src/dbnode/topology" topotestutils "github.com/m3db/m3/src/dbnode/topology/testutil" "github.com/m3db/m3cluster/shard" "github.com/m3db/m3x/ident" "github.com/m3db/m3x/instrument" xtime "github.com/m3db/m3x/time" + "github.com/stretchr/testify/require" ) @@ -66,7 +66,6 @@ func TestUnitializedSourceAvailableDataAndAvailableIndex(t *testing.T) { title string majorityReplicas int hosts topotestutils.SourceAvailableHosts - bootstrapReadConsistency topology.ReadConsistencyLevel shardsTimeRangesToBootstrap result.ShardTimeRanges expectedAvailableShardsTimeRanges result.ShardTimeRanges }{ diff --git a/src/dbnode/topology/testutil/topology.go b/src/dbnode/topology/testutil/topology.go index 169b569620..03410818d4 100644 --- a/src/dbnode/topology/testutil/topology.go +++ b/src/dbnode/topology/testutil/topology.go @@ -29,6 +29,7 @@ import ( ) const ( + // SelfID is the string used to represent the ID of the origin node. SelfID = "self" ) From 6f5398504ad6f36a187ea4fc995ac49e416b108e Mon Sep 17 00:00:00 2001 From: Richard Artoul Date: Mon, 10 Sep 2018 16:10:55 -0400 Subject: [PATCH 09/28] Fix prop test --- .../commitlog/source_prop_test.go | 28 +++++++++++++++---- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/src/dbnode/storage/bootstrap/bootstrapper/commitlog/source_prop_test.go b/src/dbnode/storage/bootstrap/bootstrapper/commitlog/source_prop_test.go index 1097e3c887..12a2332775 100644 --- a/src/dbnode/storage/bootstrap/bootstrapper/commitlog/source_prop_test.go +++ b/src/dbnode/storage/bootstrap/bootstrapper/commitlog/source_prop_test.go @@ -40,7 +40,9 @@ import ( "github.com/m3db/m3/src/dbnode/persist/fs/commitlog" "github.com/m3db/m3/src/dbnode/storage/bootstrap/result" "github.com/m3db/m3/src/dbnode/storage/namespace" + topotestutils "github.com/m3db/m3/src/dbnode/topology/testutil" "github.com/m3db/m3/src/dbnode/ts" + "github.com/m3db/m3cluster/shard" "github.com/m3db/m3x/checked" "github.com/m3db/m3x/context" "github.com/m3db/m3x/ident" @@ -306,19 +308,35 @@ func TestCommitLogSourcePropCorrectlyBootstrapsFromCommitlog(t *testing.T) { // Determine which shards we need to bootstrap (based on the randomly // generated data) - allShards := map[uint32]bool{} + var ( + allShardsMap = map[uint32]bool{} + allShardsSlice = []uint32{} + ) for _, write := range input.writes { - allShards[write.series.Shard] = true + shard := write.series.Shard + if _, ok := allShardsMap[shard]; !ok { + allShardsSlice = append(allShardsSlice, shard) + } + allShardsMap[shard] = true } // Assign the previously-determined bootstrap range to each known shard shardTimeRanges := result.ShardTimeRanges{} - for shard := range allShards { + for shard := range allShardsMap { shardTimeRanges[shard] = ranges } // Perform the bootstrap - runOpts := testDefaultRunOpts + var ( + initialTopoState = topotestutils.SourceAvailableHosts{ + topotestutils.SourceAvailableHost{ + Name: topotestutils.SelfID, + Shards: allShardsSlice, + ShardStates: shard.Available, + }, + }.TopologyState(1) + runOpts = testDefaultRunOpts.SetInitialTopologyState(initialTopoState) + ) dataResult, err := source.BootstrapData(nsMeta, shardTimeRanges, runOpts) if err != nil { return false, err @@ -341,7 +359,7 @@ func TestCommitLogSourcePropCorrectlyBootstrapsFromCommitlog(t *testing.T) { return false, err } - indexResult, err := source.BootstrapIndex(nsMeta, shardTimeRanges, testDefaultRunOpts) + indexResult, err := source.BootstrapIndex(nsMeta, shardTimeRanges, runOpts) if err != nil { return false, err } From a3d2bf116cdb9522d900f0f6c372802d9c0db540 Mon Sep 17 00:00:00 2001 From: Richard Artoul Date: Mon, 10 Sep 2018 16:29:24 -0400 Subject: [PATCH 10/28] Fix lint issues and rebase --- .../storage/bootstrap/bootstrapper/commitlog/source_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/src/dbnode/storage/bootstrap/bootstrapper/commitlog/source_test.go b/src/dbnode/storage/bootstrap/bootstrapper/commitlog/source_test.go index 4eea17de4a..8dd77c70df 100644 --- a/src/dbnode/storage/bootstrap/bootstrapper/commitlog/source_test.go +++ b/src/dbnode/storage/bootstrap/bootstrapper/commitlog/source_test.go @@ -29,6 +29,7 @@ import ( topotestutils "github.com/m3db/m3/src/dbnode/topology/testutil" "github.com/m3db/m3cluster/shard" xtime "github.com/m3db/m3x/time" + "github.com/stretchr/testify/require" ) From 4e0e180eb359aed18a35f64dcda5dea422633b32 Mon Sep 17 00:00:00 2001 From: Richard Artoul Date: Mon, 10 Sep 2018 16:55:25 -0400 Subject: [PATCH 11/28] Fix broken tests --- src/cmd/services/m3dbnode/config/bootstrap.go | 27 +++++++++++---- src/cmd/services/m3dbnode/main/common_test.go | 34 +++++++++++++++++++ .../services/m3dbnode/main/main_index_test.go | 1 + src/cmd/services/m3dbnode/main/main_test.go | 33 ------------------ 4 files changed, 56 insertions(+), 39 deletions(-) diff --git a/src/cmd/services/m3dbnode/config/bootstrap.go b/src/cmd/services/m3dbnode/config/bootstrap.go index b74c377618..6a31a85299 100644 --- a/src/cmd/services/m3dbnode/config/bootstrap.go +++ b/src/cmd/services/m3dbnode/config/bootstrap.go @@ -33,6 +33,7 @@ import ( "github.com/m3db/m3/src/dbnode/storage/bootstrap/bootstrapper/commitlog" bfs "github.com/m3db/m3/src/dbnode/storage/bootstrap/bootstrapper/fs" "github.com/m3db/m3/src/dbnode/storage/bootstrap/bootstrapper/peers" + "github.com/m3db/m3/src/dbnode/storage/bootstrap/bootstrapper/uninitialized" "github.com/m3db/m3/src/dbnode/storage/bootstrap/result" "github.com/m3db/m3/src/dbnode/storage/index" ) @@ -120,7 +121,7 @@ func (bsc BootstrapConfiguration) New( case bootstrapper.NoOpNoneBootstrapperName: bs = bootstrapper.NewNoOpNoneBootstrapperProvider() case bfs.FileSystemBootstrapperName: - fsbopts := bfs.NewOptions(). + fsbOpts := bfs.NewOptions(). SetInstrumentOptions(opts.InstrumentOptions()). SetResultOptions(rsOpts). SetFilesystemOptions(fsOpts). @@ -129,12 +130,12 @@ func (bsc BootstrapConfiguration) New( SetDatabaseBlockRetrieverManager(opts.DatabaseBlockRetrieverManager()). SetRuntimeOptionsManager(opts.RuntimeOptionsManager()). SetIdentifierPool(opts.IdentifierPool()) - bs, err = bfs.NewFileSystemBootstrapperProvider(fsbopts, bs) + bs, err = bfs.NewFileSystemBootstrapperProvider(fsbOpts, bs) if err != nil { return nil, err } case commitlog.CommitLogBootstrapperName: - copts := commitlog.NewOptions(). + cOpts := commitlog.NewOptions(). SetResultOptions(rsOpts). SetCommitLogOptions(opts.CommitLogOptions()) @@ -142,19 +143,27 @@ func (bsc BootstrapConfiguration) New( if err != nil { return nil, err } - bs, err = commitlog.NewCommitLogBootstrapperProvider(copts, inspection, bs) + bs, err = commitlog.NewCommitLogBootstrapperProvider(cOpts, inspection, bs) if err != nil { return nil, err } case peers.PeersBootstrapperName: - popts := peers.NewOptions(). + pOpts := peers.NewOptions(). SetResultOptions(rsOpts). SetAdminClient(adminClient). SetPersistManager(opts.PersistManager()). SetDatabaseBlockRetrieverManager(opts.DatabaseBlockRetrieverManager()). SetFetchBlocksMetadataEndpointVersion(bsc.peersFetchBlocksMetadataEndpointVersion()). SetRuntimeOptionsManager(opts.RuntimeOptionsManager()) - bs, err = peers.NewPeersBootstrapperProvider(popts, bs) + bs, err = peers.NewPeersBootstrapperProvider(pOpts, bs) + if err != nil { + return nil, err + } + case uninitialized.UninitializedBootstrapperName: + uopts := uninitialized.NewOptions(). + SetResultOptions(rsOpts). + SetInstrumentOptions(opts.InstrumentOptions()) + bs, err = uninitialized.NewUninitializedBootstrapperProvider(uopts, bs) if err != nil { return nil, err } @@ -195,6 +204,12 @@ func ValidateBootstrappersOrder(names []string) error { bfs.FileSystemBootstrapperName, peers.PeersBootstrapperName, }, + uninitialized.UninitializedBootstrapperName: []string{ + // Unintialized bootstrapper may appear after filesystem or peers or commitlog + bfs.FileSystemBootstrapperName, + commitlog.CommitLogBootstrapperName, + peers.PeersBootstrapperName, + }, } validated := make(map[string]struct{}) diff --git a/src/cmd/services/m3dbnode/main/common_test.go b/src/cmd/services/m3dbnode/main/common_test.go index a48cfed732..15791dfe10 100644 --- a/src/cmd/services/m3dbnode/main/common_test.go +++ b/src/cmd/services/m3dbnode/main/common_test.go @@ -34,8 +34,10 @@ import ( "time" "github.com/gogo/protobuf/proto" + "github.com/m3db/m3/src/dbnode/client" "github.com/m3db/m3/src/dbnode/retention" "github.com/m3db/m3/src/dbnode/storage/namespace" + "github.com/m3db/m3cluster/shard" "github.com/m3db/m3x/ident" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -139,3 +141,35 @@ func newNamespaceWithIndexProtoValue(id string, indexEnabled bool) (proto.Messag } return namespace.ToProto(nsMap), nil } + +// waitUntilAllShardsAreAvailable continually polls the session checking to see if the topology.Map +// that the session is currently storing contains a non-zero number of host shard sets, and if so, +// makes sure that all their shard states are Available. +func waitUntilAllShardsAreAvailable(t *testing.T, session client.AdminSession) { +outer: + for { + time.Sleep(10 * time.Millisecond) + + topoMap, err := session.TopologyMap() + require.NoError(t, err) + + var ( + hostShardSets = topoMap.HostShardSets() + ) + + if len(hostShardSets) == 0 { + // We haven't received an actual topology yet. + continue + } + + for _, hostShardSet := range hostShardSets { + for _, hostShard := range hostShardSet.ShardSet().All() { + if hostShard.State() != shard.Available { + continue outer + } + } + } + + break + } +} diff --git a/src/cmd/services/m3dbnode/main/main_index_test.go b/src/cmd/services/m3dbnode/main/main_index_test.go index 3f9678934d..f683374e62 100644 --- a/src/cmd/services/m3dbnode/main/main_index_test.go +++ b/src/cmd/services/m3dbnode/main/main_index_test.go @@ -327,6 +327,7 @@ db: bootstrappers: - filesystem - commitlog + - uninitialized fs: numProcessorsPerCPU: 0.125 diff --git a/src/cmd/services/m3dbnode/main/main_test.go b/src/cmd/services/m3dbnode/main/main_test.go index 0173bfe5ef..90285ff96b 100644 --- a/src/cmd/services/m3dbnode/main/main_test.go +++ b/src/cmd/services/m3dbnode/main/main_test.go @@ -37,7 +37,6 @@ import ( "github.com/m3db/m3cluster/integration/etcd" "github.com/m3db/m3cluster/placement" "github.com/m3db/m3cluster/services" - "github.com/m3db/m3cluster/shard" xconfig "github.com/m3db/m3x/config" "github.com/m3db/m3x/ident" "github.com/m3db/m3x/instrument" @@ -632,35 +631,3 @@ db: endpoint: {{.InitialClusterEndpoint}} ` ) - -// waitUntilAllShardsAreAvailable continually polls the session checking to see if the topology.Map -// that the session is currently storing contains a non-zero number of host shard sets, and if so, -// makes sure that all their shard states are Available. -func waitUntilAllShardsAreAvailable(t *testing.T, session client.AdminSession) { -outer: - for { - time.Sleep(10 * time.Millisecond) - - topoMap, err := session.TopologyMap() - require.NoError(t, err) - - var ( - hostShardSets = topoMap.HostShardSets() - ) - - if len(hostShardSets) == 0 { - // We haven't received an actual topology yet. - continue - } - - for _, hostShardSet := range hostShardSets { - for _, hostShard := range hostShardSet.ShardSet().All() { - if hostShard.State() != shard.Available { - continue outer - } - } - } - - break - } -} From a1fe6c22317368f06198b228d857d863148387c1 Mon Sep 17 00:00:00 2001 From: Richard Artoul Date: Mon, 10 Sep 2018 17:13:25 -0400 Subject: [PATCH 12/28] Fix broken test --- src/cmd/services/m3dbnode/main/main_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/src/cmd/services/m3dbnode/main/main_test.go b/src/cmd/services/m3dbnode/main/main_test.go index 90285ff96b..2896d91c0a 100644 --- a/src/cmd/services/m3dbnode/main/main_test.go +++ b/src/cmd/services/m3dbnode/main/main_test.go @@ -487,6 +487,7 @@ db: bootstrappers: - filesystem - commitlog + - uninitialized fs: numProcessorsPerCPU: 0.125 From 139b87a9e30eebcdb42feeccc72a33ca1fc892cc Mon Sep 17 00:00:00 2001 From: Richard Artoul Date: Mon, 10 Sep 2018 17:28:58 -0400 Subject: [PATCH 13/28] Update config files --- kube/bundle.yaml | 1 + kube/m3dbnode-configmap.yaml | 1 + src/dbnode/config/m3dbnode-local-etcd.yml | 2 +- src/dbnode/config/m3dbnode-local.yml | 2 +- src/dbnode/example/m3db-node-config.yaml | 1 + src/query/benchmark/benchmarker/main/m3dbnode-local-config.yaml | 1 + src/query/benchmark/configs/m3db_config.yaml | 1 + .../configs/multi_node_setup/m3dbnode-server1-config.yaml | 1 + .../configs/multi_node_setup/m3dbnode-server2-config.yaml | 1 + .../configs/multi_node_setup/m3dbnode-server3-config.yaml | 1 + 10 files changed, 10 insertions(+), 2 deletions(-) diff --git a/kube/bundle.yaml b/kube/bundle.yaml index f9df985bb3..8363aad91f 100644 --- a/kube/bundle.yaml +++ b/kube/bundle.yaml @@ -176,6 +176,7 @@ data: bootstrappers: - filesystem - commitlog + - uninitialized fs: numProcessorsPerCPU: 0.125 diff --git a/kube/m3dbnode-configmap.yaml b/kube/m3dbnode-configmap.yaml index 277663bc6f..94ea47aed1 100644 --- a/kube/m3dbnode-configmap.yaml +++ b/kube/m3dbnode-configmap.yaml @@ -68,6 +68,7 @@ data: bootstrappers: - filesystem - commitlog + - uninitialized fs: numProcessorsPerCPU: 0.125 diff --git a/src/dbnode/config/m3dbnode-local-etcd.yml b/src/dbnode/config/m3dbnode-local-etcd.yml index 86ada7bde1..7c202aa98b 100644 --- a/src/dbnode/config/m3dbnode-local-etcd.yml +++ b/src/dbnode/config/m3dbnode-local-etcd.yml @@ -63,7 +63,7 @@ db: bootstrappers: - filesystem - commitlog - - noop-none + - uninitialized fs: numProcessorsPerCPU: 0.125 diff --git a/src/dbnode/config/m3dbnode-local.yml b/src/dbnode/config/m3dbnode-local.yml index 2762db550a..939d7a7929 100644 --- a/src/dbnode/config/m3dbnode-local.yml +++ b/src/dbnode/config/m3dbnode-local.yml @@ -63,7 +63,7 @@ db: bootstrappers: - filesystem - commitlog - - noop-none + - uninitialized fs: numProcessorsPerCPU: 0.125 diff --git a/src/dbnode/example/m3db-node-config.yaml b/src/dbnode/example/m3db-node-config.yaml index 7b668eed79..1748d51129 100644 --- a/src/dbnode/example/m3db-node-config.yaml +++ b/src/dbnode/example/m3db-node-config.yaml @@ -47,6 +47,7 @@ bootstrap: bootstrappers: - filesystem - commitlog + - uninitialized fs: numProcessorsPerCPU: 0.125 diff --git a/src/query/benchmark/benchmarker/main/m3dbnode-local-config.yaml b/src/query/benchmark/benchmarker/main/m3dbnode-local-config.yaml index 9c8613fdf3..746ee5abeb 100644 --- a/src/query/benchmark/benchmarker/main/m3dbnode-local-config.yaml +++ b/src/query/benchmark/benchmarker/main/m3dbnode-local-config.yaml @@ -50,6 +50,7 @@ bootstrap: bootstrappers: - filesystem - commitlog + - uninitialized fs: numProcessorsPerCPU: 0.125 diff --git a/src/query/benchmark/configs/m3db_config.yaml b/src/query/benchmark/configs/m3db_config.yaml index 70eaca9b9f..852c345d31 100644 --- a/src/query/benchmark/configs/m3db_config.yaml +++ b/src/query/benchmark/configs/m3db_config.yaml @@ -49,6 +49,7 @@ bootstrap: bootstrappers: - filesystem - commitlog + - uninitialized fs: numProcessorsPerCPU: 0.125 diff --git a/src/query/benchmark/configs/multi_node_setup/m3dbnode-server1-config.yaml b/src/query/benchmark/configs/multi_node_setup/m3dbnode-server1-config.yaml index f92e5b92f9..1c24a454d5 100644 --- a/src/query/benchmark/configs/multi_node_setup/m3dbnode-server1-config.yaml +++ b/src/query/benchmark/configs/multi_node_setup/m3dbnode-server1-config.yaml @@ -48,6 +48,7 @@ bootstrap: bootstrappers: - filesystem - commitlog + - uninitialized fs: numProcessorsPerCPU: 0.125 diff --git a/src/query/benchmark/configs/multi_node_setup/m3dbnode-server2-config.yaml b/src/query/benchmark/configs/multi_node_setup/m3dbnode-server2-config.yaml index fd913b50a7..6e246e52f0 100644 --- a/src/query/benchmark/configs/multi_node_setup/m3dbnode-server2-config.yaml +++ b/src/query/benchmark/configs/multi_node_setup/m3dbnode-server2-config.yaml @@ -48,6 +48,7 @@ bootstrap: bootstrappers: - filesystem - commitlog + - uninitialized fs: numProcessorsPerCPU: 0.125 diff --git a/src/query/benchmark/configs/multi_node_setup/m3dbnode-server3-config.yaml b/src/query/benchmark/configs/multi_node_setup/m3dbnode-server3-config.yaml index 2488a5e822..750bd4e9ff 100644 --- a/src/query/benchmark/configs/multi_node_setup/m3dbnode-server3-config.yaml +++ b/src/query/benchmark/configs/multi_node_setup/m3dbnode-server3-config.yaml @@ -48,6 +48,7 @@ bootstrap: bootstrappers: - filesystem - commitlog + - uninitialized fs: numProcessorsPerCPU: 0.125 From 4ac6997bde79343a8b94f2d7c83d9c8bf842a7d4 Mon Sep 17 00:00:00 2001 From: Richard Artoul Date: Tue, 11 Sep 2018 14:16:00 -0400 Subject: [PATCH 14/28] Address review feedback --- kube/bundle.yaml | 1 + kube/m3dbnode-configmap.yaml | 1 + src/cmd/services/m3dbnode/config/bootstrap.go | 2 ++ src/cmd/services/m3dbnode/main/main_index_test.go | 1 + src/cmd/services/m3dbnode/main/main_test.go | 1 + src/dbnode/config/m3dbnode-local-etcd.yml | 1 + src/dbnode/config/m3dbnode-local.yml | 1 + src/dbnode/example/m3db-node-config.yaml | 1 + src/query/benchmark/benchmarker/main/m3dbnode-local-config.yaml | 1 + src/query/benchmark/configs/m3db_config.yaml | 1 + .../configs/multi_node_setup/m3dbnode-server1-config.yaml | 1 + .../configs/multi_node_setup/m3dbnode-server2-config.yaml | 1 + .../configs/multi_node_setup/m3dbnode-server3-config.yaml | 1 + 13 files changed, 14 insertions(+) diff --git a/kube/bundle.yaml b/kube/bundle.yaml index 8363aad91f..5b9803cc60 100644 --- a/kube/bundle.yaml +++ b/kube/bundle.yaml @@ -176,6 +176,7 @@ data: bootstrappers: - filesystem - commitlog + - peers - uninitialized fs: numProcessorsPerCPU: 0.125 diff --git a/kube/m3dbnode-configmap.yaml b/kube/m3dbnode-configmap.yaml index 94ea47aed1..37d1fc1312 100644 --- a/kube/m3dbnode-configmap.yaml +++ b/kube/m3dbnode-configmap.yaml @@ -68,6 +68,7 @@ data: bootstrappers: - filesystem - commitlog + - peers - uninitialized fs: numProcessorsPerCPU: 0.125 diff --git a/src/cmd/services/m3dbnode/config/bootstrap.go b/src/cmd/services/m3dbnode/config/bootstrap.go index 6a31a85299..e1cbfa42d1 100644 --- a/src/cmd/services/m3dbnode/config/bootstrap.go +++ b/src/cmd/services/m3dbnode/config/bootstrap.go @@ -198,6 +198,8 @@ func ValidateBootstrappersOrder(names []string) error { peers.PeersBootstrapperName: []string{ // Peers must always appear after filesystem bfs.FileSystemBootstrapperName, + // Peers may appear before OR after commitlog + commitlog.CommitLogBootstrapperName, }, commitlog.CommitLogBootstrapperName: []string{ // Commit log bootstrapper may appear after filesystem or peers diff --git a/src/cmd/services/m3dbnode/main/main_index_test.go b/src/cmd/services/m3dbnode/main/main_index_test.go index f683374e62..2a8855866d 100644 --- a/src/cmd/services/m3dbnode/main/main_index_test.go +++ b/src/cmd/services/m3dbnode/main/main_index_test.go @@ -327,6 +327,7 @@ db: bootstrappers: - filesystem - commitlog + - peers - uninitialized fs: numProcessorsPerCPU: 0.125 diff --git a/src/cmd/services/m3dbnode/main/main_test.go b/src/cmd/services/m3dbnode/main/main_test.go index 2896d91c0a..ff8a57bdd9 100644 --- a/src/cmd/services/m3dbnode/main/main_test.go +++ b/src/cmd/services/m3dbnode/main/main_test.go @@ -487,6 +487,7 @@ db: bootstrappers: - filesystem - commitlog + - peers - uninitialized fs: numProcessorsPerCPU: 0.125 diff --git a/src/dbnode/config/m3dbnode-local-etcd.yml b/src/dbnode/config/m3dbnode-local-etcd.yml index 7c202aa98b..5a57b11c3c 100644 --- a/src/dbnode/config/m3dbnode-local-etcd.yml +++ b/src/dbnode/config/m3dbnode-local-etcd.yml @@ -63,6 +63,7 @@ db: bootstrappers: - filesystem - commitlog + - peers - uninitialized fs: numProcessorsPerCPU: 0.125 diff --git a/src/dbnode/config/m3dbnode-local.yml b/src/dbnode/config/m3dbnode-local.yml index 939d7a7929..ce10d238a3 100644 --- a/src/dbnode/config/m3dbnode-local.yml +++ b/src/dbnode/config/m3dbnode-local.yml @@ -63,6 +63,7 @@ db: bootstrappers: - filesystem - commitlog + - peers - uninitialized fs: numProcessorsPerCPU: 0.125 diff --git a/src/dbnode/example/m3db-node-config.yaml b/src/dbnode/example/m3db-node-config.yaml index 1748d51129..6c5304f132 100644 --- a/src/dbnode/example/m3db-node-config.yaml +++ b/src/dbnode/example/m3db-node-config.yaml @@ -47,6 +47,7 @@ bootstrap: bootstrappers: - filesystem - commitlog + - peers - uninitialized fs: numProcessorsPerCPU: 0.125 diff --git a/src/query/benchmark/benchmarker/main/m3dbnode-local-config.yaml b/src/query/benchmark/benchmarker/main/m3dbnode-local-config.yaml index 746ee5abeb..7fe475fa37 100644 --- a/src/query/benchmark/benchmarker/main/m3dbnode-local-config.yaml +++ b/src/query/benchmark/benchmarker/main/m3dbnode-local-config.yaml @@ -50,6 +50,7 @@ bootstrap: bootstrappers: - filesystem - commitlog + - peers - uninitialized fs: numProcessorsPerCPU: 0.125 diff --git a/src/query/benchmark/configs/m3db_config.yaml b/src/query/benchmark/configs/m3db_config.yaml index 852c345d31..50fc1eadc8 100644 --- a/src/query/benchmark/configs/m3db_config.yaml +++ b/src/query/benchmark/configs/m3db_config.yaml @@ -49,6 +49,7 @@ bootstrap: bootstrappers: - filesystem - commitlog + - peers - uninitialized fs: numProcessorsPerCPU: 0.125 diff --git a/src/query/benchmark/configs/multi_node_setup/m3dbnode-server1-config.yaml b/src/query/benchmark/configs/multi_node_setup/m3dbnode-server1-config.yaml index 1c24a454d5..d63b571944 100644 --- a/src/query/benchmark/configs/multi_node_setup/m3dbnode-server1-config.yaml +++ b/src/query/benchmark/configs/multi_node_setup/m3dbnode-server1-config.yaml @@ -48,6 +48,7 @@ bootstrap: bootstrappers: - filesystem - commitlog + - peers - uninitialized fs: numProcessorsPerCPU: 0.125 diff --git a/src/query/benchmark/configs/multi_node_setup/m3dbnode-server2-config.yaml b/src/query/benchmark/configs/multi_node_setup/m3dbnode-server2-config.yaml index 6e246e52f0..b1e8b37c29 100644 --- a/src/query/benchmark/configs/multi_node_setup/m3dbnode-server2-config.yaml +++ b/src/query/benchmark/configs/multi_node_setup/m3dbnode-server2-config.yaml @@ -48,6 +48,7 @@ bootstrap: bootstrappers: - filesystem - commitlog + - peers - uninitialized fs: numProcessorsPerCPU: 0.125 diff --git a/src/query/benchmark/configs/multi_node_setup/m3dbnode-server3-config.yaml b/src/query/benchmark/configs/multi_node_setup/m3dbnode-server3-config.yaml index 750bd4e9ff..e35c81e87d 100644 --- a/src/query/benchmark/configs/multi_node_setup/m3dbnode-server3-config.yaml +++ b/src/query/benchmark/configs/multi_node_setup/m3dbnode-server3-config.yaml @@ -48,6 +48,7 @@ bootstrap: bootstrappers: - filesystem - commitlog + - peers - uninitialized fs: numProcessorsPerCPU: 0.125 From c82ffe37034a0665ef30a7a907a8795a9ed364bf Mon Sep 17 00:00:00 2001 From: Richard Artoul Date: Wed, 12 Sep 2018 16:39:52 -0400 Subject: [PATCH 15/28] Address review feedback --- .../bootstrap/bootstrapper/commitlog/source.go | 10 ++++++---- .../storage/bootstrap/bootstrapper/peers/source.go | 4 ++-- .../bootstrap/bootstrapper/uninitialized/source.go | 12 ++++-------- 3 files changed, 12 insertions(+), 14 deletions(-) diff --git a/src/dbnode/storage/bootstrap/bootstrapper/commitlog/source.go b/src/dbnode/storage/bootstrap/bootstrapper/commitlog/source.go index 8f01d8e653..ad6d23d251 100644 --- a/src/dbnode/storage/bootstrap/bootstrapper/commitlog/source.go +++ b/src/dbnode/storage/bootstrap/bootstrapper/commitlog/source.go @@ -1447,16 +1447,15 @@ func (s *commitLogSource) availability( iOpts := s.opts.CommitLogOptions().InstrumentOptions() invariantLogger := instrument.EmitInvariantViolationAndGetLogger(iOpts) invariantLogger.Errorf( - "Initial topology state does not contain shard state for origin node and shard: %d", shardIDUint) + "initial topology state does not contain shard state for origin node and shard: %d", shardIDUint) continue } originShardState := originHostShardState.ShardState switch originShardState { - // In the Initializing and Unknown states we have to assume that the commit log + // In the Initializing state we have to assume that the commit log // is missing data and can't satisfy the bootstrap request. case shard.Initializing: - case shard.Unknown: // In the Leaving and Available case, we assume that the commit log contains // all the data required to satisfy the bootstrap request because the node // had (at some point) been completely bootstrapped for the requested shard. @@ -1475,10 +1474,13 @@ func (s *commitLogSource) availability( // modify this to only say the commit log bootstrapper can fullfil // "unfulfilled" data, but not corrupt data. availableShardTimeRanges[shardIDUint] = shardsTimeRanges[shardIDUint] + case shard.Unknown: + fallthrough default: // TODO(rartoul): Make this a hard error once we refactor the interface to support // returning errors. - panic(fmt.Sprintf("unknown shard state: %v", originShardState)) + s.log.Errorf("unknown shard state: %v", originShardState) + return result.ShardTimeRanges{} } } diff --git a/src/dbnode/storage/bootstrap/bootstrapper/peers/source.go b/src/dbnode/storage/bootstrap/bootstrapper/peers/source.go index 814dac2b9c..9080f2be6d 100644 --- a/src/dbnode/storage/bootstrap/bootstrapper/peers/source.go +++ b/src/dbnode/storage/bootstrap/bootstrapper/peers/source.go @@ -724,8 +724,8 @@ func (s *peersSource) peerAvailability( default: // TODO(rartoul): Make this a hard error once we refactor the interface to support // returning errors. - panic( - fmt.Sprintf("encountered unknown shard state: %s", shardState.String())) + s.log.Errorf("unknown shard state: %v", shardState) + return result.ShardTimeRanges{} } } } diff --git a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source.go b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source.go index a053f4076f..a92600c215 100644 --- a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source.go +++ b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source.go @@ -21,8 +21,6 @@ package uninitialized import ( - "fmt" - "github.com/m3db/m3/src/dbnode/storage/bootstrap" "github.com/m3db/m3/src/dbnode/storage/bootstrap/result" "github.com/m3db/m3/src/dbnode/storage/namespace" @@ -115,18 +113,16 @@ func (s *uninitializedSource) availability( switch shardState { case shard.Initializing: numInitializing++ - case shard.Unknown: - // TODO: Right now we ignore unknown shards (which biases us towards - // failing the bootstrap). Once this interface supports returning errors, - // we should return an error in this case because the cluster is in a bad - // state and the operator should make a decision on how they want to proceed. case shard.Leaving: numLeaving++ case shard.Available: + case shard.Unknown: + fallthrough default: // TODO(rartoul): Make this a hard error once we refactor the interface to support // returning errors. - panic(fmt.Sprintf("unknown shard state: %v", shardState)) + s.opts.InstrumentOptions().Logger().Errorf("unknown shard state: %v", shardState) + return result.ShardTimeRanges{} } } From 8c8c5e773244eba928518b1e7e28f3ac42b4ddcb Mon Sep 17 00:00:00 2001 From: Richard Artoul Date: Wed, 12 Sep 2018 16:42:47 -0400 Subject: [PATCH 16/28] Address more feedback --- src/dbnode/storage/bootstrap/bootstrapper/peers/source.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/dbnode/storage/bootstrap/bootstrapper/peers/source.go b/src/dbnode/storage/bootstrap/bootstrapper/peers/source.go index 9080f2be6d..ef12104442 100644 --- a/src/dbnode/storage/bootstrap/bootstrapper/peers/source.go +++ b/src/dbnode/storage/bootstrap/bootstrapper/peers/source.go @@ -710,17 +710,17 @@ func (s *peersSource) peerAvailability( shardState := hostShardState.ShardState switch shardState { - // Skip cases - We cannot bootstrap from this host + // Don't want to peer bootstrap from a node that has not yet completely + // taken ownership of the shard. case shard.Initializing: - // Don't want to peer bootstrap from a node that has not yet completely - // taken ownership of the shard. - case shard.Unknown: // Success cases - We can bootstrap from this host, which is enough to // mark this shard as bootstrappable. case shard.Leaving: fallthrough case shard.Available: shardPeers.numAvailablePeers++ + case shard.Unknown: + fallthrough default: // TODO(rartoul): Make this a hard error once we refactor the interface to support // returning errors. From 257ddbe666dbd8ed8cb1a42dc93b0cfcbca8becd Mon Sep 17 00:00:00 2001 From: Richard Artoul Date: Wed, 12 Sep 2018 16:45:11 -0400 Subject: [PATCH 17/28] Rename var for clarity --- .../storage/bootstrap/bootstrapper/uninitialized/source.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source.go b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source.go index a92600c215..6668a775c4 100644 --- a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source.go +++ b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source.go @@ -126,8 +126,8 @@ func (s *uninitializedSource) availability( } } - shardHasEverBeenCompletelyInitialized := numInitializing-numLeaving > 0 - if shardHasEverBeenCompletelyInitialized { + shardHasNeverBeenCompletelyInitialized := numInitializing-numLeaving > 0 + if shardHasNeverBeenCompletelyInitialized { availableShardTimeRanges[shardIDUint] = shardsTimeRanges[shardIDUint] } } From 7b9f60f0526da31522e48e9bf9046c88ff7e0acc Mon Sep 17 00:00:00 2001 From: Richard Artoul Date: Thu, 13 Sep 2018 11:49:09 -0400 Subject: [PATCH 18/28] Refactor uninitialized ReadData and ReadIndex to only satisfy reads based on availability --- .../bootstrapper/uninitialized/source.go | 24 +++++++++++++++++-- .../bootstrap/result/result_data_test.go | 4 ++-- .../bootstrap/result/result_index_test.go | 16 +++++++++++++ .../storage/bootstrap/result/shard_ranges.go | 12 ++++++++-- 4 files changed, 50 insertions(+), 6 deletions(-) diff --git a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source.go b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source.go index 6668a775c4..905a9f961e 100644 --- a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source.go +++ b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source.go @@ -140,7 +140,17 @@ func (s *uninitializedSource) ReadData( shardsTimeRanges result.ShardTimeRanges, runOpts bootstrap.RunOptions, ) (result.DataBootstrapResult, error) { - return result.NewDataBootstrapResult(), nil + var ( + availability = s.availability(ns, shardsTimeRanges, runOpts) + missing = shardsTimeRanges.Copy() + ) + missing.Subtract(availability) + + if missing.IsEmpty() { + return result.NewDataBootstrapResult(), nil + } else { + return missing.ToUnfulfilledDataResult(), nil + } } func (s *uninitializedSource) ReadIndex( @@ -148,5 +158,15 @@ func (s *uninitializedSource) ReadIndex( shardsTimeRanges result.ShardTimeRanges, runOpts bootstrap.RunOptions, ) (result.IndexBootstrapResult, error) { - return result.NewIndexBootstrapResult(), nil + var ( + availability = s.availability(ns, shardsTimeRanges, runOpts) + missing = shardsTimeRanges.Copy() + ) + missing.Subtract(availability) + + if missing.IsEmpty() { + return result.NewIndexBootstrapResult(), nil + } else { + return missing.ToUnfulfilledIndexResult(), nil + } } diff --git a/src/dbnode/storage/bootstrap/result/result_data_test.go b/src/dbnode/storage/bootstrap/result/result_data_test.go index 642af841a7..c65cb3819b 100644 --- a/src/dbnode/storage/bootstrap/result/result_data_test.go +++ b/src/dbnode/storage/bootstrap/result/result_data_test.go @@ -363,7 +363,7 @@ func TestShardTimeRangesCopy(t *testing.T) { assert.True(t, str.Equal(copied)) } -func TestShardTimeRangesToUnfulfilledResult(t *testing.T) { +func TestShardTimeRangesToUnfulfilledDataResult(t *testing.T) { str := ShardTimeRanges{ 0: xtime.NewRanges(xtime.Range{ Start: time.Now(), @@ -374,7 +374,7 @@ func TestShardTimeRangesToUnfulfilledResult(t *testing.T) { End: time.Now().Add(4 * time.Minute), }), } - r := str.ToUnfulfilledResult() + r := str.ToUnfulfilledDataResult() assert.Equal(t, 0, len(r.ShardResults())) assert.True(t, r.Unfulfilled().Equal(str)) } diff --git a/src/dbnode/storage/bootstrap/result/result_index_test.go b/src/dbnode/storage/bootstrap/result/result_index_test.go index 318bf3189a..eb05686729 100644 --- a/src/dbnode/storage/bootstrap/result/result_index_test.go +++ b/src/dbnode/storage/bootstrap/result/result_index_test.go @@ -135,6 +135,22 @@ func TestIndexResultAdd(t *testing.T) { require.Equal(t, testRanges, results.Unfulfilled()) } +func TestShardTimeRangesToUnfulfilledIndexResult(t *testing.T) { + str := ShardTimeRanges{ + 0: xtime.NewRanges(xtime.Range{ + Start: time.Now(), + End: time.Now().Add(time.Minute), + }), + 1: xtime.NewRanges(xtime.Range{ + Start: time.Now().Add(3 * time.Minute), + End: time.Now().Add(4 * time.Minute), + }), + } + r := str.ToUnfulfilledIndexResult() + assert.Equal(t, 0, len(r.IndexResults())) + assert.True(t, r.Unfulfilled().Equal(str)) +} + func TestIndexResulsMarkFulfilled(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() diff --git a/src/dbnode/storage/bootstrap/result/shard_ranges.go b/src/dbnode/storage/bootstrap/result/shard_ranges.go index 1e47e6741f..0ee87b8565 100644 --- a/src/dbnode/storage/bootstrap/result/shard_ranges.go +++ b/src/dbnode/storage/bootstrap/result/shard_ranges.go @@ -99,9 +99,9 @@ func (r ShardTimeRanges) AddRanges(other ShardTimeRanges) { } } -// ToUnfulfilledResult will return a result that is comprised of wholly +// ToUnfulfilledDataResult will return a result that is comprised of wholly // unfufilled time ranges from the set of shard time ranges. -func (r ShardTimeRanges) ToUnfulfilledResult() DataBootstrapResult { +func (r ShardTimeRanges) ToUnfulfilledDataResult() DataBootstrapResult { result := NewDataBootstrapResult() for shard, ranges := range r { result.Add(shard, nil, ranges) @@ -109,6 +109,14 @@ func (r ShardTimeRanges) ToUnfulfilledResult() DataBootstrapResult { return result } +// ToUnfulfilledIndexResult will return a result that is comprised of wholly +// unfufilled time ranges from the set of shard time ranges. +func (r ShardTimeRanges) ToUnfulfilledIndexResult() IndexBootstrapResult { + result := NewIndexBootstrapResult() + result.SetUnfulfilled(r) + return result +} + // Subtract will subtract another range from the current range. func (r ShardTimeRanges) Subtract(other ShardTimeRanges) { for shard, ranges := range r { From b1a3dea90fc0ec7bfd9b83a7d9d4056e25af00c6 Mon Sep 17 00:00:00 2001 From: Richard Artoul Date: Thu, 13 Sep 2018 11:54:29 -0400 Subject: [PATCH 19/28] Add tests for ReadData and ReadIndex in uninitialized source --- .../bootstrapper/uninitialized/source_test.go | 29 ++++++++++++++----- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source_test.go b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source_test.go index fb2b9d1e4f..63cf13a9f5 100644 --- a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source_test.go +++ b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source_test.go @@ -266,15 +266,30 @@ func TestUnitializedSourceAvailableDataAndAvailableIndex(t *testing.T) { t.Run(tc.title, func(t *testing.T) { var ( - srcOpts = NewOptions().SetInstrumentOptions(instrument.NewOptions()) - src = newUninitializedSource(srcOpts) - runOpts = testDefaultRunOpts.SetInitialTopologyState(tc.hosts.TopologyState(tc.majorityReplicas)) - dataRes = src.AvailableData(nsMetadata, tc.shardsTimeRangesToBootstrap, runOpts) - indexRes = src.AvailableIndex(nsMetadata, tc.shardsTimeRangesToBootstrap, runOpts) + srcOpts = NewOptions().SetInstrumentOptions(instrument.NewOptions()) + src = newUninitializedSource(srcOpts) + runOpts = testDefaultRunOpts.SetInitialTopologyState(tc.hosts.TopologyState(tc.majorityReplicas)) + dataAvailabilityResult = src.AvailableData(nsMetadata, tc.shardsTimeRangesToBootstrap, runOpts) + indexAvailabilityResult = src.AvailableIndex(nsMetadata, tc.shardsTimeRangesToBootstrap, runOpts) ) - require.Equal(t, tc.expectedAvailableShardsTimeRanges, dataRes) - require.Equal(t, tc.expectedAvailableShardsTimeRanges, indexRes) + // Make sure AvailableData and AvailableIndex return the correct result + require.Equal(t, tc.expectedAvailableShardsTimeRanges, dataAvailabilityResult) + require.Equal(t, tc.expectedAvailableShardsTimeRanges, indexAvailabilityResult) + + // Make sure ReadData marks anything that AvailableData wouldn't return as unfulfilled + dataResult, err := src.ReadData(nsMetadata, tc.shardsTimeRangesToBootstrap, runOpts) + require.NoError(t, err) + expectedDataUnfulfilled := tc.shardsTimeRangesToBootstrap.Copy() + expectedDataUnfulfilled.Subtract(tc.expectedAvailableShardsTimeRanges) + require.Equal(t, expectedDataUnfulfilled, dataResult.Unfulfilled()) + + // Make sure ReadIndex marks anything that AvailableIndex wouldn't return as unfulfilled + indexResult, err := src.ReadIndex(nsMetadata, tc.shardsTimeRangesToBootstrap, runOpts) + require.NoError(t, err) + expectedIndexUnfulfilled := tc.shardsTimeRangesToBootstrap.Copy() + expectedIndexUnfulfilled.Subtract(tc.expectedAvailableShardsTimeRanges) + require.Equal(t, expectedIndexUnfulfilled, indexResult.Unfulfilled()) }) } } From a33867e7f194977a2c40022ef0c140272016a94b Mon Sep 17 00:00:00 2001 From: Richard Artoul Date: Thu, 13 Sep 2018 13:39:14 -0400 Subject: [PATCH 20/28] Fix lint issue --- .../bootstrap/bootstrapper/uninitialized/source.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source.go b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source.go index 905a9f961e..80624cedbc 100644 --- a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source.go +++ b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source.go @@ -148,9 +148,9 @@ func (s *uninitializedSource) ReadData( if missing.IsEmpty() { return result.NewDataBootstrapResult(), nil - } else { - return missing.ToUnfulfilledDataResult(), nil } + + return missing.ToUnfulfilledDataResult(), nil } func (s *uninitializedSource) ReadIndex( @@ -166,7 +166,7 @@ func (s *uninitializedSource) ReadIndex( if missing.IsEmpty() { return result.NewIndexBootstrapResult(), nil - } else { - return missing.ToUnfulfilledIndexResult(), nil } + + return missing.ToUnfulfilledIndexResult(), nil } From 22b4495fc00104864cfb629f4f8a122f1d2dcf46 Mon Sep 17 00:00:00 2001 From: Richard Artoul Date: Mon, 17 Sep 2018 10:24:07 -0400 Subject: [PATCH 21/28] rename uninitialzied bootstrapper to uninitialized_topology bootstrapper --- kube/bundle.yaml | 2 +- kube/m3dbnode-configmap.yaml | 2 +- src/cmd/services/m3dbnode/config/bootstrap.go | 6 ++-- .../services/m3dbnode/main/main_index_test.go | 2 +- src/cmd/services/m3dbnode/main/main_test.go | 2 +- src/dbnode/config/m3dbnode-local-etcd.yml | 2 +- src/dbnode/config/m3dbnode-local.yml | 2 +- .../bootstrapper/uninitialized/provider.go | 28 +++++++++---------- .../bootstrapper/uninitialized/source.go | 20 ++++++------- .../bootstrapper/uninitialized/source_test.go | 4 +-- .../bootstrapper/uninitialized/types.go | 10 +++---- .../m3dbnode-server1-config.yaml | 2 +- .../m3dbnode-server2-config.yaml | 2 +- .../m3dbnode-server3-config.yaml | 2 +- 14 files changed, 43 insertions(+), 43 deletions(-) diff --git a/kube/bundle.yaml b/kube/bundle.yaml index 5b9803cc60..8cd5a6e7fa 100644 --- a/kube/bundle.yaml +++ b/kube/bundle.yaml @@ -177,7 +177,7 @@ data: - filesystem - commitlog - peers - - uninitialized + - uninitialized_topology fs: numProcessorsPerCPU: 0.125 diff --git a/kube/m3dbnode-configmap.yaml b/kube/m3dbnode-configmap.yaml index 37d1fc1312..927c48c9b7 100644 --- a/kube/m3dbnode-configmap.yaml +++ b/kube/m3dbnode-configmap.yaml @@ -69,7 +69,7 @@ data: - filesystem - commitlog - peers - - uninitialized + - uninitialized_topology fs: numProcessorsPerCPU: 0.125 diff --git a/src/cmd/services/m3dbnode/config/bootstrap.go b/src/cmd/services/m3dbnode/config/bootstrap.go index e1cbfa42d1..a596c77f62 100644 --- a/src/cmd/services/m3dbnode/config/bootstrap.go +++ b/src/cmd/services/m3dbnode/config/bootstrap.go @@ -159,11 +159,11 @@ func (bsc BootstrapConfiguration) New( if err != nil { return nil, err } - case uninitialized.UninitializedBootstrapperName: + case uninitialized.UninitializedTopologyBootstrapperName: uopts := uninitialized.NewOptions(). SetResultOptions(rsOpts). SetInstrumentOptions(opts.InstrumentOptions()) - bs, err = uninitialized.NewUninitializedBootstrapperProvider(uopts, bs) + bs, err = uninitialized.NewuninitializedTopologyBootstrapperProvider(uopts, bs) if err != nil { return nil, err } @@ -206,7 +206,7 @@ func ValidateBootstrappersOrder(names []string) error { bfs.FileSystemBootstrapperName, peers.PeersBootstrapperName, }, - uninitialized.UninitializedBootstrapperName: []string{ + uninitialized.UninitializedTopologyBootstrapperName: []string{ // Unintialized bootstrapper may appear after filesystem or peers or commitlog bfs.FileSystemBootstrapperName, commitlog.CommitLogBootstrapperName, diff --git a/src/cmd/services/m3dbnode/main/main_index_test.go b/src/cmd/services/m3dbnode/main/main_index_test.go index 2a8855866d..cbf0181d2a 100644 --- a/src/cmd/services/m3dbnode/main/main_index_test.go +++ b/src/cmd/services/m3dbnode/main/main_index_test.go @@ -328,7 +328,7 @@ db: - filesystem - commitlog - peers - - uninitialized + - uninitialized_topology fs: numProcessorsPerCPU: 0.125 diff --git a/src/cmd/services/m3dbnode/main/main_test.go b/src/cmd/services/m3dbnode/main/main_test.go index ff8a57bdd9..146f5e50c5 100644 --- a/src/cmd/services/m3dbnode/main/main_test.go +++ b/src/cmd/services/m3dbnode/main/main_test.go @@ -488,7 +488,7 @@ db: - filesystem - commitlog - peers - - uninitialized + - uninitialized_topology fs: numProcessorsPerCPU: 0.125 diff --git a/src/dbnode/config/m3dbnode-local-etcd.yml b/src/dbnode/config/m3dbnode-local-etcd.yml index 5a57b11c3c..37c1c95ff6 100644 --- a/src/dbnode/config/m3dbnode-local-etcd.yml +++ b/src/dbnode/config/m3dbnode-local-etcd.yml @@ -64,7 +64,7 @@ db: - filesystem - commitlog - peers - - uninitialized + - uninitialized_topology fs: numProcessorsPerCPU: 0.125 diff --git a/src/dbnode/config/m3dbnode-local.yml b/src/dbnode/config/m3dbnode-local.yml index ce10d238a3..929c656c81 100644 --- a/src/dbnode/config/m3dbnode-local.yml +++ b/src/dbnode/config/m3dbnode-local.yml @@ -64,7 +64,7 @@ db: - filesystem - commitlog - peers - - uninitialized + - uninitialized_topology fs: numProcessorsPerCPU: 0.125 diff --git a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/provider.go b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/provider.go index 803ee32f42..6dc7046810 100644 --- a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/provider.go +++ b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/provider.go @@ -26,18 +26,18 @@ import ( ) const ( - // UninitializedBootstrapperName is the name of the uninitialized bootstrapper. - UninitializedBootstrapperName = "uninitialized" + // UninitializedTopologyBootstrapperName is the name of the uninitialized bootstrapper. + UninitializedTopologyBootstrapperName = "uninitialized_topology" ) -type uninitializedBootstrapperProvider struct { +type uninitializedTopologyBootstrapperProvider struct { opts Options next bootstrap.BootstrapperProvider } -// NewUninitializedBootstrapperProvider creates a new uninitialized bootstrapper +// NewuninitializedTopologyBootstrapperProvider creates a new uninitialized bootstrapper // provider. -func NewUninitializedBootstrapperProvider( +func NewuninitializedTopologyBootstrapperProvider( opts Options, next bootstrap.BootstrapperProvider, ) (bootstrap.BootstrapperProvider, error) { @@ -45,16 +45,16 @@ func NewUninitializedBootstrapperProvider( return nil, err } - return uninitializedBootstrapperProvider{ + return uninitializedTopologyBootstrapperProvider{ opts: opts, next: next, }, nil } -func (p uninitializedBootstrapperProvider) Provide() (bootstrap.Bootstrapper, error) { +func (p uninitializedTopologyBootstrapperProvider) Provide() (bootstrap.Bootstrapper, error) { var ( - src = newUninitializedSource(p.opts) - b = &uninitializedBootstrapper{} + src = newTopologyUninitializedSource(p.opts) + b = &uninitializedTopologyBootstrapper{} next bootstrap.Bootstrapper err error ) @@ -70,14 +70,14 @@ func (p uninitializedBootstrapperProvider) Provide() (bootstrap.Bootstrapper, er b.String(), src, p.opts.ResultOptions(), next) } -func (p uninitializedBootstrapperProvider) String() string { - return UninitializedBootstrapperName +func (p uninitializedTopologyBootstrapperProvider) String() string { + return UninitializedTopologyBootstrapperName } -type uninitializedBootstrapper struct { +type uninitializedTopologyBootstrapper struct { bootstrap.Bootstrapper } -func (*uninitializedBootstrapper) String() string { - return UninitializedBootstrapperName +func (*uninitializedTopologyBootstrapper) String() string { + return UninitializedTopologyBootstrapperName } diff --git a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source.go b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source.go index 80624cedbc..3e547488fe 100644 --- a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source.go +++ b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source.go @@ -37,18 +37,18 @@ import ( // putting the noop-all or noop-none bootstrappers at the end of the process. // Behavior is best understood by reading the test cases for the test: // TestUnitializedSourceAvailableDataAndAvailableIndex -type uninitializedSource struct { +type uninitializedTopologySource struct { opts Options } -// NewUninitializedSource creates a new uninitialized source. -func newUninitializedSource(opts Options) bootstrap.Source { - return &uninitializedSource{ +// newTopologyUninitializedSource creates a new uninitialized source. +func newTopologyUninitializedSource(opts Options) bootstrap.Source { + return &uninitializedTopologySource{ opts: opts, } } -func (s *uninitializedSource) Can(strategy bootstrap.Strategy) bool { +func (s *uninitializedTopologySource) Can(strategy bootstrap.Strategy) bool { switch strategy { case bootstrap.BootstrapSequential: return true @@ -57,7 +57,7 @@ func (s *uninitializedSource) Can(strategy bootstrap.Strategy) bool { return false } -func (s *uninitializedSource) AvailableData( +func (s *uninitializedTopologySource) AvailableData( ns namespace.Metadata, shardsTimeRanges result.ShardTimeRanges, runOpts bootstrap.RunOptions, @@ -65,7 +65,7 @@ func (s *uninitializedSource) AvailableData( return s.availability(ns, shardsTimeRanges, runOpts) } -func (s *uninitializedSource) AvailableIndex( +func (s *uninitializedTopologySource) AvailableIndex( ns namespace.Metadata, shardsTimeRanges result.ShardTimeRanges, runOpts bootstrap.RunOptions, @@ -73,7 +73,7 @@ func (s *uninitializedSource) AvailableIndex( return s.availability(ns, shardsTimeRanges, runOpts) } -func (s *uninitializedSource) availability( +func (s *uninitializedTopologySource) availability( ns namespace.Metadata, shardsTimeRanges result.ShardTimeRanges, runOpts bootstrap.RunOptions, @@ -135,7 +135,7 @@ func (s *uninitializedSource) availability( return availableShardTimeRanges } -func (s *uninitializedSource) ReadData( +func (s *uninitializedTopologySource) ReadData( ns namespace.Metadata, shardsTimeRanges result.ShardTimeRanges, runOpts bootstrap.RunOptions, @@ -153,7 +153,7 @@ func (s *uninitializedSource) ReadData( return missing.ToUnfulfilledDataResult(), nil } -func (s *uninitializedSource) ReadIndex( +func (s *uninitializedTopologySource) ReadIndex( ns namespace.Metadata, shardsTimeRanges result.ShardTimeRanges, runOpts bootstrap.RunOptions, diff --git a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source_test.go b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source_test.go index 63cf13a9f5..c0bcaf6c4c 100644 --- a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source_test.go +++ b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source_test.go @@ -44,7 +44,7 @@ var ( notSelfID3 = "not-self-3" ) -func TestUnitializedSourceAvailableDataAndAvailableIndex(t *testing.T) { +func TestUnitializedTopologySourceAvailableDataAndAvailableIndex(t *testing.T) { var ( blockSize = 2 * time.Hour shards = []uint32{0, 1, 2, 3} @@ -267,7 +267,7 @@ func TestUnitializedSourceAvailableDataAndAvailableIndex(t *testing.T) { var ( srcOpts = NewOptions().SetInstrumentOptions(instrument.NewOptions()) - src = newUninitializedSource(srcOpts) + src = newTopologyUninitializedSource(srcOpts) runOpts = testDefaultRunOpts.SetInitialTopologyState(tc.hosts.TopologyState(tc.majorityReplicas)) dataAvailabilityResult = src.AvailableData(nsMetadata, tc.shardsTimeRangesToBootstrap, runOpts) indexAvailabilityResult = src.AvailableIndex(nsMetadata, tc.shardsTimeRangesToBootstrap, runOpts) diff --git a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/types.go b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/types.go index 719ba87c1c..7bd20717c4 100644 --- a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/types.go +++ b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/types.go @@ -27,18 +27,18 @@ import ( // Options is the options interface for the uninitialized source. type Options interface { + // Validate validates the options are correct. + Validate() error + // SetResultOptions sets the result options SetResultOptions(value result.Options) Options // ResultOptions returns the result options ResultOptions() result.Options - // Return the instrument options. - InstrumentOptions() instrument.Options - // Set the instrument options. SetInstrumentOptions(value instrument.Options) Options - // Validate validates the options are correct. - Validate() error + // Return the instrument options. + InstrumentOptions() instrument.Options } diff --git a/src/query/benchmark/configs/multi_node_setup/m3dbnode-server1-config.yaml b/src/query/benchmark/configs/multi_node_setup/m3dbnode-server1-config.yaml index d63b571944..8056f15006 100644 --- a/src/query/benchmark/configs/multi_node_setup/m3dbnode-server1-config.yaml +++ b/src/query/benchmark/configs/multi_node_setup/m3dbnode-server1-config.yaml @@ -49,7 +49,7 @@ bootstrap: - filesystem - commitlog - peers - - uninitialized + - uninitialized_topology fs: numProcessorsPerCPU: 0.125 diff --git a/src/query/benchmark/configs/multi_node_setup/m3dbnode-server2-config.yaml b/src/query/benchmark/configs/multi_node_setup/m3dbnode-server2-config.yaml index b1e8b37c29..e2710995e5 100644 --- a/src/query/benchmark/configs/multi_node_setup/m3dbnode-server2-config.yaml +++ b/src/query/benchmark/configs/multi_node_setup/m3dbnode-server2-config.yaml @@ -49,7 +49,7 @@ bootstrap: - filesystem - commitlog - peers - - uninitialized + - uninitialized_topology fs: numProcessorsPerCPU: 0.125 diff --git a/src/query/benchmark/configs/multi_node_setup/m3dbnode-server3-config.yaml b/src/query/benchmark/configs/multi_node_setup/m3dbnode-server3-config.yaml index e35c81e87d..5aafa571c4 100644 --- a/src/query/benchmark/configs/multi_node_setup/m3dbnode-server3-config.yaml +++ b/src/query/benchmark/configs/multi_node_setup/m3dbnode-server3-config.yaml @@ -49,7 +49,7 @@ bootstrap: - filesystem - commitlog - peers - - uninitialized + - uninitialized_topology fs: numProcessorsPerCPU: 0.125 From b48b4e41e69b16f55a68a1e4f659dc2a95381644 Mon Sep 17 00:00:00 2001 From: Richard Artoul Date: Mon, 17 Sep 2018 10:30:43 -0400 Subject: [PATCH 22/28] Address review feedback --- .../bootstrapper/commitlog/source.go | 4 + .../bootstrapper/commitlog/source_test.go | 44 +++++----- .../bootstrapper/peers/source_test.go | 50 ++++++------ .../bootstrapper/uninitialized/source.go | 4 +- .../bootstrapper/uninitialized/source_test.go | 80 +++++++++---------- 5 files changed, 93 insertions(+), 89 deletions(-) diff --git a/src/dbnode/storage/bootstrap/bootstrapper/commitlog/source.go b/src/dbnode/storage/bootstrap/bootstrapper/commitlog/source.go index ad6d23d251..9749ab0c65 100644 --- a/src/dbnode/storage/bootstrap/bootstrapper/commitlog/source.go +++ b/src/dbnode/storage/bootstrap/bootstrapper/commitlog/source.go @@ -1421,6 +1421,10 @@ func (s commitLogSource) maybeAddToIndex( return err } +// The commitlog bootstrapper determines availability primarily by checking if the +// origin host has ever reached the "Initialized" state for the shard that is being +// bootstrapped. If not, then it can't provide data for that shard because it doesn't +// have all of it by definition. func (s *commitLogSource) availability( ns namespace.Metadata, shardsTimeRanges result.ShardTimeRanges, diff --git a/src/dbnode/storage/bootstrap/bootstrapper/commitlog/source_test.go b/src/dbnode/storage/bootstrap/bootstrapper/commitlog/source_test.go index 8dd77c70df..afb2a23883 100644 --- a/src/dbnode/storage/bootstrap/bootstrapper/commitlog/source_test.go +++ b/src/dbnode/storage/bootstrap/bootstrapper/commitlog/source_test.go @@ -26,7 +26,7 @@ import ( "github.com/m3db/m3/src/dbnode/persist/fs" "github.com/m3db/m3/src/dbnode/storage/bootstrap/result" - topotestutils "github.com/m3db/m3/src/dbnode/topology/testutil" + tu "github.com/m3db/m3/src/dbnode/topology/testutil" "github.com/m3db/m3cluster/shard" xtime "github.com/m3db/m3x/time" @@ -57,16 +57,16 @@ func TestAvailableData(t *testing.T) { testCases := []struct { title string - hosts topotestutils.SourceAvailableHosts + hosts tu.SourceAvailableHosts majorityReplicas int shardsTimeRangesToBootstrap result.ShardTimeRanges expectedAvailableShardsTimeRanges result.ShardTimeRanges }{ { title: "Single node - Shard initializing", - hosts: []topotestutils.SourceAvailableHost{ - topotestutils.SourceAvailableHost{ - Name: topotestutils.SelfID, + hosts: []tu.SourceAvailableHost{ + tu.SourceAvailableHost{ + Name: tu.SelfID, Shards: shards, ShardStates: shard.Initializing, }, @@ -77,9 +77,9 @@ func TestAvailableData(t *testing.T) { }, { title: "Single node - Shard unknown", - hosts: []topotestutils.SourceAvailableHost{ - topotestutils.SourceAvailableHost{ - Name: topotestutils.SelfID, + hosts: []tu.SourceAvailableHost{ + tu.SourceAvailableHost{ + Name: tu.SelfID, Shards: shards, ShardStates: shard.Unknown, }, @@ -90,9 +90,9 @@ func TestAvailableData(t *testing.T) { }, { title: "Single node - Shard leaving", - hosts: []topotestutils.SourceAvailableHost{ - topotestutils.SourceAvailableHost{ - Name: topotestutils.SelfID, + hosts: []tu.SourceAvailableHost{ + tu.SourceAvailableHost{ + Name: tu.SelfID, Shards: shards, ShardStates: shard.Leaving, }, @@ -103,9 +103,9 @@ func TestAvailableData(t *testing.T) { }, { title: "Single node - Shard available", - hosts: []topotestutils.SourceAvailableHost{ - topotestutils.SourceAvailableHost{ - Name: topotestutils.SelfID, + hosts: []tu.SourceAvailableHost{ + tu.SourceAvailableHost{ + Name: tu.SelfID, Shards: shards, ShardStates: shard.Available, }, @@ -116,13 +116,13 @@ func TestAvailableData(t *testing.T) { }, { title: "Multi node - Origin available", - hosts: []topotestutils.SourceAvailableHost{ - topotestutils.SourceAvailableHost{ - Name: topotestutils.SelfID, + hosts: []tu.SourceAvailableHost{ + tu.SourceAvailableHost{ + Name: tu.SelfID, Shards: shards, ShardStates: shard.Available, }, - topotestutils.SourceAvailableHost{ + tu.SourceAvailableHost{ Name: notSelfID, Shards: shards, ShardStates: shard.Initializing, @@ -134,13 +134,13 @@ func TestAvailableData(t *testing.T) { }, { title: "Multi node - Origin not available", - hosts: []topotestutils.SourceAvailableHost{ - topotestutils.SourceAvailableHost{ - Name: topotestutils.SelfID, + hosts: []tu.SourceAvailableHost{ + tu.SourceAvailableHost{ + Name: tu.SelfID, Shards: shards, ShardStates: shard.Initializing, }, - topotestutils.SourceAvailableHost{ + tu.SourceAvailableHost{ Name: notSelfID, Shards: shards, ShardStates: shard.Available, diff --git a/src/dbnode/storage/bootstrap/bootstrapper/peers/source_test.go b/src/dbnode/storage/bootstrap/bootstrapper/peers/source_test.go index 84c084498f..8247c5aaf1 100644 --- a/src/dbnode/storage/bootstrap/bootstrapper/peers/source_test.go +++ b/src/dbnode/storage/bootstrap/bootstrapper/peers/source_test.go @@ -27,7 +27,7 @@ import ( m3dbruntime "github.com/m3db/m3/src/dbnode/runtime" "github.com/m3db/m3/src/dbnode/storage/bootstrap/result" "github.com/m3db/m3/src/dbnode/topology" - topotestutils "github.com/m3db/m3/src/dbnode/topology/testutil" + tu "github.com/m3db/m3/src/dbnode/topology/testutil" "github.com/m3db/m3cluster/shard" xtime "github.com/m3db/m3x/time" @@ -65,7 +65,7 @@ func TestPeersSourceAvailableDataAndIndex(t *testing.T) { testCases := []struct { title string - hosts topotestutils.SourceAvailableHosts + hosts tu.SourceAvailableHosts majorityReplicas int bootstrapReadConsistency topology.ReadConsistencyLevel shardsTimeRangesToBootstrap result.ShardTimeRanges @@ -73,9 +73,9 @@ func TestPeersSourceAvailableDataAndIndex(t *testing.T) { }{ { title: "Returns empty if only self is available", - hosts: []topotestutils.SourceAvailableHost{ - topotestutils.SourceAvailableHost{ - Name: topotestutils.SelfID, + hosts: []tu.SourceAvailableHost{ + tu.SourceAvailableHost{ + Name: tu.SelfID, Shards: shards, ShardStates: shard.Available, }, @@ -87,18 +87,18 @@ func TestPeersSourceAvailableDataAndIndex(t *testing.T) { }, { title: "Returns empty if all other peers initializing/unknown", - hosts: []topotestutils.SourceAvailableHost{ - topotestutils.SourceAvailableHost{ - Name: topotestutils.SelfID, + hosts: []tu.SourceAvailableHost{ + tu.SourceAvailableHost{ + Name: tu.SelfID, Shards: shards, ShardStates: shard.Available, }, - topotestutils.SourceAvailableHost{ + tu.SourceAvailableHost{ Name: notSelfID1, Shards: shards, ShardStates: shard.Initializing, }, - topotestutils.SourceAvailableHost{ + tu.SourceAvailableHost{ Name: notSelfID2, Shards: shards, ShardStates: shard.Unknown, @@ -111,18 +111,18 @@ func TestPeersSourceAvailableDataAndIndex(t *testing.T) { }, { title: "Returns success if consistency can be met (available/leaving)", - hosts: []topotestutils.SourceAvailableHost{ - topotestutils.SourceAvailableHost{ - Name: topotestutils.SelfID, + hosts: []tu.SourceAvailableHost{ + tu.SourceAvailableHost{ + Name: tu.SelfID, Shards: shards, ShardStates: shard.Initializing, }, - topotestutils.SourceAvailableHost{ + tu.SourceAvailableHost{ Name: notSelfID1, Shards: shards, ShardStates: shard.Available, }, - topotestutils.SourceAvailableHost{ + tu.SourceAvailableHost{ Name: notSelfID2, Shards: shards, ShardStates: shard.Leaving, @@ -134,18 +134,18 @@ func TestPeersSourceAvailableDataAndIndex(t *testing.T) { }, { title: "Skips shards that were not in the topology at start", - hosts: []topotestutils.SourceAvailableHost{ - topotestutils.SourceAvailableHost{ - Name: topotestutils.SelfID, + hosts: []tu.SourceAvailableHost{ + tu.SourceAvailableHost{ + Name: tu.SelfID, Shards: shards, ShardStates: shard.Initializing, }, - topotestutils.SourceAvailableHost{ + tu.SourceAvailableHost{ Name: notSelfID1, Shards: shards, ShardStates: shard.Available, }, - topotestutils.SourceAvailableHost{ + tu.SourceAvailableHost{ Name: notSelfID2, Shards: shards, ShardStates: shard.Available, @@ -158,18 +158,18 @@ func TestPeersSourceAvailableDataAndIndex(t *testing.T) { }, { title: "Returns empty if consistency can not be met", - hosts: []topotestutils.SourceAvailableHost{ - topotestutils.SourceAvailableHost{ - Name: topotestutils.SelfID, + hosts: []tu.SourceAvailableHost{ + tu.SourceAvailableHost{ + Name: tu.SelfID, Shards: shards, ShardStates: shard.Initializing, }, - topotestutils.SourceAvailableHost{ + tu.SourceAvailableHost{ Name: notSelfID1, Shards: shards, ShardStates: shard.Available, }, - topotestutils.SourceAvailableHost{ + tu.SourceAvailableHost{ Name: notSelfID2, Shards: shards, ShardStates: shard.Available, diff --git a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source.go b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source.go index 3e547488fe..74bdd3210d 100644 --- a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source.go +++ b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source.go @@ -29,8 +29,8 @@ import ( ) // The purpose of the unitializedSource is to succeed bootstraps for any -// shard/time-ranges if the given shard/namespace combination has never -// been completely initialized (is a new namespace). This is required for +// shard/time-ranges if the cluster they're associated with has never +// been completely initialized (is a new cluster). This is required for // allowing us to configure the bootstrappers such that the commitlog // bootstrapper can precede the peers bootstrapper and still succeed bootstraps // for brand new namespaces without permitting unintentional data loss by diff --git a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source_test.go b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source_test.go index c0bcaf6c4c..5902a9489b 100644 --- a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source_test.go +++ b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source_test.go @@ -27,7 +27,7 @@ import ( "github.com/m3db/m3/src/dbnode/storage/bootstrap" "github.com/m3db/m3/src/dbnode/storage/bootstrap/result" "github.com/m3db/m3/src/dbnode/storage/namespace" - topotestutils "github.com/m3db/m3/src/dbnode/topology/testutil" + tu "github.com/m3db/m3/src/dbnode/topology/testutil" "github.com/m3db/m3cluster/shard" "github.com/m3db/m3x/ident" "github.com/m3db/m3x/instrument" @@ -65,7 +65,7 @@ func TestUnitializedTopologySourceAvailableDataAndAvailableIndex(t *testing.T) { testCases := []struct { title string majorityReplicas int - hosts topotestutils.SourceAvailableHosts + hosts tu.SourceAvailableHosts shardsTimeRangesToBootstrap result.ShardTimeRanges expectedAvailableShardsTimeRanges result.ShardTimeRanges }{ @@ -74,9 +74,9 @@ func TestUnitializedTopologySourceAvailableDataAndAvailableIndex(t *testing.T) { { title: "Single node - Shard initializing", majorityReplicas: 1, - hosts: []topotestutils.SourceAvailableHost{ - topotestutils.SourceAvailableHost{ - Name: topotestutils.SelfID, + hosts: []tu.SourceAvailableHost{ + tu.SourceAvailableHost{ + Name: tu.SelfID, Shards: shards, ShardStates: shard.Initializing, }, @@ -89,9 +89,9 @@ func TestUnitializedTopologySourceAvailableDataAndAvailableIndex(t *testing.T) { { title: "Single node - Shard unknown", majorityReplicas: 1, - hosts: []topotestutils.SourceAvailableHost{ - topotestutils.SourceAvailableHost{ - Name: topotestutils.SelfID, + hosts: []tu.SourceAvailableHost{ + tu.SourceAvailableHost{ + Name: tu.SelfID, Shards: shards, ShardStates: shard.Unknown, }, @@ -104,9 +104,9 @@ func TestUnitializedTopologySourceAvailableDataAndAvailableIndex(t *testing.T) { { title: "Single node - Shard leaving", majorityReplicas: 1, - hosts: []topotestutils.SourceAvailableHost{ - topotestutils.SourceAvailableHost{ - Name: topotestutils.SelfID, + hosts: []tu.SourceAvailableHost{ + tu.SourceAvailableHost{ + Name: tu.SelfID, Shards: shards, ShardStates: shard.Leaving, }, @@ -119,9 +119,9 @@ func TestUnitializedTopologySourceAvailableDataAndAvailableIndex(t *testing.T) { { title: "Single node - Shard available", majorityReplicas: 1, - hosts: []topotestutils.SourceAvailableHost{ - topotestutils.SourceAvailableHost{ - Name: topotestutils.SelfID, + hosts: []tu.SourceAvailableHost{ + tu.SourceAvailableHost{ + Name: tu.SelfID, Shards: shards, ShardStates: shard.Available, }, @@ -134,18 +134,18 @@ func TestUnitializedTopologySourceAvailableDataAndAvailableIndex(t *testing.T) { { title: "Multi node - Brand new namespace (all nodes initializing)", majorityReplicas: 2, - hosts: []topotestutils.SourceAvailableHost{ - topotestutils.SourceAvailableHost{ - Name: topotestutils.SelfID, + hosts: []tu.SourceAvailableHost{ + tu.SourceAvailableHost{ + Name: tu.SelfID, Shards: shards, ShardStates: shard.Initializing, }, - topotestutils.SourceAvailableHost{ + tu.SourceAvailableHost{ Name: notSelfID1, Shards: shards, ShardStates: shard.Initializing, }, - topotestutils.SourceAvailableHost{ + tu.SourceAvailableHost{ Name: notSelfID2, Shards: shards, ShardStates: shard.Initializing, @@ -160,18 +160,18 @@ func TestUnitializedTopologySourceAvailableDataAndAvailableIndex(t *testing.T) { { title: "Multi node - Recently created namespace (one node still initializing)", majorityReplicas: 2, - hosts: []topotestutils.SourceAvailableHost{ - topotestutils.SourceAvailableHost{ - Name: topotestutils.SelfID, + hosts: []tu.SourceAvailableHost{ + tu.SourceAvailableHost{ + Name: tu.SelfID, Shards: shards, ShardStates: shard.Initializing, }, - topotestutils.SourceAvailableHost{ + tu.SourceAvailableHost{ Name: notSelfID1, Shards: shards, ShardStates: shard.Available, }, - topotestutils.SourceAvailableHost{ + tu.SourceAvailableHost{ Name: notSelfID2, Shards: shards, ShardStates: shard.Available, @@ -185,18 +185,18 @@ func TestUnitializedTopologySourceAvailableDataAndAvailableIndex(t *testing.T) { { title: "Multi node - Initialized namespace (no nodes initializing)", majorityReplicas: 2, - hosts: []topotestutils.SourceAvailableHost{ - topotestutils.SourceAvailableHost{ - Name: topotestutils.SelfID, + hosts: []tu.SourceAvailableHost{ + tu.SourceAvailableHost{ + Name: tu.SelfID, Shards: shards, ShardStates: shard.Available, }, - topotestutils.SourceAvailableHost{ + tu.SourceAvailableHost{ Name: notSelfID1, Shards: shards, ShardStates: shard.Available, }, - topotestutils.SourceAvailableHost{ + tu.SourceAvailableHost{ Name: notSelfID2, Shards: shards, ShardStates: shard.Available, @@ -210,23 +210,23 @@ func TestUnitializedTopologySourceAvailableDataAndAvailableIndex(t *testing.T) { { title: "Multi node - Node replace (one node leaving, one initializing)", majorityReplicas: 2, - hosts: []topotestutils.SourceAvailableHost{ - topotestutils.SourceAvailableHost{ - Name: topotestutils.SelfID, + hosts: []tu.SourceAvailableHost{ + tu.SourceAvailableHost{ + Name: tu.SelfID, Shards: shards, ShardStates: shard.Available, }, - topotestutils.SourceAvailableHost{ + tu.SourceAvailableHost{ Name: notSelfID1, Shards: shards, ShardStates: shard.Leaving, }, - topotestutils.SourceAvailableHost{ + tu.SourceAvailableHost{ Name: notSelfID2, Shards: shards, ShardStates: shard.Available, }, - topotestutils.SourceAvailableHost{ + tu.SourceAvailableHost{ Name: notSelfID3, Shards: shards, ShardStates: shard.Initializing, @@ -240,18 +240,18 @@ func TestUnitializedTopologySourceAvailableDataAndAvailableIndex(t *testing.T) { { title: "Multi node - One node unknown", majorityReplicas: 2, - hosts: []topotestutils.SourceAvailableHost{ - topotestutils.SourceAvailableHost{ - Name: topotestutils.SelfID, + hosts: []tu.SourceAvailableHost{ + tu.SourceAvailableHost{ + Name: tu.SelfID, Shards: shards, ShardStates: shard.Available, }, - topotestutils.SourceAvailableHost{ + tu.SourceAvailableHost{ Name: notSelfID1, Shards: shards, ShardStates: shard.Available, }, - topotestutils.SourceAvailableHost{ + tu.SourceAvailableHost{ Name: notSelfID2, Shards: shards, ShardStates: shard.Unknown, From 9c1b059eaa4ee78672af9cb4772699c99e0762c8 Mon Sep 17 00:00:00 2001 From: Richard Artoul Date: Mon, 17 Sep 2018 10:34:25 -0400 Subject: [PATCH 23/28] Address more review feedback --- .../bootstrap/bootstrapper/commitlog/source.go | 2 +- .../bootstrap/bootstrapper/uninitialized/source.go | 12 +++++++++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/dbnode/storage/bootstrap/bootstrapper/commitlog/source.go b/src/dbnode/storage/bootstrap/bootstrapper/commitlog/source.go index 9749ab0c65..3b092a420f 100644 --- a/src/dbnode/storage/bootstrap/bootstrapper/commitlog/source.go +++ b/src/dbnode/storage/bootstrap/bootstrapper/commitlog/source.go @@ -1422,7 +1422,7 @@ func (s commitLogSource) maybeAddToIndex( } // The commitlog bootstrapper determines availability primarily by checking if the -// origin host has ever reached the "Initialized" state for the shard that is being +// origin host has ever reached the "Available" state for the shard that is being // bootstrapped. If not, then it can't provide data for that shard because it doesn't // have all of it by definition. func (s *commitLogSource) availability( diff --git a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source.go b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source.go index 74bdd3210d..0e40be5876 100644 --- a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source.go +++ b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source.go @@ -93,12 +93,12 @@ func (s *uninitializedTopologySource) availability( } // The basic idea for the algorithm is that on a shard-by-shard basis we - // need to determine if the namespace is "new" in the sense that it has + // need to determine if the cluster is "new" in the sense that it has // never been completely initialized (reached a state where all the hosts - // in the topology are "available"). + // in the topology are "available" for that specific shard). // In order to determine this, we simply count the number of hosts in the // "initializing" state. If this number is larger than zero, than the - // namespace is "new". + // cluster is "new". // The one exception to this case is when we perform topology changes and // we end up with one extra node that is initializing which should be offset // by the corresponding node that is leaving. I.E if numInitializing > 0 @@ -126,6 +126,12 @@ func (s *uninitializedTopologySource) availability( } } + // This heuristic works for all scenarios except for if we tried to change the replication + // factor of a cluster that was already initialized. In that case, we might have to come + // up with a new heuristic, or simply require that the peers bootstrapper be configured as + // a bootstrapper if users want to change the replication factor dynamically, which is fine + // because otherwise you'd have to wait for one entire retention period for the replicaiton + // factor to actually increase correctly. shardHasNeverBeenCompletelyInitialized := numInitializing-numLeaving > 0 if shardHasNeverBeenCompletelyInitialized { availableShardTimeRanges[shardIDUint] = shardsTimeRanges[shardIDUint] From 437df7afa01f97d4d5e99b4818dd58bc2195520d Mon Sep 17 00:00:00 2001 From: Richard Artoul Date: Mon, 17 Sep 2018 11:43:38 -0400 Subject: [PATCH 24/28] Fix config to use new bootstrapper name --- src/dbnode/example/m3db-node-config.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dbnode/example/m3db-node-config.yaml b/src/dbnode/example/m3db-node-config.yaml index 6c5304f132..10276cda72 100644 --- a/src/dbnode/example/m3db-node-config.yaml +++ b/src/dbnode/example/m3db-node-config.yaml @@ -48,7 +48,7 @@ bootstrap: - filesystem - commitlog - peers - - uninitialized + - uninitialized_topology fs: numProcessorsPerCPU: 0.125 From d3bb0182b72f9093dc101a60fc76d4e0af34021b Mon Sep 17 00:00:00 2001 From: Richard Artoul Date: Mon, 17 Sep 2018 11:45:14 -0400 Subject: [PATCH 25/28] Fix config to use new bootstrapper name --- src/query/benchmark/benchmarker/main/m3dbnode-local-config.yaml | 2 +- src/query/benchmark/configs/m3db_config.yaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/query/benchmark/benchmarker/main/m3dbnode-local-config.yaml b/src/query/benchmark/benchmarker/main/m3dbnode-local-config.yaml index 7fe475fa37..c161b7ada5 100644 --- a/src/query/benchmark/benchmarker/main/m3dbnode-local-config.yaml +++ b/src/query/benchmark/benchmarker/main/m3dbnode-local-config.yaml @@ -51,7 +51,7 @@ bootstrap: - filesystem - commitlog - peers - - uninitialized + - uninitialized_topology fs: numProcessorsPerCPU: 0.125 diff --git a/src/query/benchmark/configs/m3db_config.yaml b/src/query/benchmark/configs/m3db_config.yaml index 50fc1eadc8..45c28e86c8 100644 --- a/src/query/benchmark/configs/m3db_config.yaml +++ b/src/query/benchmark/configs/m3db_config.yaml @@ -50,7 +50,7 @@ bootstrap: - filesystem - commitlog - peers - - uninitialized + - uninitialized_topology fs: numProcessorsPerCPU: 0.125 From 777776368c9a0da999a2238cbecf9021532a84fd Mon Sep 17 00:00:00 2001 From: Richard Artoul Date: Mon, 17 Sep 2018 15:12:15 -0400 Subject: [PATCH 26/28] Rewrite SourceAvailableHosts to use map structure --- src/cmd/services/m3dbnode/config/bootstrap.go | 5 +- .../commitlog/source_prop_test.go | 12 +- .../bootstrapper/commitlog/source_test.go | 95 +++------ .../bootstrapper/peers/source_test.go | 116 +++-------- .../bootstrapper/uninitialized/options.go | 12 -- .../bootstrapper/uninitialized/provider.go | 8 +- .../bootstrapper/uninitialized/source_test.go | 195 +++++------------- .../bootstrapper/uninitialized/types.go | 3 - src/dbnode/topology/testutil/topology.go | 33 ++- 9 files changed, 130 insertions(+), 349 deletions(-) diff --git a/src/cmd/services/m3dbnode/config/bootstrap.go b/src/cmd/services/m3dbnode/config/bootstrap.go index a596c77f62..8476f96582 100644 --- a/src/cmd/services/m3dbnode/config/bootstrap.go +++ b/src/cmd/services/m3dbnode/config/bootstrap.go @@ -163,10 +163,7 @@ func (bsc BootstrapConfiguration) New( uopts := uninitialized.NewOptions(). SetResultOptions(rsOpts). SetInstrumentOptions(opts.InstrumentOptions()) - bs, err = uninitialized.NewuninitializedTopologyBootstrapperProvider(uopts, bs) - if err != nil { - return nil, err - } + bs = uninitialized.NewuninitializedTopologyBootstrapperProvider(uopts, bs) default: return nil, fmt.Errorf("unknown bootstrapper: %s", bsc.Bootstrappers[i]) } diff --git a/src/dbnode/storage/bootstrap/bootstrapper/commitlog/source_prop_test.go b/src/dbnode/storage/bootstrap/bootstrapper/commitlog/source_prop_test.go index 12a2332775..ef5ff1142b 100644 --- a/src/dbnode/storage/bootstrap/bootstrapper/commitlog/source_prop_test.go +++ b/src/dbnode/storage/bootstrap/bootstrapper/commitlog/source_prop_test.go @@ -40,7 +40,7 @@ import ( "github.com/m3db/m3/src/dbnode/persist/fs/commitlog" "github.com/m3db/m3/src/dbnode/storage/bootstrap/result" "github.com/m3db/m3/src/dbnode/storage/namespace" - topotestutils "github.com/m3db/m3/src/dbnode/topology/testutil" + tu "github.com/m3db/m3/src/dbnode/topology/testutil" "github.com/m3db/m3/src/dbnode/ts" "github.com/m3db/m3cluster/shard" "github.com/m3db/m3x/checked" @@ -328,13 +328,9 @@ func TestCommitLogSourcePropCorrectlyBootstrapsFromCommitlog(t *testing.T) { // Perform the bootstrap var ( - initialTopoState = topotestutils.SourceAvailableHosts{ - topotestutils.SourceAvailableHost{ - Name: topotestutils.SelfID, - Shards: allShardsSlice, - ShardStates: shard.Available, - }, - }.TopologyState(1) + initialTopoState = tu.NewStateSnapshot(1, tu.HostShardStates{ + tu.SelfID: tu.Shards(allShardsSlice, shard.Available), + }) runOpts = testDefaultRunOpts.SetInitialTopologyState(initialTopoState) ) dataResult, err := source.BootstrapData(nsMeta, shardTimeRanges, runOpts) diff --git a/src/dbnode/storage/bootstrap/bootstrapper/commitlog/source_test.go b/src/dbnode/storage/bootstrap/bootstrapper/commitlog/source_test.go index afb2a23883..96cbc56485 100644 --- a/src/dbnode/storage/bootstrap/bootstrapper/commitlog/source_test.go +++ b/src/dbnode/storage/bootstrap/bootstrapper/commitlog/source_test.go @@ -26,6 +26,7 @@ import ( "github.com/m3db/m3/src/dbnode/persist/fs" "github.com/m3db/m3/src/dbnode/storage/bootstrap/result" + "github.com/m3db/m3/src/dbnode/topology" tu "github.com/m3db/m3/src/dbnode/topology/testutil" "github.com/m3db/m3cluster/shard" xtime "github.com/m3db/m3x/time" @@ -42,7 +43,7 @@ func TestAvailableData(t *testing.T) { var ( nsMetadata = testNsMetadata(t) blockSize = 2 * time.Hour - shards = []uint32{0, 1, 2, 3} + numShards = uint32(4) blockStart = time.Now().Truncate(blockSize) shardTimeRangesToBootstrap = result.ShardTimeRanges{} bootstrapRanges = xtime.Ranges{}.AddRange(xtime.Range{ @@ -51,102 +52,63 @@ func TestAvailableData(t *testing.T) { }) ) - for _, shard := range shards { - shardTimeRangesToBootstrap[shard] = bootstrapRanges + for i := 0; i < int(numShards); i++ { + shardTimeRangesToBootstrap[uint32(i)] = bootstrapRanges } testCases := []struct { title string - hosts tu.SourceAvailableHosts - majorityReplicas int + topoState *topology.StateSnapshot shardsTimeRangesToBootstrap result.ShardTimeRanges expectedAvailableShardsTimeRanges result.ShardTimeRanges }{ { title: "Single node - Shard initializing", - hosts: []tu.SourceAvailableHost{ - tu.SourceAvailableHost{ - Name: tu.SelfID, - Shards: shards, - ShardStates: shard.Initializing, - }, - }, - majorityReplicas: 1, + topoState: tu.NewStateSnapshot(1, tu.HostShardStates{ + tu.SelfID: tu.ShardsRange(0, numShards, shard.Initializing), + }), shardsTimeRangesToBootstrap: shardTimeRangesToBootstrap, expectedAvailableShardsTimeRanges: result.ShardTimeRanges{}, }, { title: "Single node - Shard unknown", - hosts: []tu.SourceAvailableHost{ - tu.SourceAvailableHost{ - Name: tu.SelfID, - Shards: shards, - ShardStates: shard.Unknown, - }, - }, - majorityReplicas: 1, + topoState: tu.NewStateSnapshot(1, tu.HostShardStates{ + tu.SelfID: tu.ShardsRange(0, numShards, shard.Unknown), + }), shardsTimeRangesToBootstrap: shardTimeRangesToBootstrap, expectedAvailableShardsTimeRanges: result.ShardTimeRanges{}, }, { title: "Single node - Shard leaving", - hosts: []tu.SourceAvailableHost{ - tu.SourceAvailableHost{ - Name: tu.SelfID, - Shards: shards, - ShardStates: shard.Leaving, - }, - }, - majorityReplicas: 1, + topoState: tu.NewStateSnapshot(1, tu.HostShardStates{ + tu.SelfID: tu.ShardsRange(0, numShards, shard.Leaving), + }), shardsTimeRangesToBootstrap: shardTimeRangesToBootstrap, expectedAvailableShardsTimeRanges: shardTimeRangesToBootstrap, }, { title: "Single node - Shard available", - hosts: []tu.SourceAvailableHost{ - tu.SourceAvailableHost{ - Name: tu.SelfID, - Shards: shards, - ShardStates: shard.Available, - }, - }, - majorityReplicas: 1, + topoState: tu.NewStateSnapshot(1, tu.HostShardStates{ + tu.SelfID: tu.ShardsRange(0, numShards, shard.Available), + }), shardsTimeRangesToBootstrap: shardTimeRangesToBootstrap, expectedAvailableShardsTimeRanges: shardTimeRangesToBootstrap, }, { title: "Multi node - Origin available", - hosts: []tu.SourceAvailableHost{ - tu.SourceAvailableHost{ - Name: tu.SelfID, - Shards: shards, - ShardStates: shard.Available, - }, - tu.SourceAvailableHost{ - Name: notSelfID, - Shards: shards, - ShardStates: shard.Initializing, - }, - }, - majorityReplicas: 1, + topoState: tu.NewStateSnapshot(1, tu.HostShardStates{ + tu.SelfID: tu.ShardsRange(0, numShards, shard.Available), + notSelfID: tu.ShardsRange(0, numShards, shard.Initializing), + }), shardsTimeRangesToBootstrap: shardTimeRangesToBootstrap, expectedAvailableShardsTimeRanges: shardTimeRangesToBootstrap, }, { title: "Multi node - Origin not available", - hosts: []tu.SourceAvailableHost{ - tu.SourceAvailableHost{ - Name: tu.SelfID, - Shards: shards, - ShardStates: shard.Initializing, - }, - tu.SourceAvailableHost{ - Name: notSelfID, - Shards: shards, - ShardStates: shard.Available, - }, - }, - majorityReplicas: 1, + topoState: tu.NewStateSnapshot(1, tu.HostShardStates{ + tu.SelfID: tu.ShardsRange(0, numShards, shard.Initializing), + notSelfID: tu.ShardsRange(0, numShards, shard.Available), + }), shardsTimeRangesToBootstrap: shardTimeRangesToBootstrap, expectedAvailableShardsTimeRanges: result.ShardTimeRanges{}, }, @@ -156,10 +118,9 @@ func TestAvailableData(t *testing.T) { t.Run(tc.title, func(t *testing.T) { var ( - src = newCommitLogSource(testOptions(), fs.Inspection{}) - topoState = tc.hosts.TopologyState(tc.majorityReplicas) - runOpts = testDefaultRunOpts.SetInitialTopologyState(topoState) - dataRes = src.AvailableData(nsMetadata, tc.shardsTimeRangesToBootstrap, runOpts) + src = newCommitLogSource(testOptions(), fs.Inspection{}) + runOpts = testDefaultRunOpts.SetInitialTopologyState(tc.topoState) + dataRes = src.AvailableData(nsMetadata, tc.shardsTimeRangesToBootstrap, runOpts) ) require.Equal(t, tc.expectedAvailableShardsTimeRanges, dataRes) diff --git a/src/dbnode/storage/bootstrap/bootstrapper/peers/source_test.go b/src/dbnode/storage/bootstrap/bootstrapper/peers/source_test.go index 8247c5aaf1..419ca8d6a4 100644 --- a/src/dbnode/storage/bootstrap/bootstrapper/peers/source_test.go +++ b/src/dbnode/storage/bootstrap/bootstrapper/peers/source_test.go @@ -47,7 +47,7 @@ func TestPeersSourceAvailableDataAndIndex(t *testing.T) { var ( blockSize = 2 * time.Hour nsMetadata = testNamespaceMetadata(t) - shards = []uint32{0, 1, 2, 3} + numShards = uint32(4) blockStart = time.Now().Truncate(blockSize) shardTimeRangesToBootstrap = result.ShardTimeRanges{} bootstrapRanges = xtime.Ranges{}.AddRange(xtime.Range{ @@ -56,8 +56,8 @@ func TestPeersSourceAvailableDataAndIndex(t *testing.T) { }) ) - for _, shard := range shards { - shardTimeRangesToBootstrap[shard] = bootstrapRanges + for i := 0; i < int(numShards); i++ { + shardTimeRangesToBootstrap[uint32(i)] = bootstrapRanges } shardTimeRangesToBootstrapOneExtra := shardTimeRangesToBootstrap.Copy() @@ -65,117 +65,60 @@ func TestPeersSourceAvailableDataAndIndex(t *testing.T) { testCases := []struct { title string - hosts tu.SourceAvailableHosts - majorityReplicas int + topoState *topology.StateSnapshot bootstrapReadConsistency topology.ReadConsistencyLevel shardsTimeRangesToBootstrap result.ShardTimeRanges expectedAvailableShardsTimeRanges result.ShardTimeRanges }{ { title: "Returns empty if only self is available", - hosts: []tu.SourceAvailableHost{ - tu.SourceAvailableHost{ - Name: tu.SelfID, - Shards: shards, - ShardStates: shard.Available, - }, - }, - majorityReplicas: 1, + topoState: tu.NewStateSnapshot(1, tu.HostShardStates{ + tu.SelfID: tu.ShardsRange(0, numShards, shard.Available), + }), bootstrapReadConsistency: topology.ReadConsistencyLevelMajority, shardsTimeRangesToBootstrap: shardTimeRangesToBootstrap, expectedAvailableShardsTimeRanges: result.ShardTimeRanges{}, }, { title: "Returns empty if all other peers initializing/unknown", - hosts: []tu.SourceAvailableHost{ - tu.SourceAvailableHost{ - Name: tu.SelfID, - Shards: shards, - ShardStates: shard.Available, - }, - tu.SourceAvailableHost{ - Name: notSelfID1, - Shards: shards, - ShardStates: shard.Initializing, - }, - tu.SourceAvailableHost{ - Name: notSelfID2, - Shards: shards, - ShardStates: shard.Unknown, - }, - }, - majorityReplicas: 2, + topoState: tu.NewStateSnapshot(2, tu.HostShardStates{ + tu.SelfID: tu.ShardsRange(0, numShards, shard.Available), + notSelfID1: tu.ShardsRange(0, numShards, shard.Initializing), + notSelfID2: tu.ShardsRange(0, numShards, shard.Unknown), + }), bootstrapReadConsistency: topology.ReadConsistencyLevelMajority, shardsTimeRangesToBootstrap: shardTimeRangesToBootstrap, expectedAvailableShardsTimeRanges: result.ShardTimeRanges{}, }, { title: "Returns success if consistency can be met (available/leaving)", - hosts: []tu.SourceAvailableHost{ - tu.SourceAvailableHost{ - Name: tu.SelfID, - Shards: shards, - ShardStates: shard.Initializing, - }, - tu.SourceAvailableHost{ - Name: notSelfID1, - Shards: shards, - ShardStates: shard.Available, - }, - tu.SourceAvailableHost{ - Name: notSelfID2, - Shards: shards, - ShardStates: shard.Leaving, - }, - }, + topoState: tu.NewStateSnapshot(2, tu.HostShardStates{ + tu.SelfID: tu.ShardsRange(0, numShards, shard.Initializing), + notSelfID1: tu.ShardsRange(0, numShards, shard.Available), + notSelfID2: tu.ShardsRange(0, numShards, shard.Leaving), + }), bootstrapReadConsistency: topology.ReadConsistencyLevelMajority, shardsTimeRangesToBootstrap: shardTimeRangesToBootstrap, expectedAvailableShardsTimeRanges: shardTimeRangesToBootstrap, }, { title: "Skips shards that were not in the topology at start", - hosts: []tu.SourceAvailableHost{ - tu.SourceAvailableHost{ - Name: tu.SelfID, - Shards: shards, - ShardStates: shard.Initializing, - }, - tu.SourceAvailableHost{ - Name: notSelfID1, - Shards: shards, - ShardStates: shard.Available, - }, - tu.SourceAvailableHost{ - Name: notSelfID2, - Shards: shards, - ShardStates: shard.Available, - }, - }, - majorityReplicas: 2, + topoState: tu.NewStateSnapshot(2, tu.HostShardStates{ + tu.SelfID: tu.ShardsRange(0, numShards, shard.Initializing), + notSelfID1: tu.ShardsRange(0, numShards, shard.Available), + notSelfID2: tu.ShardsRange(0, numShards, shard.Available), + }), bootstrapReadConsistency: topology.ReadConsistencyLevelMajority, shardsTimeRangesToBootstrap: shardTimeRangesToBootstrapOneExtra, expectedAvailableShardsTimeRanges: shardTimeRangesToBootstrap, }, { title: "Returns empty if consistency can not be met", - hosts: []tu.SourceAvailableHost{ - tu.SourceAvailableHost{ - Name: tu.SelfID, - Shards: shards, - ShardStates: shard.Initializing, - }, - tu.SourceAvailableHost{ - Name: notSelfID1, - Shards: shards, - ShardStates: shard.Available, - }, - tu.SourceAvailableHost{ - Name: notSelfID2, - Shards: shards, - ShardStates: shard.Available, - }, - }, - majorityReplicas: 2, + topoState: tu.NewStateSnapshot(2, tu.HostShardStates{ + tu.SelfID: tu.ShardsRange(0, numShards, shard.Initializing), + notSelfID1: tu.ShardsRange(0, numShards, shard.Available), + notSelfID2: tu.ShardsRange(0, numShards, shard.Available), + }), bootstrapReadConsistency: topology.ReadConsistencyLevelAll, shardsTimeRangesToBootstrap: shardTimeRangesToBootstrap, expectedAvailableShardsTimeRanges: result.ShardTimeRanges{}, @@ -205,9 +148,8 @@ func TestPeersSourceAvailableDataAndIndex(t *testing.T) { require.NoError(t, err) var ( - topoState = tc.hosts.TopologyState(tc.majorityReplicas) - runOpts = testDefaultRunOpts.SetInitialTopologyState(topoState) - dataRes = src.AvailableData(nsMetadata, tc.shardsTimeRangesToBootstrap, runOpts) + runOpts = testDefaultRunOpts.SetInitialTopologyState(tc.topoState) + dataRes = src.AvailableData(nsMetadata, tc.shardsTimeRangesToBootstrap, runOpts) ) require.Equal(t, tc.expectedAvailableShardsTimeRanges, dataRes) diff --git a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/options.go b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/options.go index 32b10f333b..ae90518896 100644 --- a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/options.go +++ b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/options.go @@ -42,18 +42,6 @@ func NewOptions() Options { return &options{} } -func (o *options) Validate() error { - if o.iOpts == nil { - return errInstrumentOptsNotSet - } - - if o.resultOpts == nil { - return errResultOptsNotSet - } - - return nil -} - func (o *options) SetResultOptions(value result.Options) Options { opts := *o opts.resultOpts = value diff --git a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/provider.go b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/provider.go index 6dc7046810..a47dd97f05 100644 --- a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/provider.go +++ b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/provider.go @@ -40,15 +40,11 @@ type uninitializedTopologyBootstrapperProvider struct { func NewuninitializedTopologyBootstrapperProvider( opts Options, next bootstrap.BootstrapperProvider, -) (bootstrap.BootstrapperProvider, error) { - if err := opts.Validate(); err != nil { - return nil, err - } - +) bootstrap.BootstrapperProvider { return uninitializedTopologyBootstrapperProvider{ opts: opts, next: next, - }, nil + } } func (p uninitializedTopologyBootstrapperProvider) Provide() (bootstrap.Bootstrapper, error) { diff --git a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source_test.go b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source_test.go index 5902a9489b..be20411eea 100644 --- a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source_test.go +++ b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/source_test.go @@ -27,6 +27,7 @@ import ( "github.com/m3db/m3/src/dbnode/storage/bootstrap" "github.com/m3db/m3/src/dbnode/storage/bootstrap/result" "github.com/m3db/m3/src/dbnode/storage/namespace" + "github.com/m3db/m3/src/dbnode/topology" tu "github.com/m3db/m3/src/dbnode/topology/testutil" "github.com/m3db/m3cluster/shard" "github.com/m3db/m3x/ident" @@ -47,7 +48,7 @@ var ( func TestUnitializedTopologySourceAvailableDataAndAvailableIndex(t *testing.T) { var ( blockSize = 2 * time.Hour - shards = []uint32{0, 1, 2, 3} + numShards = uint32(4) blockStart = time.Now().Truncate(blockSize) shardTimeRangesToBootstrap = result.ShardTimeRanges{} bootstrapRanges = xtime.Ranges{}.AddRange(xtime.Range{ @@ -58,99 +59,65 @@ func TestUnitializedTopologySourceAvailableDataAndAvailableIndex(t *testing.T) { nsMetadata, err := namespace.NewMetadata(testNamespaceID, namespace.NewOptions()) require.NoError(t, err) - for _, shard := range shards { - shardTimeRangesToBootstrap[shard] = bootstrapRanges + for i := 0; i < int(numShards); i++ { + shardTimeRangesToBootstrap[uint32(i)] = bootstrapRanges } testCases := []struct { title string - majorityReplicas int - hosts tu.SourceAvailableHosts + topoState *topology.StateSnapshot shardsTimeRangesToBootstrap result.ShardTimeRanges expectedAvailableShardsTimeRanges result.ShardTimeRanges }{ // Snould return that it can bootstrap everything because // it's a new namespace. { - title: "Single node - Shard initializing", - majorityReplicas: 1, - hosts: []tu.SourceAvailableHost{ - tu.SourceAvailableHost{ - Name: tu.SelfID, - Shards: shards, - ShardStates: shard.Initializing, - }, - }, + title: "Single node - Shard initializing", + topoState: tu.NewStateSnapshot(1, tu.HostShardStates{ + tu.SelfID: tu.ShardsRange(0, numShards, shard.Initializing), + }), shardsTimeRangesToBootstrap: shardTimeRangesToBootstrap, expectedAvailableShardsTimeRanges: shardTimeRangesToBootstrap, }, // Snould return that it can't bootstrap anything because we don't // know how to handle unknown shard states. { - title: "Single node - Shard unknown", - majorityReplicas: 1, - hosts: []tu.SourceAvailableHost{ - tu.SourceAvailableHost{ - Name: tu.SelfID, - Shards: shards, - ShardStates: shard.Unknown, - }, - }, + title: "Single node - Shard unknown", + topoState: tu.NewStateSnapshot(1, tu.HostShardStates{ + tu.SelfID: tu.ShardsRange(0, numShards, shard.Unknown), + }), shardsTimeRangesToBootstrap: shardTimeRangesToBootstrap, expectedAvailableShardsTimeRanges: result.ShardTimeRanges{}, }, // Snould return that it can't bootstrap anything because it's not // a new namespace. { - title: "Single node - Shard leaving", - majorityReplicas: 1, - hosts: []tu.SourceAvailableHost{ - tu.SourceAvailableHost{ - Name: tu.SelfID, - Shards: shards, - ShardStates: shard.Leaving, - }, - }, + title: "Single node - Shard leaving", + topoState: tu.NewStateSnapshot(1, tu.HostShardStates{ + tu.SelfID: tu.ShardsRange(0, numShards, shard.Leaving), + }), shardsTimeRangesToBootstrap: shardTimeRangesToBootstrap, expectedAvailableShardsTimeRanges: result.ShardTimeRanges{}, }, // Snould return that it can't bootstrap anything because it's not // a new namespace. { - title: "Single node - Shard available", - majorityReplicas: 1, - hosts: []tu.SourceAvailableHost{ - tu.SourceAvailableHost{ - Name: tu.SelfID, - Shards: shards, - ShardStates: shard.Available, - }, - }, + title: "Single node - Shard available", + topoState: tu.NewStateSnapshot(1, tu.HostShardStates{ + tu.SelfID: tu.ShardsRange(0, numShards, shard.Available), + }), shardsTimeRangesToBootstrap: shardTimeRangesToBootstrap, expectedAvailableShardsTimeRanges: result.ShardTimeRanges{}, }, // Snould return that it can bootstrap everything because // it's a new namespace. { - title: "Multi node - Brand new namespace (all nodes initializing)", - majorityReplicas: 2, - hosts: []tu.SourceAvailableHost{ - tu.SourceAvailableHost{ - Name: tu.SelfID, - Shards: shards, - ShardStates: shard.Initializing, - }, - tu.SourceAvailableHost{ - Name: notSelfID1, - Shards: shards, - ShardStates: shard.Initializing, - }, - tu.SourceAvailableHost{ - Name: notSelfID2, - Shards: shards, - ShardStates: shard.Initializing, - }, - }, + title: "Multi node - Brand new namespace (all nodes initializing)", + topoState: tu.NewStateSnapshot(2, tu.HostShardStates{ + tu.SelfID: tu.ShardsRange(0, numShards, shard.Initializing), + notSelfID1: tu.ShardsRange(0, numShards, shard.Initializing), + notSelfID2: tu.ShardsRange(0, numShards, shard.Initializing), + }), shardsTimeRangesToBootstrap: shardTimeRangesToBootstrap, expectedAvailableShardsTimeRanges: shardTimeRangesToBootstrap, }, @@ -158,105 +125,49 @@ func TestUnitializedTopologySourceAvailableDataAndAvailableIndex(t *testing.T) { // it's a new namespace (one of the nodes hasn't completed // initializing yet.) { - title: "Multi node - Recently created namespace (one node still initializing)", - majorityReplicas: 2, - hosts: []tu.SourceAvailableHost{ - tu.SourceAvailableHost{ - Name: tu.SelfID, - Shards: shards, - ShardStates: shard.Initializing, - }, - tu.SourceAvailableHost{ - Name: notSelfID1, - Shards: shards, - ShardStates: shard.Available, - }, - tu.SourceAvailableHost{ - Name: notSelfID2, - Shards: shards, - ShardStates: shard.Available, - }, - }, + title: "Multi node - Recently created namespace (one node still initializing)", + topoState: tu.NewStateSnapshot(2, tu.HostShardStates{ + tu.SelfID: tu.ShardsRange(0, numShards, shard.Initializing), + notSelfID1: tu.ShardsRange(0, numShards, shard.Available), + notSelfID2: tu.ShardsRange(0, numShards, shard.Available), + }), shardsTimeRangesToBootstrap: shardTimeRangesToBootstrap, expectedAvailableShardsTimeRanges: shardTimeRangesToBootstrap, }, // Snould return that it can't bootstrap anything because it's not // a new namespace. { - title: "Multi node - Initialized namespace (no nodes initializing)", - majorityReplicas: 2, - hosts: []tu.SourceAvailableHost{ - tu.SourceAvailableHost{ - Name: tu.SelfID, - Shards: shards, - ShardStates: shard.Available, - }, - tu.SourceAvailableHost{ - Name: notSelfID1, - Shards: shards, - ShardStates: shard.Available, - }, - tu.SourceAvailableHost{ - Name: notSelfID2, - Shards: shards, - ShardStates: shard.Available, - }, - }, + title: "Multi node - Initialized namespace (no nodes initializing)", + topoState: tu.NewStateSnapshot(2, tu.HostShardStates{ + tu.SelfID: tu.ShardsRange(0, numShards, shard.Available), + notSelfID1: tu.ShardsRange(0, numShards, shard.Available), + notSelfID2: tu.ShardsRange(0, numShards, shard.Available), + }), shardsTimeRangesToBootstrap: shardTimeRangesToBootstrap, expectedAvailableShardsTimeRanges: result.ShardTimeRanges{}, }, // Snould return that it can't bootstrap anything because it's not // a new namespace, we're just doing a node replace. { - title: "Multi node - Node replace (one node leaving, one initializing)", - majorityReplicas: 2, - hosts: []tu.SourceAvailableHost{ - tu.SourceAvailableHost{ - Name: tu.SelfID, - Shards: shards, - ShardStates: shard.Available, - }, - tu.SourceAvailableHost{ - Name: notSelfID1, - Shards: shards, - ShardStates: shard.Leaving, - }, - tu.SourceAvailableHost{ - Name: notSelfID2, - Shards: shards, - ShardStates: shard.Available, - }, - tu.SourceAvailableHost{ - Name: notSelfID3, - Shards: shards, - ShardStates: shard.Initializing, - }, - }, + title: "Multi node - Node replace (one node leaving, one initializing)", + topoState: tu.NewStateSnapshot(2, tu.HostShardStates{ + tu.SelfID: tu.ShardsRange(0, numShards, shard.Available), + notSelfID1: tu.ShardsRange(0, numShards, shard.Leaving), + notSelfID2: tu.ShardsRange(0, numShards, shard.Available), + notSelfID3: tu.ShardsRange(0, numShards, shard.Initializing), + }), shardsTimeRangesToBootstrap: shardTimeRangesToBootstrap, expectedAvailableShardsTimeRanges: result.ShardTimeRanges{}, }, // Snould return that it can't bootstrap anything because we don't // know how to interpret the unknown host. { - title: "Multi node - One node unknown", - majorityReplicas: 2, - hosts: []tu.SourceAvailableHost{ - tu.SourceAvailableHost{ - Name: tu.SelfID, - Shards: shards, - ShardStates: shard.Available, - }, - tu.SourceAvailableHost{ - Name: notSelfID1, - Shards: shards, - ShardStates: shard.Available, - }, - tu.SourceAvailableHost{ - Name: notSelfID2, - Shards: shards, - ShardStates: shard.Unknown, - }, - }, + title: "Multi node - One node unknown", + topoState: tu.NewStateSnapshot(2, tu.HostShardStates{ + tu.SelfID: tu.ShardsRange(0, numShards, shard.Available), + notSelfID1: tu.ShardsRange(0, numShards, shard.Available), + notSelfID2: tu.ShardsRange(0, numShards, shard.Unknown), + }), shardsTimeRangesToBootstrap: shardTimeRangesToBootstrap, expectedAvailableShardsTimeRanges: result.ShardTimeRanges{}, }, @@ -268,7 +179,7 @@ func TestUnitializedTopologySourceAvailableDataAndAvailableIndex(t *testing.T) { var ( srcOpts = NewOptions().SetInstrumentOptions(instrument.NewOptions()) src = newTopologyUninitializedSource(srcOpts) - runOpts = testDefaultRunOpts.SetInitialTopologyState(tc.hosts.TopologyState(tc.majorityReplicas)) + runOpts = testDefaultRunOpts.SetInitialTopologyState(tc.topoState) dataAvailabilityResult = src.AvailableData(nsMetadata, tc.shardsTimeRangesToBootstrap, runOpts) indexAvailabilityResult = src.AvailableIndex(nsMetadata, tc.shardsTimeRangesToBootstrap, runOpts) ) diff --git a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/types.go b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/types.go index 7bd20717c4..5e529ad6e0 100644 --- a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/types.go +++ b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/types.go @@ -27,9 +27,6 @@ import ( // Options is the options interface for the uninitialized source. type Options interface { - // Validate validates the options are correct. - Validate() error - // SetResultOptions sets the result options SetResultOptions(value result.Options) Options diff --git a/src/dbnode/topology/testutil/topology.go b/src/dbnode/topology/testutil/topology.go index 03410818d4..27df4ffbbc 100644 --- a/src/dbnode/topology/testutil/topology.go +++ b/src/dbnode/topology/testutil/topology.go @@ -107,38 +107,31 @@ func (v TopologyView) Map() (topology.Map, error) { return topology.NewStaticMap(opts), nil } -// SourceAvailableHost is a human-friendly way of constructing -// TopologyStates for test cases. -type SourceAvailableHost struct { - Name string - Shards []uint32 - ShardStates shard.State -} - -// SourceAvailableHosts is a slice of SourceAvailableHost. -type SourceAvailableHosts []SourceAvailableHost +// HostShardStates is a human-readable way of describing an initial state topology +// on a host-by-host basis. +type HostShardStates map[string][]shard.Shard -// TopologyState returns a topology.StateSnapshot that is equivalent to -// the slice of SourceAvailableHosts. -func (s SourceAvailableHosts) TopologyState(numMajorityReplicas int) *topology.StateSnapshot { +// NewStateSnapshot creates a new initial topology state snapshot using HostShardStates +// as input. +func NewStateSnapshot(numMajorityReplicas int, hostShardStates HostShardStates) *topology.StateSnapshot { topoState := &topology.StateSnapshot{ Origin: topology.NewHost(SelfID, "127.0.0.1"), MajorityReplicas: numMajorityReplicas, ShardStates: make(map[topology.ShardID]map[topology.HostID]topology.HostShardState), } - for _, host := range s { - for _, shard := range host.Shards { - hostShardStates, ok := topoState.ShardStates[topology.ShardID(shard)] + for host, shards := range hostShardStates { + for _, shard := range shards { + hostShardStates, ok := topoState.ShardStates[topology.ShardID(shard.ID())] if !ok { hostShardStates = make(map[topology.HostID]topology.HostShardState) } - hostShardStates[topology.HostID(host.Name)] = topology.HostShardState{ - Host: topology.NewHost(host.Name, host.Name+"address"), - ShardState: host.ShardStates, + hostShardStates[topology.HostID(host)] = topology.HostShardState{ + Host: topology.NewHost(host, host+"address"), + ShardState: shard.State(), } - topoState.ShardStates[topology.ShardID(shard)] = hostShardStates + topoState.ShardStates[topology.ShardID(shard.ID())] = hostShardStates } } From c27062a1d385965834182d98676dc91da0a176ee Mon Sep 17 00:00:00 2001 From: Richard Artoul Date: Mon, 17 Sep 2018 15:18:46 -0400 Subject: [PATCH 27/28] Set default for uninitialized source options --- .../storage/bootstrap/bootstrapper/uninitialized/options.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/options.go b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/options.go index ae90518896..1afa0673e4 100644 --- a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/options.go +++ b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/options.go @@ -39,7 +39,10 @@ type options struct { // NewOptions creates a new Options. func NewOptions() Options { - return &options{} + return &options{ + resultOpts: result.NewOptions(), + iOpts: instrument.NewOptions(), + } } func (o *options) SetResultOptions(value result.Options) Options { From 1d11b84765121236fa56c496f9a36ee478d9cd76 Mon Sep 17 00:00:00 2001 From: Richard Artoul Date: Mon, 17 Sep 2018 15:34:07 -0400 Subject: [PATCH 28/28] Delete unused errors --- .../bootstrap/bootstrapper/uninitialized/options.go | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/options.go b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/options.go index 1afa0673e4..dfc75bbf9e 100644 --- a/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/options.go +++ b/src/dbnode/storage/bootstrap/bootstrapper/uninitialized/options.go @@ -21,17 +21,10 @@ package uninitialized import ( - "errors" - "github.com/m3db/m3/src/dbnode/storage/bootstrap/result" "github.com/m3db/m3x/instrument" ) -var ( - errInstrumentOptsNotSet = errors.New("instrument options not set") - errResultOptsNotSet = errors.New("result options not set") -) - type options struct { resultOpts result.Options iOpts instrument.Options