diff --git a/Makefile b/Makefile index 9114fda54..9eabcc9ea 100644 --- a/Makefile +++ b/Makefile @@ -311,8 +311,8 @@ install-gometalinter: GO111MODULE=off go get github.com/remyoudompheng/go-misc/deadcode test-txstore-rbf: - PILOSA_TXSRC=rbf $(MAKE) testv-race + PILOSA_STORAGE_BACKEND=rbf $(MAKE) testv-race test-txstore-rbf_bolt: - PILOSA_TXSRC=rbf_bolt $(MAKE) testv-race + PILOSA_STORAGE_BACKEND=rbf_bolt $(MAKE) testv-race diff --git a/cluster_internal_test.go b/cluster_internal_test.go index ea391b52d..f6f40af04 100644 --- a/cluster_internal_test.go +++ b/cluster_internal_test.go @@ -138,6 +138,22 @@ func TestFragCombos(t *testing.T) { } } +// newHolderWithTempPath returns a new instance of Holder. +func newHolderWithTempPath(tb testing.TB, backend string) *Holder { + path, err := testhook.TempDirInDir(tb, *TempDir, "pilosa-holder-") + if err != nil { + panic(err) + } + cfg := mustHolderConfig() + cfg.StorageConfig.Backend = backend + h := NewHolder(path, cfg) + panicOn(h.Open()) + testhook.Cleanup(tb, func() { + h.Close() + }) + return h +} + // newIndexWithTempPath returns a new instance of Index. func newIndexWithTempPath(tb testing.TB, name string) *Index { path, err := testhook.TempDirInDir(tb, *TempDir, "pilosa-index-") diff --git a/ctl/server.go b/ctl/server.go index c72f7d379..0d814cb86 100644 --- a/ctl/server.go +++ b/ctl/server.go @@ -102,11 +102,6 @@ func BuildServerFlags(cmd *cobra.Command, srv *server.Command) { flags.IntVar(&srv.Config.Profile.BlockRate, "profile.block-rate", srv.Config.Profile.BlockRate, "Sampling rate for goroutine blocking profiler. One sample per ns.") flags.IntVar(&srv.Config.Profile.MutexFraction, "profile.mutex-fraction", srv.Config.Profile.MutexFraction, "Sampling fraction for mutex contention profiling. Sample 1/ of events.") - // Transactional storage engine - // Note: the default for --tx must be kept "" empty string. Otherwise we - // cannot detect and honor the PILOSA_TXSRC env var over-ride. - flags.StringVarP(&srv.Config.Txsrc, "txsrc", "", "", fmt.Sprintf("transaction/storage to use: one of roaring, rbf, bolt, or a blue-green setup: rbf_roaring, roaring_rbf, bolt_roaring, roaring_bolt, bolt_rbf, etc. The default is: %v. The env var PILOSA_TXSRC is over-ridden by --txsrc option on the command line.", storage.DefaultBackend)) - // Storage // Note: the default for --storage.backend must be kept "" empty string. // Otherwise we cannot detect and honor the PILOSA_STORAGE_BACKEND env var diff --git a/ctl/server_test.go b/ctl/server_test.go index 81f49a5fd..b99a2ed25 100644 --- a/ctl/server_test.go +++ b/ctl/server_test.go @@ -35,14 +35,3 @@ func TestBuildServerFlags(t *testing.T) { t.Fatal("log-path flag is required") } } - -func TestServerDefaultTxsrcFlags(t *testing.T) { - cm := &cobra.Command{} - buf := bytes.Buffer{} - stdin, stdout, stderr := GetIO(buf) - Server := server.NewCommand(stdin, stdout, stderr) - BuildServerFlags(cm, Server) - if cm.Flags().Lookup("txsrc").DefValue != "" { - t.Fatal("cannot set the txsrc default in ctl/server.go, otherwise we won't know to let the environment override the lack of --txsrc on the command line. We want explicit command line --txsrc to override the env value.") - } -} diff --git a/dbshard.go b/dbshard.go index d08f06b87..3cd3bfceb 100644 --- a/dbshard.go +++ b/dbshard.go @@ -925,7 +925,7 @@ func listDirUnderDir(root string, includeRoot bool, requiredSuffix string, ignor // The blue is the destination -- this is always types[0]. // The green source is always types[1]. The mnemonic is blue_geen. // The blue is first, so it is in types[0]. The green -// is second, in types[1]. For example, with PILOSA_TXSRC=bolt_roaring +// is second, in types[1]. For example, with PILOSA_STORAGE_BACKEND=bolt_roaring // we have bolt as blue, and roaring as green. The contents of // bolt must be empty or exactly match roaring. If bolt // starts empty, it will be populated from roaring by diff --git a/dbshard_internal_test.go b/dbshard_internal_test.go index 18858485c..cbc3d7aaa 100644 --- a/dbshard_internal_test.go +++ b/dbshard_internal_test.go @@ -71,13 +71,9 @@ func TestShardPerDB_SetBit(t *testing.T) { // test that we find all *local* shards func Test_DBPerShard_GetShardsForIndex_LocalOnly(t *testing.T) { - tmpdir, err := ioutil.TempDir("", "Test_DBPerShard_GetShardsForIndex_LocalOnly") panicOn(err) - orig := os.Getenv("PILOSA_TXSRC") - defer os.Setenv("PILOSA_TXSRC", orig) // must restore or will mess up other tests! - v2s := NewFieldView2Shards() stdShardSet := newShardSet() for _, shard := range []uint64{93, 223, 221, 215, 219, 217} { @@ -88,11 +84,9 @@ func Test_DBPerShard_GetShardsForIndex_LocalOnly(t *testing.T) { } for _, src := range []string{"roaring", "bolt", "rbf"} { - - os.Setenv("PILOSA_TXSRC", src) - - // must make Holder AFTER setting src. - holder := NewHolder(tmpdir, nil) + cfg := mustHolderConfig() + cfg.StorageConfig.Backend = src + holder := NewHolder(tmpdir, cfg) index := "rick" idx := makeSampleRoaringDir(tmpdir, index, src, 1, holder, v2s) @@ -216,7 +210,6 @@ rick.index.txstores@@@/store-rbfdb@@/shard.0223-rbfdb@ } func makeSampleRoaringDir(root, index, txsrc string, minBytes int, h *Holder, view2shards *FieldView2Shards) (idx *Index) { - shards := []uint64{0, 93, 215, 217, 219, 221, 223} fns := strings.Split(sampleRoaringDirList[txsrc], "\n") firstDone := false @@ -328,13 +321,9 @@ func Test_DBPerShard_GetFieldView2Shards_map_from_RBF(t *testing.T) { tmpdir, err := ioutil.TempDir("", "Test_DBPerShard_GetFieldView2Shards_map_from_RBF") panicOn(err) - orig := os.Getenv("PILOSA_TXSRC") - defer os.Setenv("PILOSA_TXSRC", orig) // must restore or will mess up other tests! - - os.Setenv("PILOSA_TXSRC", "rbf") - - // must make Holder AFTER setting src. - holder := NewHolder(tmpdir, nil) + cfg := mustHolderConfig() + cfg.StorageConfig.Backend = "rbf" + holder := NewHolder(tmpdir, cfg) defer holder.Close() index := "rick" diff --git a/executor_test.go b/executor_test.go index 8c5323303..66fb91b27 100644 --- a/executor_test.go +++ b/executor_test.go @@ -25,7 +25,6 @@ import ( "io/ioutil" "math" "math/rand" - "os" "reflect" "strconv" "strings" @@ -539,7 +538,7 @@ func TestExecutor_Execute_Count(t *testing.T) { } func roaringOnlyTest(t *testing.T) { - src := os.Getenv("PILOSA_TXSRC") + src := pilosa.CurrentBackend() if src == pilosa.RoaringTxn || (storage.DefaultBackend == pilosa.RoaringTxn && src == "") { // okay to run, we are under roaring only } else { diff --git a/fragment_internal_test.go b/fragment_internal_test.go index 780a6d0b7..093b5e7aa 100644 --- a/fragment_internal_test.go +++ b/fragment_internal_test.go @@ -1720,7 +1720,7 @@ func TestFragment_RankCache_Persistence(t *testing.T) { } func roaringOnlyTest(t *testing.T) { - src := os.Getenv("PILOSA_TXSRC") + src := CurrentBackend() if src == RoaringTxn || (storage.DefaultBackend == RoaringTxn && src == "") { // okay to run, we are under roaring only } else { @@ -1729,7 +1729,7 @@ func roaringOnlyTest(t *testing.T) { } func roaringOnlyBenchmark(b *testing.B) { - src := os.Getenv("PILOSA_TXSRC") + src := CurrentBackend() if src == RoaringTxn || (storage.DefaultBackend == RoaringTxn && src == "") { // okay to run, we are under roaring only } else { @@ -3574,7 +3574,7 @@ func mustOpenBSIFragment(tb testing.TB, index, field, view string, shard uint64) func newTestHolder(tb testing.TB) *Holder { path, _ := testhook.TempDirInDir(tb, *TempDir, "holder-dir") - h := NewHolder(path, nil) + h := NewHolder(path, mustHolderConfig()) panicOn(h.Open()) testhook.Cleanup(tb, func() { h.Close() @@ -5530,8 +5530,7 @@ func TestFragment_Bug_Q2DoubleDelete(t *testing.T) { } func notBlueGreenTest(t *testing.T) { - src := os.Getenv("PILOSA_TXSRC") - if strings.Contains(src, "_") { + if strings.Contains(CurrentBackend(), "_") { t.Skip("skip under blue green") } } diff --git a/holder.go b/holder.go index 21b4ada8e..9fd37703d 100644 --- a/holder.go +++ b/holder.go @@ -242,13 +242,6 @@ func DefaultHolderConfig() *HolderConfig { func NewHolder(path string, cfg *HolderConfig) *Holder { if cfg == nil { cfg = DefaultHolderConfig() - // still want the PILOSA_TXSRC to override, for tests use. - txsrc := os.Getenv("PILOSA_TXSRC") - if txsrc != "" { - _ = MustTxsrcToTxtype(txsrc) - // INVAR: have valid txsrc. - cfg.StorageConfig.Backend = txsrc - } } if cfg.StorageConfig == nil { cfg.StorageConfig = storage.NewDefaultConfig() diff --git a/holder_internal_test.go b/holder_internal_test.go index 1db02a8b0..6ed4a63a6 100644 --- a/holder_internal_test.go +++ b/holder_internal_test.go @@ -76,12 +76,16 @@ func (t *testHolderOperator) ProcessFragment(*fragment) error { return nil } -func makeHolder(tb testing.TB) (*Holder, string, error) { +func makeHolder(tb testing.TB, backend string) (*Holder, string, error) { path, err := testhook.TempDir(tb, "pilosa-") if err != nil { return nil, "", err } - h := NewHolder(path, nil) + cfg := mustHolderConfig() + if backend != "" { + cfg.StorageConfig.Backend = backend + } + h := NewHolder(path, cfg) return h, path, h.Open() } @@ -170,7 +174,7 @@ func testHasBit(t *testing.T, h *Holder, index, field string, rowID, columnID ui } func TestHolderOperatorProcess(t *testing.T) { - h, path, err := makeHolder(t) + h, path, err := makeHolder(t, "") if err != nil { t.Fatalf("creating holder: %v", err) } @@ -200,7 +204,7 @@ func TestHolderOperatorProcess(t *testing.T) { } func TestHolderOperatorCancel(t *testing.T) { - h, path, err := makeHolder(t) + h, path, err := makeHolder(t, "") if err != nil { t.Fatalf("creating holder: %v", err) } @@ -247,3 +251,17 @@ func TestHolderOperatorCancel(t *testing.T) { t.Fatalf("holder processor did not cancel. expected something other than %#v", expected) } } + +// mustHolderConfig is meant to help minimize the number of places in the code +// where we're reading the PILOSA_STORAGE_BACKEND environment variable for +// testing purposes. Ideally we would handle this differently, but this is a +// first attempt at improving things. Note: the actual os.Getenv() call was +// moved to the CurrentBackend() function. +func mustHolderConfig() *HolderConfig { + cfg := DefaultHolderConfig() + if backend := CurrentBackend(); backend != "" { + _ = MustTxsrcToTxtype(backend) + cfg.StorageConfig.Backend = backend + } + return cfg +} diff --git a/http/handler.go b/http/handler.go index 49e3b1efa..316f5cec9 100644 --- a/http/handler.go +++ b/http/handler.go @@ -820,8 +820,7 @@ func (h *Handler) handlePostQuery(w http.ResponseWriter, r *http.Request) { req, ok := qreq.(*pilosa.QueryRequest) if DoPerQueryProfiling { - - txsrc := os.Getenv("PILOSA_TXSRC") + txsrc := pilosa.CurrentBackend() reqHash := hash(req.Query) qlen := len(req.Query) @@ -839,13 +838,6 @@ func (h *Handler) handlePostQuery(w http.ResponseWriter, r *http.Request) { defer pprof.StopCPUProfile() } // end DoPerQueryProfiling - /* - er = trace.Start(f) - if er != nil { - panic(er) - } - defer trace.Stop() - */ var err error err, _ = qerr.(error) diff --git a/pilosa.go b/pilosa.go index edb0240d1..39c98f34f 100644 --- a/pilosa.go +++ b/pilosa.go @@ -16,6 +16,7 @@ package pilosa import ( "encoding/json" + "os" "regexp" "time" @@ -215,3 +216,11 @@ func AddressWithDefaults(addr string) (*pnet.URI, error) { } return pnet.NewURIFromAddress(addr) } + +// CurrentBackend is one step in an attempt to centralize (and either minimize +// or completely remove), the calls to environment variables throughout the +// tests. Ideally we could get rid of this and rely completely on the +// configuration parameters. +func CurrentBackend() string { + return os.Getenv("PILOSA_STORAGE_BACKEND") +} diff --git a/pprof.go b/pprof.go index a13ef62d2..2fd9768af 100644 --- a/pprof.go +++ b/pprof.go @@ -31,7 +31,7 @@ import ( // commented out—in holder.go. func CPUProfileForDur(dur time.Duration, outpath string) { // per-query pprof output: - txsrc := os.Getenv("PILOSA_TXSRC") + txsrc := CurrentBackend() if txsrc == "" { txsrc = storage.DefaultBackend } @@ -57,7 +57,7 @@ func CPUProfileForDur(dur time.Duration, outpath string) { // commented out—in holder.go. func MemProfileForDur(dur time.Duration, outpath string) { // per-query pprof output: - txsrc := os.Getenv("PILOSA_TXSRC") + txsrc := CurrentBackend() if txsrc == "" { txsrc = storage.DefaultBackend } diff --git a/rrtx_internal_test.go b/rrtx_internal_test.go index a993b25b4..7ceefe6c9 100644 --- a/rrtx_internal_test.go +++ b/rrtx_internal_test.go @@ -15,17 +15,14 @@ package pilosa import ( - "os" "testing" ) func TestRoaring_HasData(t *testing.T) { + holder := newHolderWithTempPath(t, "roaring") - orig := os.Getenv("PILOSA_TXSRC") - defer os.Setenv("PILOSA_TXSRC", orig) // must restore or will mess up other tests! - os.Setenv("PILOSA_TXSRC", "roaring") - - idx := newIndexWithTempPath(t, "i") + idx, err := holder.CreateIndex("i", IndexOptions{}) + panicOn(err) defer idx.Close() db, err := globalRoaringReg.OpenDBWrapper(idx.path, false, nil) diff --git a/server/cluster_test.go b/server/cluster_test.go index 75878f493..9108771ac 100644 --- a/server/cluster_test.go +++ b/server/cluster_test.go @@ -20,7 +20,6 @@ import ( "fmt" "net" "net/http" - "os" "reflect" "strings" "testing" @@ -144,9 +143,9 @@ func TestClusterResize_AddNode(t *testing.T) { // Why are we skipping this test under blue-green with Roaring? // // We see red test: during resize during importRoaringBits - // PILOSA_TXSRC=rbf_roaring go test -v -tags=' shardwidth20' "-gcflags=all=-d=checkptr=0" -run TestClusterResize_AddNode/"ContinuousShards" + // PILOSA_STORAGE_BACKEND=rbf_roaring go test -v -tags=' shardwidth20' "-gcflags=all=-d=checkptr=0" -run TestClusterResize_AddNode/"ContinuousShards" // green: - // PILOSA_TXSRC=roaring_rbf go test -v -tags=' shardwidth20' "-gcflags=all=-d=checkptr=0" -run TestClusterResize_AddNode/"ContinuousShards" + // PILOSA_STORAGE_BACKEND=roaring_rbf go test -v -tags=' shardwidth20' "-gcflags=all=-d=checkptr=0" -run TestClusterResize_AddNode/"ContinuousShards" // // but rbf_badger and badger_rbf are both green (use the same data values for containers). // @@ -807,7 +806,7 @@ func TestClusterMutualTLS(t *testing.T) { } func skipTestUnderBlueGreenWithRoaring(t *testing.T) { - src := os.Getenv("PILOSA_TXSRC") + src := pilosa.CurrentBackend() if strings.Contains(src, "_") { if strings.Contains(src, "roaring") { t.Skip("skip for roaring blue-green") diff --git a/server/config.go b/server/config.go index 920320d97..f374e5db2 100644 --- a/server/config.go +++ b/server/config.go @@ -194,19 +194,17 @@ type Config struct { ConnectionLimit uint16 `toml:"max-connections"` } `toml:"postgres"` - // Txsrc determines which Tx implementation the holder/Index will use; one - // of the available transactional-storage engines. Choices are listed - // in the string constants below. Should be one of - // "roaring","bolt", "rbf", "bolt_roaring", "roaring_bolt", "rbf_roaring", - // "roaring_rbf", "bolt_rbf", "rbf_bolt", or any later addition. The - // engines with _ underscore indicate use of a blueGreenTx with a comparison - // of values back from each Tx method, and a panic if they differ. This - // is an effective test for consistency. If "rbf_roaring" is specified, then - // the roaring values are the ones actually returned from the blueGreenTx. - // If "roaring_rbf" is chosen, then the RBF values are the ones actually + // Storage.Backend determines which Tx implementation the holder/Index will + // use; one of the available transactional-storage engines. Choices are + // listed in the string constants below. Should be one of "roaring","bolt", + // "rbf", "bolt_roaring", "roaring_bolt", "rbf_roaring", "roaring_rbf", + // "bolt_rbf", "rbf_bolt", or any later addition. The engines with _ + // underscore indicate use of a blueGreenTx with a comparison of values back + // from each Tx method, and a panic if they differ. This is an effective + // test for consistency. If "rbf_roaring" is specified, then the roaring + // values are the ones actually returned from the blueGreenTx. If + // "roaring_rbf" is chosen, then the RBF values are the ones actually // returned from the blueGreenTx. - Txsrc string `toml:"txsrc"` - Storage *storage.Config `toml:"storage"` // RowcacheOn, if true, turns on the row cache for all storage backends. diff --git a/server/server.go b/server/server.go index 702f57b30..0839db450 100644 --- a/server/server.go +++ b/server/server.go @@ -55,7 +55,6 @@ import ( "github.com/pilosa/pilosa/v2/statik" "github.com/pilosa/pilosa/v2/stats" "github.com/pilosa/pilosa/v2/statsd" - "github.com/pilosa/pilosa/v2/storage" "github.com/pilosa/pilosa/v2/syswrap" "github.com/pilosa/pilosa/v2/testhook" "github.com/pkg/errors" @@ -283,32 +282,6 @@ func (m *Command) SetupServer() error { m.logger.Printf("%s", pilosa.VersionInfo()) - // If the pilosa command line uses -tx to override the - // PILOSA_TXSRC env variable, then we must also correct - // the environment, so that pilosa/txfactory.go can determine the - // desired Tx engine. This enables "go test" testing in pilosa that - // does not spin up a full server, while still respecting the pilosa - // server's choice when run full in production. - envTxsrc := os.Getenv("PILOSA_TXSRC") - if m.Config.Txsrc == "" { - // INVAR: No -tx flag on the command line. - // We defer to the environment, and then the DefaultBackend - if envTxsrc == "" { - // no env variable requested either. - m.Config.Txsrc = storage.DefaultBackend - } else { - // Tell the "regular" prod server what to use. - m.Config.Txsrc = envTxsrc - } - } - // INVAR: m.Config.Storage.Backend is valid and not "", but storage.DefaultBackend could be bad. - txty := pilosa.MustTxsrcToTxtype(m.Config.Storage.Backend) // will panic on unknown Backend. - os.Setenv("PILOSA_TXSRC", m.Config.Txsrc) - m.logger.Printf("using Txsrc '%v'/%v", m.Config.Txsrc, txty) - if len(txty) == 2 { - m.logger.Printf("blue='%v' / green='%v'", txty[0], txty[1]) - } - // validateAddrs sets the appropriate values for Bind and Advertise // based on the inputs. It is not responsible for applying defaults, although // it does provide a non-zero port (10101) in the case where no port is specified. diff --git a/stattx.go b/stattx.go index 8004e795f..68e97f7c6 100644 --- a/stattx.go +++ b/stattx.go @@ -18,7 +18,6 @@ import ( "fmt" "io" "math" - "os" "runtime" "sort" "sync" @@ -70,7 +69,7 @@ func (w *callStats) reset() { } func (c *callStats) report() (r string) { - txsrc := os.Getenv("PILOSA_TXSRC") + txsrc := CurrentBackend() r = fmt.Sprintf("callStats: (%v)\n", txsrc) c.mu.Lock() defer c.mu.Unlock() diff --git a/test/cluster.go b/test/cluster.go index 22714739f..e085ee790 100644 --- a/test/cluster.go +++ b/test/cluster.go @@ -30,6 +30,7 @@ import ( "github.com/pilosa/pilosa/v2/api/client" "github.com/pilosa/pilosa/v2/proto" "github.com/pilosa/pilosa/v2/server" + "github.com/pilosa/pilosa/v2/storage" "github.com/pilosa/pilosa/v2/test/port" "github.com/pkg/errors" "golang.org/x/sync/errgroup" @@ -446,7 +447,6 @@ func newCluster(tb testing.TB, size int, opts ...[]server.CommandOption) (*Clust // MustRunCluster creates and starts a new cluster. The opts parameter // is slightly magical; see MustNewCluster. func MustRunCluster(tb testing.TB, size int, opts ...[]server.CommandOption) *Cluster { - cluster := MustNewCluster(tb, size, opts...) err := cluster.Start() if err != nil { @@ -481,7 +481,14 @@ func prependOpts(opts [][]server.CommandOption, size int) [][]server.CommandOpti // prependTestServerOpts prepends opts with the OpenInMemTranslateStore. func prependTestServerOpts(opts []server.CommandOption) []server.CommandOption { defaultOpts := []server.CommandOption{ - server.OptCommandServerOptions(pilosa.OptServerOpenTranslateStore(pilosa.OpenInMemTranslateStore), pilosa.OptServerNodeDownRetries(5, 100*time.Millisecond)), + server.OptCommandServerOptions( + pilosa.OptServerOpenTranslateStore(pilosa.OpenInMemTranslateStore), + pilosa.OptServerNodeDownRetries(5, 100*time.Millisecond), + pilosa.OptServerStorageConfig(&storage.Config{ + Backend: pilosa.CurrentBackend(), + FsyncEnabled: true, + }), + ), } return append(defaultOpts, opts...) } diff --git a/test/pilosa.go b/test/pilosa.go index b5335ca23..e999a3cd3 100644 --- a/test/pilosa.go +++ b/test/pilosa.go @@ -70,12 +70,6 @@ func newCommand(tb testing.TB, opts ...server.CommandOption) *Command { m.Config.DataDir = path defaultConf := server.NewConfig() - // TODO: this is temporary and should be removed and - // automatically replaced with PILOSA_STORAGE_BACKEND. - if txsrc := os.Getenv("PILOSA_TXSRC"); txsrc != "" { - m.Config.Storage.Backend = txsrc - } - if m.Config.Bind == defaultConf.Bind { m.Config.Bind = "http://localhost:0" } diff --git a/tournament.sh b/tournament.sh index 793ef7231..1729ef167 100755 --- a/tournament.sh +++ b/tournament.sh @@ -1,13 +1,13 @@ #!/bin/bash ## tournament.sh runs a sequence of duels between greens and blues. -## Each test run changes the PILOSA_TXSRC and runs either +## Each test run changes the PILOSA_STORAGE_BACKEND and runs either ## one or two backends through the rigors of make testv-race. ## logs are saved to the tourna.log.${i} files. for i in rbf roaring bolt rbf_roaring roaring_rbf roaring_bolt; do echo "$(date) starting ${i}, output to tourna.log.${i}" echo "***=== ${i} ====================*** $(date)" &> tourna.log.${i} - PILOSA_TXSRC=${i} make testv-race 2>&1 > tourna.log.${i} + PILOSA_STORAGE_BACKEND=${i} make testv-race 2>&1 > tourna.log.${i} done diff --git a/tx_test.go b/tx_test.go index b942651b2..00bec7e7a 100644 --- a/tx_test.go +++ b/tx_test.go @@ -17,7 +17,6 @@ package pilosa_test import ( "context" "fmt" - "os" "strings" "testing" @@ -61,7 +60,7 @@ func queryBalances(m0api *pilosa.API, acctOwnerID uint64, fldAcct0, fldAcct1, in } func skipForRoaring(t *testing.T) { - src := os.Getenv("PILOSA_TXSRC") + src := pilosa.CurrentBackend() // once txfactory.go storage.DefaultBackend != RoaringTxn, this // will break, of course. Take out the src == "" below. if (src == "" && storage.DefaultBackend == pilosa.RoaringTxn) || strings.Contains(src, "roaring") { diff --git a/txfactory.go b/txfactory.go index 818bb8ac3..84281d63e 100644 --- a/txfactory.go +++ b/txfactory.go @@ -1284,7 +1284,7 @@ func (f *TxFactory) greenHasData() (hasData bool, err error) { // Called by test Test_TxFactory_UpdateBlueFromGreen_OnStartup() in // txfactory_internal_test.go as well. // -// This is a noop if we aren't running under a blue_green PILOSA_TXSRC. +// This is a noop if we aren't running under a blue_green PILOSA_STORAGE_BACKEND. func (f *TxFactory) green2blue(holder *Holder) (err0 error) { // Holder.Open will always call us, even without blue_green. Which is fine. diff --git a/txfactory_internal_test.go b/txfactory_internal_test.go index 7a71243d4..f3299b14f 100644 --- a/txfactory_internal_test.go +++ b/txfactory_internal_test.go @@ -23,7 +23,7 @@ import ( ) func Test_TxFactory_Qcx_query_context(t *testing.T) { - src := os.Getenv("PILOSA_TXSRC") + src := CurrentBackend() if src == "rbf" || src == "bolt" { // ok } else { @@ -114,10 +114,6 @@ func Test_TxFactory_Qcx_query_context(t *testing.T) { // and b) we have an easy migration mechanism, to go from one storage format to another. // func Test_TxFactory_UpdateBlueFromGreen_OnStartup(t *testing.T) { - - orig := os.Getenv("PILOSA_TXSRC") - defer os.Setenv("PILOSA_TXSRC", orig) // must restore or will mess up other tests! - checked := []string{"roaring", "rbf"} expectError := false @@ -140,8 +136,7 @@ func Test_TxFactory_UpdateBlueFromGreen_OnStartup(t *testing.T) { // // Setup happens with green only. - os.Setenv("PILOSA_TXSRC", green) - h, path, err := makeHolder(t) + h, path, err := makeHolder(t, green) if err != nil { t.Fatalf("creating holder: %v", err) } @@ -179,7 +174,7 @@ func Test_TxFactory_UpdateBlueFromGreen_OnStartup(t *testing.T) { testMustHaveBit(t, h, "i1", "f", 100, 200) testMustHaveBit(t, h, "i1", "f", 100, 12345678) - //vv("about to reopen; blue_green = '%v' but PILOSA_TXSRC='%v'", blue_green, os.Getenv("PILOSA_TXSRC")) + //vv("about to reopen; blue_green = '%v' but PILOSA_STORAGE_BACKEND='%v'", blue_green, os.Getenv("PILOSA_STORAGE_BACKEND")) //h.DumpAllShards() //vv("after dump, about to close") @@ -190,7 +185,7 @@ func Test_TxFactory_UpdateBlueFromGreen_OnStartup(t *testing.T) { // can we re.Open the same holder h? hopefully without a problem. panicOn(h.Open()) - //vv("h.Open() re-open worked; blue_green = '%v'; dump; with PILOSA_TXSRC='%v'", blue_green, os.Getenv("PILOSA_TXSRC")) + //vv("h.Open() re-open worked; blue_green = '%v'; dump; with PILOSA_STORAGE_BACKEND='%v'", blue_green, os.Getenv("PILOSA_STORAGE_BACKEND")) //h.DumpAllShards() testMustHaveBit(t, h, "i0", "f", rowID, colID) // panic here, colID 200 bit was cold. @@ -202,7 +197,9 @@ func Test_TxFactory_UpdateBlueFromGreen_OnStartup(t *testing.T) { // check that we can open a NewHolder on green, on same path, and still see our bits. // Because the NewHolder is the code that creates and configures TxFactory as blue_green. - h2 := NewHolder(path, nil) + cfg := mustHolderConfig() + cfg.StorageConfig.Backend = green + h2 := NewHolder(path, cfg) panicOn(h2.Open()) testMustHaveBit(t, h2, "i0", "f", rowID, colID) @@ -212,9 +209,9 @@ func Test_TxFactory_UpdateBlueFromGreen_OnStartup(t *testing.T) { // verify that blue does not have it. // open a new holder on path, just looking at blue. - os.Setenv("PILOSA_TXSRC", blue) - - h3 := NewHolder(path, nil) + cfg = mustHolderConfig() + cfg.StorageConfig.Backend = blue + h3 := NewHolder(path, cfg) panicOn(h3.Open()) testMustNotHaveBit(t, h3, "i0", "f", rowID, colID) @@ -232,11 +229,11 @@ func Test_TxFactory_UpdateBlueFromGreen_OnStartup(t *testing.T) { // Since blue is empty, the blue database will get synched up // with the green during Holder.Open(). - os.Setenv("PILOSA_TXSRC", blue_green) - // open a holder with path again, now looking at both blue and green. // The Holder.Open should do the migration from green, populating blue. - h4 := NewHolder(path, nil) + cfg = mustHolderConfig() + cfg.StorageConfig.Backend = blue_green + h4 := NewHolder(path, cfg) //vv("about to h4.Open we should populate blue from green") err = h4.Open() @@ -263,10 +260,6 @@ func Test_TxFactory_UpdateBlueFromGreen_OnStartup(t *testing.T) { // go to verify it but blue has more data than green. // That will also cause query divergence. func Test_TxFactory_verifyBlueEqualsGreen(t *testing.T) { - - orig := os.Getenv("PILOSA_TXSRC") - defer os.Setenv("PILOSA_TXSRC", orig) // must restore or will mess up other tests! - checked := []string{"roaring", "bolt", "rbf"} for _, blue := range checked { @@ -285,8 +278,7 @@ func Test_TxFactory_verifyBlueEqualsGreen(t *testing.T) { // // Setup happens with green only. - os.Setenv("PILOSA_TXSRC", green) - h, path, err := makeHolder(t) + h, path, err := makeHolder(t, green) if err != nil { t.Fatalf("creating holder: %v", err) } @@ -328,11 +320,12 @@ func Test_TxFactory_verifyBlueEqualsGreen(t *testing.T) { // verify that blue does not have it. // open a new holder on path, just looking at blue. - os.Setenv("PILOSA_TXSRC", blue) //vv("on blue, which is '%v'", blue) - h3 := NewHolder(path, nil) + cfg := mustHolderConfig() + cfg.StorageConfig.Backend = blue + h3 := NewHolder(path, cfg) panicOn(h3.Open()) testMustNotHaveBit(t, h3, "i0", "f", rowID, colID) @@ -350,13 +343,13 @@ func Test_TxFactory_verifyBlueEqualsGreen(t *testing.T) { // Since blue is empty, the blue database will get synched up // with the green during Holder.Open(). - os.Setenv("PILOSA_TXSRC", blue_green) - //vv("on blue_green, which is '%v'", blue_green) // open a holder with path again, now looking at both blue and green. // The Holder.Open should do the migration from green, populating blue. - h4 := NewHolder(path, nil) + cfg = mustHolderConfig() + cfg.StorageConfig.Backend = blue_green + h4 := NewHolder(path, cfg) panicOn(h4.Open()) testMustHaveBit(t, h4, "i0", "f", rowID, colID) @@ -365,11 +358,10 @@ func Test_TxFactory_verifyBlueEqualsGreen(t *testing.T) { h4.Close() // now open just blue, and add a bit to a new index, i2. - os.Setenv("PILOSA_TXSRC", blue) - //vv("on blue, which is '%v'", blue) - - h5 := NewHolder(path, nil) + cfg = mustHolderConfig() + cfg.StorageConfig.Backend = blue + h5 := NewHolder(path, cfg) panicOn(h5.Open()) testSetBit(t, h5, "i2", "f", 500, 777) @@ -380,13 +372,14 @@ func Test_TxFactory_verifyBlueEqualsGreen(t *testing.T) { // now open blue_green. should get a verification failure // due to the extra bit in blue. - os.Setenv("PILOSA_TXSRC", blue_green) // BEGIN verficiation that should ERROR out b/c blue has more data. // open a holder with path again, now looking at both blue and green. // The Holder.Open should verify blue against green and notice the extra bit. - h6 := NewHolder(path, nil) + cfg = mustHolderConfig() + cfg.StorageConfig.Backend = blue_green + h6 := NewHolder(path, cfg) err = h6.Open() //h6.DumpAllShards()