From 13ceb7da410d62e4a35b958e212a3764ca84f553 Mon Sep 17 00:00:00 2001 From: tidwall Date: Sat, 24 Sep 2022 15:42:07 -0700 Subject: [PATCH] Removed global variables from core package The core package uses global variables that keep from having more than one Tile38 instance runnning in the same process. Move the core variables in the server.Options type which are uniquely stated per Server instance. The build variables are still present in the core package. --- cmd/tile38-server/main.go | 56 ++++++++++++++++++++++++++---------- core/options.go | 19 ------------ internal/server/aofshrink.go | 11 ++++--- internal/server/checksum.go | 5 ++-- internal/server/follow.go | 5 ++-- internal/server/server.go | 46 ++++++++++++++++++++++------- internal/server/stats.go | 4 +-- scripts/test.sh | 6 ++++ tests/aof_test.go | 35 ++++++++++++++++++++++ tests/mock_test.go | 38 ++++++++++++++++++------ tests/tests_test.go | 10 +++++-- 11 files changed, 166 insertions(+), 69 deletions(-) delete mode 100644 core/options.go create mode 100644 tests/aof_test.go diff --git a/cmd/tile38-server/main.go b/cmd/tile38-server/main.go index cbc3d241..0479b5b8 100644 --- a/cmd/tile38-server/main.go +++ b/cmd/tile38-server/main.go @@ -141,12 +141,33 @@ Developer Options: } var ( - devMode bool nohup bool showEvioDisabled bool showThreadsDisabled bool ) + var ( + // use to be in core/options.go + + // DevMode puts application in to dev mode + devMode = false + + // ShowDebugMessages allows for log.Debug to print to console. + showDebugMessages = false + + // ProtectedMode forces Tile38 to default in protected mode. + protectedMode = "no" + + // AppendOnly allows for disabling the appendonly file. + appendOnly = true + + // AppendFileName allows for custom appendonly file path + appendFileName = "" + + // QueueFileName allows for custom queue.db file path + queueFileName = "" + ) + // parse non standard args. nargs := []string{os.Args[0]} for i := 1; i < len(os.Args); i++ { @@ -163,10 +184,10 @@ Developer Options: if i < len(os.Args) { switch strings.ToLower(os.Args[i]) { case "no": - core.ProtectedMode = "no" + protectedMode = "no" continue case "yes": - core.ProtectedMode = "yes" + protectedMode = "yes" continue } } @@ -183,10 +204,10 @@ Developer Options: if i < len(os.Args) { switch strings.ToLower(os.Args[i]) { case "no": - core.AppendOnly = false + appendOnly = false continue case "yes": - core.AppendOnly = true + appendOnly = true continue } } @@ -198,14 +219,14 @@ Developer Options: fmt.Fprintf(os.Stderr, "appendfilename must have a value\n") os.Exit(1) } - core.AppendFileName = os.Args[i] + appendFileName = os.Args[i] case "--queuefilename", "-queuefilename": i++ if i == len(os.Args) || os.Args[i] == "" { fmt.Fprintf(os.Stderr, "queuefilename must have a value\n") os.Exit(1) } - core.QueueFileName = os.Args[i] + queueFileName = os.Args[i] case "--http-transport", "-http-transport": i++ if i < len(os.Args) { @@ -308,8 +329,7 @@ Developer Options: log.Level = 1 } - core.DevMode = devMode - core.ShowDebugMessages = veryVerbose + showDebugMessages = veryVerbose hostd := "" if host != "" { @@ -449,12 +469,18 @@ Developer Options: log.Warnf("thread flag is deprecated use GOMAXPROCS to set number of threads instead") } opts := server.Options{ - Host: host, - Port: port, - Dir: dir, - UseHTTP: httpTransport, - MetricsAddr: *metricsAddr, - UnixSocketPath: unixSocket, + Host: host, + Port: port, + Dir: dir, + UseHTTP: httpTransport, + MetricsAddr: *metricsAddr, + UnixSocketPath: unixSocket, + DevMode: devMode, + ShowDebugMessages: showDebugMessages, + ProtectedMode: protectedMode, + AppendOnly: appendOnly, + AppendFileName: appendFileName, + QueueFileName: queueFileName, } if err := server.Serve(opts); err != nil { log.Fatal(err) diff --git a/core/options.go b/core/options.go deleted file mode 100644 index 76e2fcfd..00000000 --- a/core/options.go +++ /dev/null @@ -1,19 +0,0 @@ -package core - -// DevMode puts application in to dev mode -var DevMode = false - -// ShowDebugMessages allows for log.Debug to print to console. -var ShowDebugMessages = false - -// ProtectedMode forces Tile38 to default in protected mode. -var ProtectedMode = "no" - -// AppendOnly allows for disabling the appendonly file. -var AppendOnly = true - -// AppendFileName allows for custom appendonly file path -var AppendFileName = "" - -// QueueFileName allows for custom queue.db file path -var QueueFileName = "" diff --git a/internal/server/aofshrink.go b/internal/server/aofshrink.go index c47693c1..efd93a3e 100644 --- a/internal/server/aofshrink.go +++ b/internal/server/aofshrink.go @@ -8,7 +8,6 @@ import ( "time" "github.com/tidwall/btree" - "github.com/tidwall/tile38/core" "github.com/tidwall/tile38/internal/collection" "github.com/tidwall/tile38/internal/field" "github.com/tidwall/tile38/internal/log" @@ -42,7 +41,7 @@ func (s *Server) aofshrink() { }() err := func() error { - f, err := os.Create(core.AppendFileName + "-shrink") + f, err := os.Create(s.opts.AppendFileName + "-shrink") if err != nil { return err } @@ -279,13 +278,13 @@ func (s *Server) aofshrink() { if err := f.Close(); err != nil { log.Fatalf("shrink new aof close fatal operation: %v", err) } - if err := os.Rename(core.AppendFileName, core.AppendFileName+"-bak"); err != nil { + if err := os.Rename(s.opts.AppendFileName, s.opts.AppendFileName+"-bak"); err != nil { log.Fatalf("shrink backup fatal operation: %v", err) } - if err := os.Rename(core.AppendFileName+"-shrink", core.AppendFileName); err != nil { + if err := os.Rename(s.opts.AppendFileName+"-shrink", s.opts.AppendFileName); err != nil { log.Fatalf("shrink rename fatal operation: %v", err) } - s.aof, err = os.OpenFile(core.AppendFileName, os.O_CREATE|os.O_RDWR, 0600) + s.aof, err = os.OpenFile(s.opts.AppendFileName, os.O_CREATE|os.O_RDWR, 0600) if err != nil { log.Fatalf("shrink openfile fatal operation: %v", err) } @@ -296,7 +295,7 @@ func (s *Server) aofshrink() { } s.aofsz = int(n) - os.Remove(core.AppendFileName + "-bak") // ignore error + os.Remove(s.opts.AppendFileName + "-bak") // ignore error return nil }() diff --git a/internal/server/checksum.go b/internal/server/checksum.go index 3538be2b..23462be6 100644 --- a/internal/server/checksum.go +++ b/internal/server/checksum.go @@ -9,7 +9,6 @@ import ( "time" "github.com/tidwall/resp" - "github.com/tidwall/tile38/core" "github.com/tidwall/tile38/internal/log" ) @@ -140,7 +139,7 @@ func getEndOfLastValuePositionInFile(fname string, startPos int64) (int64, error // We will do some various checksums on the leader until we find the correct position to start at. func (s *Server) followCheckSome(addr string, followc int, auth string, ) (pos int64, err error) { - if core.ShowDebugMessages { + if s.opts.ShowDebugMessages { log.Debug("follow:", addr, ":check some") } s.mu.Lock() @@ -211,7 +210,7 @@ func (s *Server) followCheckSome(addr string, followc int, auth string, return 0, err } if pos == fullpos { - if core.ShowDebugMessages { + if s.opts.ShowDebugMessages { log.Debug("follow: aof fully intact") } return pos, nil diff --git a/internal/server/follow.go b/internal/server/follow.go index 3b2a6772..cc8b4222 100644 --- a/internal/server/follow.go +++ b/internal/server/follow.go @@ -9,7 +9,6 @@ import ( "time" "github.com/tidwall/resp" - "github.com/tidwall/tile38/core" "github.com/tidwall/tile38/internal/log" ) @@ -240,7 +239,7 @@ func (s *Server) followStep(host string, port int, followc int) error { if v.String() != "OK" { return errors.New("invalid response to replconf request") } - if core.ShowDebugMessages { + if s.opts.ShowDebugMessages { log.Debug("follow:", addr, ":replconf") } @@ -254,7 +253,7 @@ func (s *Server) followStep(host string, port int, followc int) error { if v.String() != "OK" { return errors.New("invalid response to aof live request") } - if core.ShowDebugMessages { + if s.opts.ShowDebugMessages { log.Debug("follow:", addr, ":read aof") } diff --git a/internal/server/server.go b/internal/server/server.go index f3093320..7c19bac3 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -135,6 +135,8 @@ type Server struct { monconnsMu sync.RWMutex monconns map[net.Conn]bool // monitor connections + + opts Options } // Options for Serve() @@ -145,15 +147,36 @@ type Options struct { UseHTTP bool MetricsAddr string UnixSocketPath string // path for unix socket + + // DevMode puts application in to dev mode + DevMode bool + + // ShowDebugMessages allows for log.Debug to print to console. + ShowDebugMessages bool + + // ProtectedMode forces Tile38 to default in protected mode. + ProtectedMode string + + // AppendOnly allows for disabling the appendonly file. + AppendOnly bool + + // AppendFileName allows for custom appendonly file path + AppendFileName string + + // QueueFileName allows for custom queue.db file path + QueueFileName string } // Serve starts a new tile38 server func Serve(opts Options) error { - if core.AppendFileName == "" { - core.AppendFileName = path.Join(opts.Dir, "appendonly.aof") + if opts.AppendFileName == "" { + opts.AppendFileName = path.Join(opts.Dir, "appendonly.aof") } - if core.QueueFileName == "" { - core.QueueFileName = path.Join(opts.Dir, "queue.db") + if opts.QueueFileName == "" { + opts.QueueFileName = path.Join(opts.Dir, "queue.db") + } + if opts.ProtectedMode == "" { + opts.ProtectedMode = "no" } log.Infof("Server started, Tile38 version %s, git %s", core.Version, core.GitSHA) @@ -183,6 +206,7 @@ func Serve(opts Options) error { groupHooks: btree.NewNonConcurrent(byGroupHook), groupObjects: btree.NewNonConcurrent(byGroupObject), hookExpires: btree.NewNonConcurrent(byHookExpires), + opts: opts, } s.epc = endpoint.NewManager(s) @@ -256,7 +280,7 @@ func Serve(opts Options) error { }() // Load the queue before the aof - qdb, err := buntdb.Open(core.QueueFileName) + qdb, err := buntdb.Open(opts.QueueFileName) if err != nil { return err } @@ -284,8 +308,8 @@ func Serve(opts Options) error { if err := s.migrateAOF(); err != nil { return err } - if core.AppendOnly { - f, err := os.OpenFile(core.AppendFileName, os.O_CREATE|os.O_RDWR, 0600) + if opts.AppendOnly { + f, err := os.OpenFile(opts.AppendFileName, os.O_CREATE|os.O_RDWR, 0600) if err != nil { return err } @@ -334,7 +358,7 @@ func Serve(opts Options) error { } func (s *Server) isProtected() bool { - if core.ProtectedMode == "no" { + if s.opts.ProtectedMode == "no" { // --protected-mode no return false } @@ -1051,19 +1075,19 @@ func (s *Server) command(msg *Message, client *Client) ( case "ttl": res, err = s.cmdTTL(msg) case "shutdown": - if !core.DevMode { + if !s.opts.DevMode { err = fmt.Errorf("unknown command '%s'", msg.Args[0]) return } log.Fatal("shutdown requested by developer") case "massinsert": - if !core.DevMode { + if !s.opts.DevMode { err = fmt.Errorf("unknown command '%s'", msg.Args[0]) return } res, err = s.cmdMassInsert(msg) case "sleep": - if !core.DevMode { + if !s.opts.DevMode { err = fmt.Errorf("unknown command '%s'", msg.Args[0]) return } diff --git a/internal/server/stats.go b/internal/server/stats.go index c0ab0bc4..84d2b962 100644 --- a/internal/server/stats.go +++ b/internal/server/stats.go @@ -318,7 +318,7 @@ func (s *Server) extStats(m map[string]interface{}) { // Whether or not a cluster is enabled m["tile38_cluster_enabled"] = false // Whether or not the Tile38 AOF is enabled - m["tile38_aof_enabled"] = core.AppendOnly + m["tile38_aof_enabled"] = s.opts.AppendOnly // Whether or not an AOF shrink is currently in progress m["tile38_aof_rewrite_in_progress"] = s.shrinking // Length of time the last AOF shrink took @@ -409,7 +409,7 @@ func boolInt(t bool) int { return 0 } func (s *Server) writeInfoPersistence(w *bytes.Buffer) { - fmt.Fprintf(w, "aof_enabled:%d\r\n", boolInt(core.AppendOnly)) + fmt.Fprintf(w, "aof_enabled:%d\r\n", boolInt(s.opts.AppendOnly)) fmt.Fprintf(w, "aof_rewrite_in_progress:%d\r\n", boolInt(s.shrinking)) // Flag indicating a AOF rewrite operation is on-going fmt.Fprintf(w, "aof_last_rewrite_time_sec:%d\r\n", s.lastShrinkDuration.get()/int(time.Second)) // Duration of the last AOF rewrite operation in seconds diff --git a/scripts/test.sh b/scripts/test.sh index 36086708..93f63322 100755 --- a/scripts/test.sh +++ b/scripts/test.sh @@ -7,6 +7,12 @@ export CGO_ENABLED=0 cd tests go test -coverpkg=../internal/server -coverprofile=/tmp/coverage.out $GOTEST + + +# go test \ +# -coverpkg=../internal/... -coverprofile=/tmp/coverage.out \ +# -v . -v ../... $GOTEST + go tool cover -html=/tmp/coverage.out -o /tmp/coverage.html echo "details: file:///tmp/coverage.html" cd .. diff --git a/tests/aof_test.go b/tests/aof_test.go new file mode 100644 index 00000000..d9cb68b4 --- /dev/null +++ b/tests/aof_test.go @@ -0,0 +1,35 @@ +package tests + +import ( + "testing" +) + +func subTestAOF(t *testing.T, mc *mockServer) { + runStep(t, mc, "loading", aof_loading_test) +} + +func aof_loading_test(mc *mockServer) error { + + // aof, err := mc.readAOF() + // if err != nil { + // return err + // } + + // aof = append(aof, "asdfasdf\r\n"...) + // aof = nil + // mc2, err := mockOpenServer(MockServerOptions{ + // Silent: false, + // Metrics: false, + // AOFData: aof, + // }) + // if err != nil { + // return err + // } + // defer mc2.Close() + + // time.Sleep(time.Minute) + + // ` + + return nil +} diff --git a/tests/mock_test.go b/tests/mock_test.go index cfd9c44d..cbec7fe3 100644 --- a/tests/mock_test.go +++ b/tests/mock_test.go @@ -7,12 +7,12 @@ import ( "log" "math/rand" "os" + "path/filepath" "strings" "time" "github.com/gomodule/redigo/redis" "github.com/tidwall/sjson" - "github.com/tidwall/tile38/core" tlog "github.com/tidwall/tile38/internal/log" "github.com/tidwall/tile38/internal/server" ) @@ -38,34 +38,55 @@ type mockServer struct { port int conn redis.Conn ioJSON bool + dir string // alt *mockServer } -func mockOpenServer(silent, metrics bool) (*mockServer, error) { +func (mc *mockServer) readAOF() ([]byte, error) { + return os.ReadFile(filepath.Join(mc.dir, "appendonly.aof")) +} + +type MockServerOptions struct { + AOFData []byte + Silent bool + Metrics bool +} + +func mockOpenServer(opts MockServerOptions) (*mockServer, error) { rand.Seed(time.Now().UnixNano()) port := rand.Int()%20000 + 20000 dir := fmt.Sprintf("data-mock-%d", port) - if !silent { + if !opts.Silent { fmt.Printf("Starting test server at port %d\n", port) } + if len(opts.AOFData) > 0 { + if err := os.MkdirAll(dir, 0777); err != nil { + return nil, err + } + err := os.WriteFile(filepath.Join(dir, "appendonly.aof"), + opts.AOFData, 0666) + if err != nil { + return nil, err + } + } logOutput := io.Discard if os.Getenv("PRINTLOG") == "1" { logOutput = os.Stderr } - core.DevMode = true s := &mockServer{port: port} tlog.SetOutput(logOutput) go func() { - opts := server.Options{ + sopts := server.Options{ Host: "localhost", Port: port, Dir: dir, UseHTTP: true, + DevMode: true, } - if metrics { - opts.MetricsAddr = ":4321" + if opts.Metrics { + sopts.MetricsAddr = ":4321" } - if err := server.Serve(opts); err != nil { + if err := server.Serve(sopts); err != nil { log.Fatal(err) } }() @@ -73,6 +94,7 @@ func mockOpenServer(silent, metrics bool) (*mockServer, error) { s.Close() return nil, err } + s.dir = dir return s, nil } diff --git a/tests/tests_test.go b/tests/tests_test.go index 3fb3b424..415a44d0 100644 --- a/tests/tests_test.go +++ b/tests/tests_test.go @@ -38,7 +38,10 @@ func TestAll(t *testing.T) { os.Exit(1) }() - mc, err := mockOpenServer(false, true) + mc, err := mockOpenServer(MockServerOptions{ + Silent: false, + Metrics: true, + }) if err != nil { t.Fatal(err) } @@ -62,6 +65,7 @@ func TestAll(t *testing.T) { runSubTest(t, "info", mc, subTestInfo) runSubTest(t, "timeouts", mc, subTestTimeout) runSubTest(t, "metrics", mc, subTestMetrics) + runSubTest(t, "aof", mc, subTestAOF) } func runSubTest(t *testing.T, name string, mc *mockServer, test func(t *testing.T, mc *mockServer)) { @@ -111,7 +115,9 @@ func BenchmarkAll(b *testing.B) { os.Exit(1) }() - mc, err := mockOpenServer(true, true) + mc, err := mockOpenServer(MockServerOptions{ + Silent: true, Metrics: true, + }) if err != nil { b.Fatal(err) }