From 36368d6786aadf321bc384422813bc44122dfd7f Mon Sep 17 00:00:00 2001 From: Philip O'Toole Date: Mon, 21 Nov 2016 23:13:32 -0800 Subject: [PATCH 1/4] Start removing batch load --- http/service.go | 5 +-- http/service_test.go | 2 +- store/store.go | 23 ++++-------- store/store_test.go | 84 +++----------------------------------------- 4 files changed, 12 insertions(+), 102 deletions(-) diff --git a/http/service.go b/http/service.go index b8976991..59380499 100644 --- a/http/service.go +++ b/http/service.go @@ -56,7 +56,7 @@ type Store interface { Backup(leader bool) ([]byte, error) // Load loads a SQLite .dump state from a reader - Load(r io.Reader, sz int) (int64, error) + Load(r io.Reader) (int, error) } // CredentialStore is the interface credential stores must support. @@ -101,9 +101,6 @@ const ( PermStatus = "status" // PermBackup means user can backup node. PermBackup = "backup" - - // LoadBatchSz is the batch size for loading a dump file. - LoadBatchSz = 1000 ) func init() { diff --git a/http/service_test.go b/http/service_test.go index fc1778b4..a729f050 100644 --- a/http/service_test.go +++ b/http/service_test.go @@ -393,7 +393,7 @@ func (m *MockStore) Backup(leader bool) ([]byte, error) { return nil, nil } -func (m *MockStore) Load(r io.Reader, sz int) (int64, error) { +func (m *MockStore) Load(r io.Reader) (int, error) { return 0, nil } diff --git a/store/store.go b/store/store.go index 87b9c648..3c31cd9b 100644 --- a/store/store.go +++ b/store/store.go @@ -442,7 +442,7 @@ func (s *Store) Execute(queries []string, timings, tx bool) ([]*sql.Result, erro } // Load loads a SQLite .dump state from a reader. -func (s *Store) Load(r io.Reader, sz int) (int64, error) { +func (s *Store) Load(r io.Reader) (int, error) { if s.raft.State() != raft.Leader { return 0, ErrNotLeader } @@ -458,15 +458,13 @@ func (s *Store) Load(r io.Reader, sz int) (int64, error) { // Read the dump, executing the commands. var queries []string - var n int64 scanner := parser.NewScanner(r) for { cmd, err := scanner.Scan() if err != nil && err != io.EOF { - return n, err + return len(queries), err } - if cmd == "PRAGMA foreign_keys=OFF" || - cmd == "BEGIN TRANSACTION" || + if cmd == "BEGIN TRANSACTION" || cmd == "COMMIT" { continue } @@ -478,30 +476,21 @@ func (s *Store) Load(r io.Reader, sz int) (int64, error) { } queries = append(queries, cmd) - if len(queries) == sz { - _, err = s.Execute(queries, false, true) - if err != nil { - return n, err - } - n += int64(len(queries)) - queries = nil - } } // Flush residual if len(queries) > 0 { _, err = s.Execute(queries, false, true) if err != nil { - return n, err + return len(queries), err } - n += int64(len(queries)) } // Restore FK constraints to starting state. if err := s.db.EnableFKConstraints(currFK); err != nil { - return n, err + return len(queries), err } - return n, nil + return len(queries), nil } // Backup return a consistent snapshot of the underlying database. diff --git a/store/store_test.go b/store/store_test.go index ac8f39c7..4fca16a3 100644 --- a/store/store_test.go +++ b/store/store_test.go @@ -221,83 +221,7 @@ INSERT INTO "foo" VALUES(1,'fiona'); COMMIT; ` buf := bytes.NewBufferString(dump) - n, err := s.Load(buf, 0) - if err != nil { - t.Fatalf("failed to load dump: %s", err.Error()) - } - if n != 2 { - t.Fatal("wrong number of statements loaded") - } - - // Check that data were loaded correctly. - r, err := s.Query([]string{`SELECT * FROM foo`}, false, true, Strong) - if err != nil { - t.Fatalf("failed to query single node: %s", err.Error()) - } - if exp, got := `["id","name"]`, asJSON(r[0].Columns); exp != got { - t.Fatalf("unexpected results for query\nexp: %s\ngot: %s", exp, got) - } - if exp, got := `[[1,"fiona"]]`, asJSON(r[0].Values); exp != got { - t.Fatalf("unexpected results for query\nexp: %s\ngot: %s", exp, got) - } -} - -func Test_SingleNodeLoadBatch1(t *testing.T) { - s := mustNewStore(true) - defer os.RemoveAll(s.Path()) - - if err := s.Open(true); err != nil { - t.Fatalf("failed to open single-node store: %s", err.Error()) - } - defer s.Close(true) - s.WaitForLeader(10 * time.Second) - - dump := `PRAGMA foreign_keys=OFF; -BEGIN TRANSACTION; -CREATE TABLE foo (id integer not null primary key, name text); -INSERT INTO "foo" VALUES(1,'fiona'); -COMMIT; -` - buf := bytes.NewBufferString(dump) - n, err := s.Load(buf, 1) - if err != nil { - t.Fatalf("failed to load dump: %s", err.Error()) - } - if n != 2 { - t.Fatalf("wrong number of statements loaded, exp %d, got %d", 2, n) - } - - // Check that data were loaded correctly. - r, err := s.Query([]string{`SELECT * FROM foo`}, false, true, Strong) - if err != nil { - t.Fatalf("failed to query single node: %s", err.Error()) - } - if exp, got := `["id","name"]`, asJSON(r[0].Columns); exp != got { - t.Fatalf("unexpected results for query\nexp: %s\ngot: %s", exp, got) - } - if exp, got := `[[1,"fiona"]]`, asJSON(r[0].Values); exp != got { - t.Fatalf("unexpected results for query\nexp: %s\ngot: %s", exp, got) - } -} - -func Test_SingleNodeLoadBatchLarge(t *testing.T) { - s := mustNewStore(true) - defer os.RemoveAll(s.Path()) - - if err := s.Open(true); err != nil { - t.Fatalf("failed to open single-node store: %s", err.Error()) - } - defer s.Close(true) - s.WaitForLeader(10 * time.Second) - - dump := `PRAGMA foreign_keys=OFF; -BEGIN TRANSACTION; -CREATE TABLE foo (id integer not null primary key, name text); -INSERT INTO "foo" VALUES(1,'fiona'); -COMMIT; -` - buf := bytes.NewBufferString(dump) - n, err := s.Load(buf, 100) + n, err := s.Load(buf) if err != nil { t.Fatalf("failed to load dump: %s", err.Error()) } @@ -337,7 +261,7 @@ INSERT INTO "foo" VALUES(1,'fiona'); COMMIT; ` buf := bytes.NewBufferString(dump) - n, err := s.Load(buf, 100) + n, err := s.Load(buf) if err != nil { t.Fatalf("failed to load dump: %s", err.Error()) } @@ -373,7 +297,7 @@ BEGIN TRANSACTION; COMMIT; ` buf := bytes.NewBufferString(dump) - n, err := s.Load(buf, 0) + n, err := s.Load(buf) if err != nil { t.Fatalf("failed to load dump: %s", err.Error()) } @@ -393,7 +317,7 @@ func Test_SingleNodeLoadEmpty(t *testing.T) { s.WaitForLeader(10 * time.Second) buf := bytes.NewBufferString("") - n, err := s.Load(buf, 0) + n, err := s.Load(buf) if err != nil { t.Fatalf("failed to load dump: %s", err.Error()) } From f87ee412e2bfd4c7fd2093d2f985a06425ac0727 Mon Sep 17 00:00:00 2001 From: Philip O'Toole Date: Tue, 22 Nov 2016 19:22:21 -0800 Subject: [PATCH 2/4] Correct unit tests 'PRAGMA foreign_keys=OFF' is no longer filtered. --- store/store_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/store/store_test.go b/store/store_test.go index 4fca16a3..d1a4a9c8 100644 --- a/store/store_test.go +++ b/store/store_test.go @@ -225,7 +225,7 @@ COMMIT; if err != nil { t.Fatalf("failed to load dump: %s", err.Error()) } - if n != 2 { + if n != 3 { t.Fatal("wrong number of statements loaded") } @@ -265,8 +265,8 @@ COMMIT; if err != nil { t.Fatalf("failed to load dump: %s", err.Error()) } - if n != 2 { - t.Fatal("wrong number of statements loaded") + if n != 3 { + t.Fatal("wrong number of statements loaded, exp: 2, got: ", n) } // Check that data were loaded correctly. @@ -301,8 +301,8 @@ COMMIT; if err != nil { t.Fatalf("failed to load dump: %s", err.Error()) } - if n != 0 { - t.Fatal("wrong number of statements loaded") + if n != 1 { + t.Fatal("wrong number of statements loaded, exp: 1, got: ", n) } } From 210367b51b0a9d63cc5a6f8c2c147f5fc2f76d61 Mon Sep 17 00:00:00 2001 From: Philip O'Toole Date: Tue, 22 Nov 2016 19:23:56 -0800 Subject: [PATCH 3/4] Fix call to Load() --- http/service.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/http/service.go b/http/service.go index 59380499..0d95e295 100644 --- a/http/service.go +++ b/http/service.go @@ -382,7 +382,7 @@ func (s *Service) handleLoad(w http.ResponseWriter, r *http.Request) { return } - n, err := s.store.Load(r.Body, LoadBatchSz) + n, err := s.store.Load(r.Body) if err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) return From ed304b78409191137e4c45e61e77273875f2b505 Mon Sep 17 00:00:00 2001 From: Philip O'Toole Date: Tue, 22 Nov 2016 19:29:00 -0800 Subject: [PATCH 4/4] Remove useless comment --- store/store.go | 1 - 1 file changed, 1 deletion(-) diff --git a/store/store.go b/store/store.go index 3c31cd9b..97ac0cc9 100644 --- a/store/store.go +++ b/store/store.go @@ -478,7 +478,6 @@ func (s *Store) Load(r io.Reader) (int, error) { queries = append(queries, cmd) } - // Flush residual if len(queries) > 0 { _, err = s.Execute(queries, false, true) if err != nil {