From 21dff4a68cbbdc9f568a200ada86619379c1c241 Mon Sep 17 00:00:00 2001 From: Philip O'Toole Date: Sat, 8 Jul 2023 09:19:51 -0400 Subject: [PATCH 1/4] Remove on-disk-startup control With the move to WAL and "synchronous mode" to OFF, on-disk startup times are very close to in-memory. There is no need for this control anymore and it complicates the start-up code. --- cmd/rqlited/flags.go | 2 +- cmd/rqlited/main.go | 1 - store/store.go | 74 ++++--------------------------------- store/store_restart_test.go | 15 +------- 4 files changed, 11 insertions(+), 81 deletions(-) diff --git a/cmd/rqlited/flags.go b/cmd/rqlited/flags.go index 0a3142e9..ae0dd00a 100644 --- a/cmd/rqlited/flags.go +++ b/cmd/rqlited/flags.go @@ -476,7 +476,7 @@ func ParseFlags(name, desc string, build *BuildInfo) (*Config, error) { flag.BoolVar(&config.PprofEnabled, "pprof", true, "Serve pprof data on HTTP server") flag.BoolVar(&config.OnDisk, "on-disk", false, "Use an on-disk SQLite database") flag.StringVar(&config.OnDiskPath, "on-disk-path", "", "Path for SQLite on-disk database file. If not set, use a file in data directory") - flag.BoolVar(&config.OnDiskStartup, "on-disk-startup", false, "Do not initialize on-disk database in memory first at startup") + flag.BoolVar(&config.OnDiskStartup, "on-disk-startup", false, "Ignored, on-disk startup optimization control no longer necessary") flag.BoolVar(&config.FKConstraints, "fk", false, "Enable SQLite foreign key constraints") flag.BoolVar(&showVersion, "version", false, "Show version information and exit") flag.BoolVar(&config.RaftNonVoter, "raft-non-voter", false, "Configure as non-voting node") diff --git a/cmd/rqlited/main.go b/cmd/rqlited/main.go index b33e04db..ac5989e4 100644 --- a/cmd/rqlited/main.go +++ b/cmd/rqlited/main.go @@ -304,7 +304,6 @@ func createStore(cfg *Config, ln *tcp.Layer) (*store.Store, error) { }) // Set optional parameters on store. - str.StartupOnDisk = cfg.OnDiskStartup str.SetRequestCompression(cfg.CompressionBatch, cfg.CompressionSize) str.RaftLogLevel = cfg.RaftLogLevel str.NoFreeListSync = cfg.RaftNoFreelistSync diff --git a/store/store.go b/store/store.go index 626a9b52..82913b25 100644 --- a/store/store.go +++ b/store/store.go @@ -203,7 +203,6 @@ type Store struct { observer *raft.Observer onDiskCreated bool // On disk database actually created? - snapsExistOnOpen bool // Any snaps present when store opens? firstIdxOnOpen uint64 // First index on log when Store opens. lastIdxOnOpen uint64 // Last index on log when Store opens. lastCommandIdxOnOpen uint64 // Last command index before applied index when Store opens. @@ -219,14 +218,6 @@ type Store struct { bootstrapped bool notifyingNodes map[string]*Server - // StartupOnDisk disables in-memory initialization of on-disk databases. - // Restarting a node with an on-disk database can be slow so, by default, - // rqlite creates on-disk databases in memory first, and then moves the - // database to disk before Raft starts. However, this optimization can - // prevent nodes with very large (2GB+) databases from starting. This - // flag allows control of the optimization. - StartupOnDisk bool - ShutdownOnRemove bool SnapshotThreshold uint64 SnapshotInterval time.Duration @@ -338,7 +329,6 @@ func (s *Store) Open() (retErr error) { if !s.dbConf.Memory { s.logger.Printf("configured for an on-disk database at %s", s.dbPath) - s.logger.Printf("on-disk database in-memory creation %s", enabledFromBool(!s.StartupOnDisk)) parentDir := filepath.Dir(s.dbPath) s.logger.Printf("ensuring directory for on-disk database exists at %s", parentDir) err := os.MkdirAll(parentDir, 0755) @@ -377,7 +367,6 @@ func (s *Store) Open() (retErr error) { return fmt.Errorf("list snapshots: %s", err) } s.logger.Printf("%d preexisting snapshots present", len(snaps)) - s.snapsExistOnOpen = len(snaps) > 0 // Create the log store and stable store. s.boltStore, err = rlog.New(filepath.Join(s.raftDir, raftDBPath), s.NoFreeListSync) @@ -414,25 +403,19 @@ func (s *Store) Open() (retErr error) { s.logger.Printf("first log index: %d, last log index: %d, last applied index: %d, last command log index: %d:", s.firstIdxOnOpen, s.lastIdxOnOpen, s.lastAppliedIdxOnOpen, s.lastCommandIdxOnOpen) - // If an on-disk database has been requested, and there are no snapshots, and - // there are no commands in the log, then this is the only opportunity to - // create that on-disk database file before Raft initializes. In addition, this - // can also happen if the user explicitly disables the startup optimization of - // building the SQLite database in memory, before switching to disk. - if s.StartupOnDisk || (!s.dbConf.Memory && !s.snapsExistOnOpen && s.lastCommandIdxOnOpen == 0) { + if s.dbConf.Memory { + s.db, err = createInMemory(nil, s.dbConf.FKConstraints) + if err != nil { + return fmt.Errorf("failed to create in-memory database: %s", err) + } + s.logger.Printf("created in-memory database at open") + } else { s.db, err = createOnDisk(nil, s.dbPath, s.dbConf.FKConstraints, !s.dbConf.DisableWAL) if err != nil { return fmt.Errorf("failed to create on-disk database: %s", err) } s.onDiskCreated = true s.logger.Printf("created on-disk database at open") - } else { - // We need an in-memory database, at least for bootstrapping purposes. - s.db, err = createInMemory(nil, s.dbConf.FKConstraints) - if err != nil { - return fmt.Errorf("failed to create in-memory database: %s", err) - } - s.logger.Printf("created in-memory database at open") } // Instantiate the Raft system. @@ -910,7 +893,6 @@ func (s *Store) Stats() (map[string]interface{}, error) { "observed": s.observer.GetNumObserved(), "dropped": s.observer.GetNumDropped(), }, - "startup_on_disk": s.StartupOnDisk, "apply_timeout": s.ApplyTimeout.String(), "heartbeat_timeout": s.HeartbeatTimeout.String(), "election_timeout": s.ElectionTimeout.String(), @@ -1508,36 +1490,6 @@ func (s *Store) Apply(l *raft.Log) (e interface{}) { if l.Index == s.lastCommandIdxOnOpen { s.logger.Printf("%d confirmed committed log entries applied in %s, took %s since open", s.appliedOnOpen, time.Since(s.firstLogAppliedT), time.Since(s.openT)) - - // Last command log applied. Time to switch to on-disk database? - if s.dbConf.Memory { - s.logger.Println("continuing use of in-memory database") - } else if s.onDiskCreated { - s.logger.Println("continuing use of on-disk database") - } else { - // Since we're here, it means that a) an on-disk database was requested, - // b) in-memory creation of the on-disk database is enabled, and c) there - // were commands in the log. A snapshot may or may not have been applied, - // but it wouldn't have created the on-disk database in that case since - // there were commands in the log. This is the very last chance to convert - // from in-memory to on-disk. - s.queryTxMu.Lock() - defer s.queryTxMu.Unlock() - b, _ := s.db.Serialize() - err := s.db.Close() - if err != nil { - e = &fsmGenericResponse{error: fmt.Errorf("close failed: %s", err)} - return - } - // Open a new on-disk database. - s.db, err = createOnDisk(b, s.dbPath, s.dbConf.FKConstraints, !s.dbConf.DisableWAL) - if err != nil { - e = &fsmGenericResponse{error: fmt.Errorf("open on-disk failed: %s", err)} - return - } - s.onDiskCreated = true - s.logger.Println("successfully switched to on-disk database") - } } } }() @@ -1614,12 +1566,7 @@ func (s *Store) Restore(rc io.ReadCloser) error { } var db *sql.DB - if s.StartupOnDisk || (!s.dbConf.Memory && s.lastCommandIdxOnOpen == 0) { - // A snapshot clearly exists (this function has been called) but there - // are no command entries in the log -- so Apply will not be called. - // Therefore, this is the last opportunity to create the on-disk database - // before Raft starts. This could also happen because the user has explicitly - // disabled the build-on-disk-database-in-memory-first optimization. + if !s.dbConf.Memory { db, err = createOnDisk(b, s.dbPath, s.dbConf.FKConstraints, !s.dbConf.DisableWAL) if err != nil { return fmt.Errorf("open on-disk file during restore: %s", err) @@ -1627,11 +1574,6 @@ func (s *Store) Restore(rc io.ReadCloser) error { s.onDiskCreated = true s.logger.Println("successfully switched to on-disk database due to restore") } else { - // Deserialize into an in-memory database because a) an in-memory database - // has been requested, or b) while there was a snapshot, there are also - // command entries in the log. So by sticking with an in-memory database - // those entries will be applied in the fastest possible manner. We will - // defer creation of any database on disk until the Apply function. db, err = createInMemory(b, s.dbConf.FKConstraints) if err != nil { return fmt.Errorf("createInMemory: %s", err) diff --git a/store/store_restart_test.go b/store/store_restart_test.go index fa0885f8..450f9f3a 100644 --- a/store/store_restart_test.go +++ b/store/store_restart_test.go @@ -206,21 +206,10 @@ func openStoreCloseStartup(t *testing.T, s *Store) { } } -// Test_OpenStoreCloseStartupOnDiskSingleNode tests that the on-disk -// optimization can be disabled in various scenarios. +// Test_OpenStoreCloseStartupOnDiskSingleNode tests that on-disk +// works fine during various restart scenarios. func Test_OpenStoreCloseStartupOnDiskSingleNode(t *testing.T) { s, ln := mustNewStore(t, false) - s.StartupOnDisk = true - defer ln.Close() - - openStoreCloseStartup(t, s) -} - -// Test_OpenStoreCloseStartupMemorySingleNode tests that the on-disk -// optimization works fine. -func Test_OpenStoreCloseStartupMemorySingleNode(t *testing.T) { - s, ln := mustNewStore(t, false) - s.StartupOnDisk = false defer ln.Close() openStoreCloseStartup(t, s) From f81aad8fd34b2c6f7716722c4c4f391e1118c60a Mon Sep 17 00:00:00 2001 From: Philip O'Toole Date: Sat, 8 Jul 2023 09:22:01 -0400 Subject: [PATCH 2/4] Revert logic for clarity --- store/store.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/store/store.go b/store/store.go index 82913b25..1cbf4116 100644 --- a/store/store.go +++ b/store/store.go @@ -1566,18 +1566,18 @@ func (s *Store) Restore(rc io.ReadCloser) error { } var db *sql.DB - if !s.dbConf.Memory { + if s.dbConf.Memory { + db, err = createInMemory(b, s.dbConf.FKConstraints) + if err != nil { + return fmt.Errorf("createInMemory: %s", err) + } + } else { db, err = createOnDisk(b, s.dbPath, s.dbConf.FKConstraints, !s.dbConf.DisableWAL) if err != nil { return fmt.Errorf("open on-disk file during restore: %s", err) } s.onDiskCreated = true s.logger.Println("successfully switched to on-disk database due to restore") - } else { - db, err = createInMemory(b, s.dbConf.FKConstraints) - if err != nil { - return fmt.Errorf("createInMemory: %s", err) - } } s.db = db From a40c43725f6f60a79d95a4641910f4072d316952 Mon Sep 17 00:00:00 2001 From: Philip O'Toole Date: Sat, 8 Jul 2023 09:25:35 -0400 Subject: [PATCH 3/4] More logic reversal --- store/store.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/store/store.go b/store/store.go index 1cbf4116..ad5dfd47 100644 --- a/store/store.go +++ b/store/store.go @@ -327,7 +327,9 @@ func (s *Store) Open() (retErr error) { s.openT = time.Now() s.logger.Printf("opening store with node ID %s", s.raftID) - if !s.dbConf.Memory { + if s.dbConf.Memory { + s.logger.Printf("configured for an in-memory database") + } else { s.logger.Printf("configured for an on-disk database at %s", s.dbPath) parentDir := filepath.Dir(s.dbPath) s.logger.Printf("ensuring directory for on-disk database exists at %s", parentDir) @@ -335,8 +337,6 @@ func (s *Store) Open() (retErr error) { if err != nil { return err } - } else { - s.logger.Printf("configured for an in-memory database") } // Create all the required Raft directories. From 007c81ae8acbcc4aa51a8567a720db1eb9d59ad3 Mon Sep 17 00:00:00 2001 From: Philip O'Toole Date: Sat, 8 Jul 2023 09:33:16 -0400 Subject: [PATCH 4/4] CHANGELOG --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5a39faea..b5348e1f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +## 7.21.4 (unreleased) +### Implementation changes and bug fixes +- [PR #1336](https://github.com/rqlite/rqlite/pull/1336): Remove on-disk-startup control. + ## 7.21.3 (July 7th 2023) ### Implementation changes and bug fixes - [PR #1329](https://github.com/rqlite/rqlite/pull/1329): Try a different version of V2 Snapshot codec.