Repository: upper/db
Pull request #583 (2020-07-15 00:10:56)

v4: caching improvements


Patch:
diff --git a/.travis.yml b/.travis.yml
index 2326c21..4c0e290 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -8,6 +8,7 @@ env:
   global:
     - GOARCH=amd64
     - DB_HOST=127.0.0.1
+    - UPPER_DB_LOG=ERROR
   matrix:
     - TEST_CMD="make test"
     - TEST_CMD="make benchmark"
@@ -38,4 +39,4 @@ install:
   - go get -v modernc.org/ql/ql
 
 script:
-  - travis_wait 45 bash -c "$TEST_CMD"
+  - bash -c "$TEST_CMD"
diff --git a/Makefile b/Makefile
index e88650e..5d7ed62 100644
--- a/Makefile
+++ b/Makefile
@@ -4,7 +4,7 @@ PARALLEL_FLAGS        ?= --halt-on-error 2 --jobs=4 -v -u
 
 TEST_FLAGS            ?=
 
-UPPER_DB_LOG          = WARN
+UPPER_DB_LOG          ?= WARN
 
 export TEST_FLAGS
 export PARALLEL_FLAGS
diff --git a/adapter/cockroachdb/Makefile b/adapter/cockroachdb/Makefile
index 5bbdd87..9c894bc 100644
--- a/adapter/cockroachdb/Makefile
+++ b/adapter/cockroachdb/Makefile
@@ -1,9 +1,7 @@
 SHELL					        ?= bash
 
-COCKROACHDB_VERSION      ?= v20.1.1
-#COCKROACHDB_VERSION      ?= v19.1.9
-COCKROACHDB_SUPPORTED    ?= $(COCKROACHDB_VERSION)
-#COCKROACHDB_SUPPORTED    ?= v19.1.9 $(COCKROACHDB_VERSION)
+COCKROACHDB_VERSION      ?= v20.1.3
+COCKROACHDB_SUPPORTED    ?= v19.1.9 $(COCKROACHDB_VERSION)
 PROJECT                  ?= upper_cockroachdb_$(COCKROACHDB_VERSION)
 
 DB_HOST               ?= 127.0.0.1
@@ -26,7 +24,7 @@ export DB_USERNAME
 export TEST_FLAGS
 
 test:
-	go test -timeout 30m  -v $(TEST_FLAGS)
+	go test -v $(TEST_FLAGS)
 
 server-up: server-down
 	docker-compose -p $(PROJECT) up -d && \
diff --git a/adapter/cockroachdb/template.go b/adapter/cockroachdb/template.go
index 34016b0..361620f 100644
--- a/adapter/cockroachdb/template.go
+++ b/adapter/cockroachdb/template.go
@@ -153,7 +153,7 @@ const (
   `
 
 	adapterTruncateLayout = `
-    TRUNCATE TABLE {{.Table | compile}}
+		DELETE FROM {{.Table | compile}}
   `
 
 	adapterDropDatabaseLayout = `
diff --git a/internal/sqladapter/collection.go b/internal/sqladapter/collection.go
index ad01db9..22d1e8e 100644
--- a/internal/sqladapter/collection.go
+++ b/internal/sqladapter/collection.go
@@ -76,21 +76,19 @@ type collection struct {
 
 	adapter CollectionAdapter
 
-	pk  []string
 	err error
 }
 
 // NewCollection initializes a Collection by wrapping a CollectionAdapter.
 func NewCollection(sess Session, name string, adapter CollectionAdapter) Collection {
 	if adapter == nil {
-		panic("uppper: received nil adapter")
+		panic("upper: received nil adapter")
 	}
 	c := &collection{
 		sess:    sess,
 		name:    name,
 		adapter: adapter,
 	}
-	c.pk, c.err = c.sess.PrimaryKeys(c.Name())
 	return c
 }
 
@@ -120,13 +118,19 @@ func (c *collection) Insert(item interface{}) (*db.InsertResult, error) {
 }
 
 func (c *collection) PrimaryKeys() []string {
-	return c.pk
+	pk, err := c.sess.PrimaryKeys(c.Name())
+	if err == nil {
+		return pk
+	}
+	c.err = err
+	return nil
 }
 
 func (c *collection) filterConds(conds ...interface{}) []interface{} {
-	if len(conds) == 1 && len(c.pk) == 1 {
+	pk := c.PrimaryKeys()
+	if len(conds) == 1 && len(pk) == 1 {
 		if id := conds[0]; IsKeyValue(id) {
-			conds[0] = db.Cond{c.pk[0]: db.Eq(id)}
+			conds[0] = db.Cond{pk[0]: db.Eq(id)}
 		}
 	}
 	if tr, ok := c.adapter.(condsFilter); ok {
diff --git a/internal/sqladapter/database.go b/internal/sqladapter/database.go
index 2ba17eb..a966c21 100644
--- a/internal/sqladapter/database.go
+++ b/internal/sqladapter/database.go
@@ -21,6 +21,8 @@ var (
 	lastTxID   uint64
 )
 
+var slowQueryThreshold = time.Millisecond * 100
+
 // hasCleanUp is implemented by structs that have a clean up routine that needs
 // to be called before Close().
 type hasCleanUp interface {
@@ -164,6 +166,7 @@ func NewTx(adapter AdapterSession, tx *sql.Tx) (sqlbuilder.Tx, error) {
 		Settings: db.DefaultSettings,
 
 		adapter:           adapter,
+		cachedPKs:         cache.NewCache(),
 		cachedCollections: cache.NewCache(),
 		cachedStatements:  cache.NewCache(),
 	}
@@ -179,6 +182,7 @@ func NewSession(connURL db.ConnectionURL, adapter AdapterSession) Session {
 
 		connURL:           connURL,
 		adapter:           adapter,
+		cachedPKs:         cache.NewCache(),
 		cachedCollections: cache.NewCache(),
 		cachedStatements:  cache.NewCache(),
 	}
@@ -210,6 +214,7 @@ type session struct {
 	txID   uint64
 
 	cacheMu           sync.Mutex // guards cachedStatements and cachedCollections
+	cachedPKs         *cache.Cache
 	cachedStatements  *cache.Cache
 	cachedCollections *cache.Cache
 
@@ -241,7 +246,19 @@ func (sess *session) Err(errIn error) (errOur error) {
 }
 
 func (sess *session) PrimaryKeys(tableName string) ([]string, error) {
-	return sess.adapter.PrimaryKeys(sess, tableName)
+	h := cache.String(tableName)
+	cachedPK, ok := sess.cachedPKs.ReadRaw(h)
+	if ok {
+		return cachedPK.([]string), nil
+	}
+
+	pk, err := sess.adapter.PrimaryKeys(sess, tableName)
+	if err != nil {
+		return nil, err
+	}
+
+	sess.cachedPKs.Write(h, pk)
+	return pk, nil
 }
 
 func (sess *session) TableExists(name string) error {
@@ -437,6 +454,7 @@ func (sess *session) Reset() {
 	sess.cacheMu.Lock()
 	defer sess.cacheMu.Unlock()
 
+	sess.cachedPKs.Clear()
 	sess.cachedCollections.Clear()
 	sess.cachedStatements.Clear()
 
@@ -450,6 +468,7 @@ func (sess *session) NewClone(adapter AdapterSession, checkConn bool) (Session,
 
 	newSess.name = sess.name
 	newSess.sqlDB = sess.sqlDB
+	newSess.cachedPKs = sess.cachedPKs
 
 	if checkConn {
 		if err := newSess.Ping(); err != nil {
@@ -528,7 +547,7 @@ func queryLog(status *QueryStatus) {
 	diff := status.End.Sub(status.Start)
 
 	slowQuery := false
-	if diff >= time.Millisecond*100 {
+	if diff >= slowQueryThreshold {
 		status.Err = db.ErrWarnSlowQuery
 		slowQuery = true
 	}
diff --git a/logger.go b/logger.go
index 0eb0a3c..1696094 100644
--- a/logger.go
+++ b/logger.go
@@ -26,6 +26,7 @@ import (
 	"log"
 	"os"
 	"runtime"
+	"strings"
 )
 
 type LogLevel int8
@@ -51,6 +52,10 @@ var logLevels = map[LogLevel]string{
 	LogLevelPanic: "PANIC",
 }
 
+func (ll LogLevel) String() string {
+	return logLevels[ll]
+}
+
 const (
 	defaultLogLevel LogLevel = LogLevelWarn
 )
@@ -73,6 +78,8 @@ type Logger interface {
 type LoggingCollector interface {
 	Enabled(LogLevel) bool
 
+	Level() LogLevel
+
 	SetLogger(Logger)
 	SetLevel(LogLevel)
 
@@ -127,9 +134,9 @@ func (c *loggingCollector) SetLogger(logger Logger) {
 }
 
 func (c *loggingCollector) logf(level LogLevel, f string, v ...interface{}) {
-	format := logLevels[level] + "\n" + f
+	format := level.String() + "\n" + f
 	if _, file, line, ok := runtime.Caller(2); ok {
-		format = fmt.Sprintf("log_level=%s file=%s:%d\n%s", logLevels[level], file, line, f)
+		format = fmt.Sprintf("log_level=%s file=%s:%d\n%s", level, file, line, f)
 	}
 	format = "upper/db: " + format
 
@@ -145,9 +152,9 @@ func (c *loggingCollector) logf(level LogLevel, f string, v ...interface{}) {
 }
 
 func (c *loggingCollector) log(level LogLevel, v ...interface{}) {
-	format := logLevels[level] + "\n"
+	format := level.String() + "\n"
 	if _, file, line, ok := runtime.Caller(2); ok {
-		format = fmt.Sprintf("log_level=%s file=%s:%d\n", logLevels[level], file, line)
+		format = fmt.Sprintf("log_level=%s file=%s:%d\n", level, file, line)
 	}
 	format = "upper/db: " + format
 	v = append([]interface{}{format}, v...)
@@ -222,11 +229,11 @@ func Log() LoggingCollector {
 }
 
 func init() {
-	if logLevel := os.Getenv("UPPER_DB_LOG"); logLevel != "" {
-		for k := range logLevels {
-			if logLevels[k] == logLevel {
-				Log().SetLevel(k)
-				return
+	if logLevel := strings.ToUpper(os.Getenv("UPPER_DB_LOG")); logLevel != "" {
+		for ll := range logLevels {
+			if ll.String() == logLevel {
+				Log().SetLevel(ll)
+				break
 			}
 		}
 	}


Test patch:
diff --git a/adapter/cockroachdb/helper_test.go b/adapter/cockroachdb/helper_test.go
index 6fb3644..f632e40 100644
--- a/adapter/cockroachdb/helper_test.go
+++ b/adapter/cockroachdb/helper_test.go
@@ -113,31 +113,47 @@ func (h *Helper) TearUp() error {
 		return err
 	}
 
-	batch := []string{
-		`DROP TABLE IF EXISTS artist`,
-		`CREATE TABLE IF NOT EXISTS artist (
+	batches := [][]string{
+		[]string{
+			`DROP TABLE IF EXISTS artist`,
+			`DROP TABLE IF EXISTS publication`,
+			`DROP TABLE IF EXISTS review`,
+			`DROP TABLE IF EXISTS data_types`,
+			`DROP TABLE IF EXISTS stats_test`,
+			`DROP TABLE IF EXISTS composite_keys`,
+			`DROP TABLE IF EXISTS option_types`,
+			`DROP TABLE IF EXISTS pg_types`,
+			`DROP TABLE IF EXISTS issue_370`,
+			`DROP TABLE IF EXISTS varchar_primary_key`,
+			`DROP TABLE IF EXISTS "birthdays"`,
+			`DROP TABLE IF EXISTS "fibonacci"`,
+			`DROP TABLE IF EXISTS "is_even"`,
+			`DROP TABLE IF EXISTS "CaSe_TesT"`,
+			`DROP TABLE IF EXISTS accounts`,
+			`DROP TABLE IF EXISTS users`,
+			`DROP TABLE IF EXISTS logs`,
+			//`DROP TABLE IF EXISTS test_schema.test`,
+			//`DROP SCHEMA IF EXISTS test_schema`,
+			//`DROP TABLE IF EXISTS issue_370_2`,
+		},
+		[]string{
+			`CREATE TABLE IF NOT EXISTS artist (
 			id serial primary key,
 			name varchar(60)
 		)`,
-
-		`DROP TABLE IF EXISTS publication`,
-		`CREATE TABLE IF NOT EXISTS publication (
+			`CREATE TABLE IF NOT EXISTS publication (
 			id serial primary key,
 			title varchar(80),
 			author_id integer
 		)`,
-
-		`DROP TABLE IF EXISTS review`,
-		`CREATE TABLE IF NOT EXISTS review (
+			`CREATE TABLE IF NOT EXISTS review (
 			id serial primary key,
 			publication_id integer,
 			name varchar(80),
 			comments text,
 			created timestamp without time zone
 		)`,
-
-		`DROP TABLE IF EXISTS data_types`,
-		`CREATE TABLE IF NOT EXISTS data_types (
+			`CREATE TABLE IF NOT EXISTS data_types (
 			id serial primary key,
 			_uint integer,
 			_uint8 integer,
@@ -160,38 +176,26 @@ func (h *Helper) TearUp() error {
 			_defaultdate timestamp without time zone DEFAULT now(),
 			_time bigint
 		)`,
-
-		`DROP TABLE IF EXISTS stats_test`,
-		`CREATE TABLE IF NOT EXISTS stats_test (
+			`CREATE TABLE IF NOT EXISTS stats_test (
 			id serial primary key,
 			numeric integer,
 			value integer
 		)`,
-
-		`DROP TABLE IF EXISTS composite_keys`,
-		`CREATE TABLE IF NOT EXISTS composite_keys (
+			`CREATE TABLE IF NOT EXISTS composite_keys (
 			code varchar(255) default '',
 			user_id varchar(255) default '',
 			some_val varchar(255) default '',
 			primary key (code, user_id)
 		)`,
-
-		`DROP TABLE IF EXISTS option_types`,
-		`CREATE TABLE IF NOT EXISTS option_types (
+			`CREATE TABLE IF NOT EXISTS option_types (
 			id serial primary key,
 			name varchar(255) default '',
 			tags varchar(64)[],
 			settings jsonb
 		)`,
-
-		//`DROP TABLE IF EXISTS test_schema.test`,
-		//`DROP SCHEMA IF EXISTS test_schema`,
-
-		//`CREATE SCHEMA test_schema`,
-		//`CREATE TABLE IF NOT EXISTS test_schema.test (id integer)`,
-
-		`DROP TABLE IF EXISTS pg_types`,
-		`CREATE TABLE IF NOT EXISTS pg_types (id serial primary key
+			//`CREATE SCHEMA test_schema`,
+			//`CREATE TABLE IF NOT EXISTS test_schema.test (id integer)`,
+			`CREATE TABLE IF NOT EXISTS pg_types (id serial primary key
 			, uint8_value smallint
 			, uint8_value_array smallint[]
 
@@ -244,81 +248,71 @@ func (h *Helper) TearUp() error {
 			, decimal_value_ptr decimal
 
 		)`,
-
-		`DROP TABLE IF EXISTS issue_370`,
-		`CREATE TABLE IF NOT EXISTS issue_370 (
+			`CREATE TABLE IF NOT EXISTS issue_370 (
 			id UUID PRIMARY KEY,
 			name VARCHAR(25)
 		)`,
-
-		/*
-			`DROP TABLE IF EXISTS issue_370_2`,
-			`CREATE TABLE IF NOT EXISTS issue_370_2 (
-				id INTEGER[3] PRIMARY KEY,
-				name VARCHAR(25)
-			)`,
-		*/
-
-		`DROP TABLE IF EXISTS varchar_primary_key`,
-		`CREATE TABLE IF NOT EXISTS varchar_primary_key (
+			/*
+				`CREATE TABLE IF NOT EXISTS issue_370_2 (
+						id INTEGER[3] PRIMARY KEY,
+						name VARCHAR(25)
+					)`,
+			*/
+			`CREATE TABLE IF NOT EXISTS varchar_primary_key (
 			address VARCHAR(42) PRIMARY KEY NOT NULL,
 			name VARCHAR(25)
 		)`,
-
-		`DROP TABLE IF EXISTS "birthdays"`,
-		`CREATE TABLE IF NOT EXISTS "birthdays" (
+			`CREATE TABLE IF NOT EXISTS "birthdays" (
 			"id" serial primary key,
 			"name" CHARACTER VARYING(50),
 			"born" TIMESTAMP WITH TIME ZONE,
 			"born_ut" INT
 		)`,
-
-		`DROP TABLE IF EXISTS "fibonacci"`,
-		`CREATE TABLE IF NOT EXISTS "fibonacci" (
+			`CREATE TABLE IF NOT EXISTS "fibonacci" (
 			"id" serial primary key,
 			"input" NUMERIC,
 			"output" NUMERIC
 		)`,
-
-		`DROP TABLE IF EXISTS "is_even"`,
-		`CREATE TABLE IF NOT EXISTS "is_even" (
+			`CREATE TABLE IF NOT EXISTS "is_even" (
 			"input" NUMERIC,
 			"is_even" BOOL
 		)`,
-
-		`DROP TABLE IF EXISTS "CaSe_TesT"`,
-		`CREATE TABLE IF NOT EXISTS "CaSe_TesT" (
+			`CREATE TABLE IF NOT EXISTS "CaSe_TesT" (
 			"id" SERIAL PRIMARY KEY,
 			"case_test" VARCHAR(60)
 		)`,
-
-		// bond
-		`DROP TABLE IF EXISTS accounts`,
-		`CREATE TABLE IF NOT EXISTS accounts (
+			`CREATE TABLE IF NOT EXISTS accounts (
 			id serial primary key,
 			name varchar(255),
 			disabled boolean,
 			created_at timestamp with time zone
 		)`,
-
-		`DROP TABLE IF EXISTS users`,
-		`CREATE TABLE IF NOT EXISTS users (
+			`CREATE TABLE IF NOT EXISTS users (
 			id serial primary key,
 			account_id integer,
 			username varchar(255) UNIQUE
 		)`,
-
-		`DROP TABLE IF EXISTS logs`,
-		`CREATE TABLE IF NOT EXISTS logs (
+			`CREATE TABLE IF NOT EXISTS logs (
 			id serial primary key,
 			message VARCHAR
 		)`,
+		},
 	}
 
-	for _, query := range batch {
+	for _, batch := range batches {
 		driver := h.sess.Driver().(*sql.DB)
-		if _, err := driver.Exec(query); err != nil {
-			panic(err.Error())
+		tx, err := driver.Begin()
+		if err != nil {
+			return err
+		}
+
+		for _, query := range batch {
+			if _, err := tx.Exec(query); err != nil {
+				return err
+			}
+		}
+
+		if err := tx.Commit(); err != nil {
 			return err
 		}
 	}
diff --git a/adapter/postgresql/helper_test.go b/adapter/postgresql/helper_test.go
index f2f1418..44e823b 100644
--- a/adapter/postgresql/helper_test.go
+++ b/adapter/postgresql/helper_test.go
@@ -308,12 +308,20 @@ func (h *Helper) TearUp() error {
 		)`,
 	}
 
+	driver := h.sess.Driver().(*sql.DB)
+	tx, err := driver.Begin()
+	if err != nil {
+		return err
+	}
 	for _, query := range batch {
-		driver := h.sess.Driver().(*sql.DB)
-		if _, err := driver.Exec(query); err != nil {
+		if _, err := tx.Exec(query); err != nil {
+			_ = tx.Rollback()
 			return err
 		}
 	}
+	if err := tx.Commit(); err != nil {
+		return err
+	}
 
 	return nil
 }
diff --git a/adapter/ql/helper_test.go b/adapter/ql/helper_test.go
index 2d06514..243a3b8 100644
--- a/adapter/ql/helper_test.go
+++ b/adapter/ql/helper_test.go
@@ -176,7 +176,6 @@ func (h *Helper) TearUp() error {
 			return err
 		}
 		if _, err := tx.Exec(query); err != nil {
-			_ = tx.Rollback()
 			return err
 		}
 		if err := tx.Commit(); err != nil {
diff --git a/internal/testsuite/bond_suite.go b/internal/testsuite/bond_suite.go
index 440d91e..df16b18 100644
--- a/internal/testsuite/bond_suite.go
+++ b/internal/testsuite/bond_suite.go
@@ -120,7 +120,9 @@ func (s *BondTestSuite) BeforeTest(suiteName, testName string) {
 
 	sess := s.Helper.Session().(sqlbuilder.Session)
 
-	cols, _ := sess.Collections()
+	cols, err := sess.Collections()
+	s.NoError(err)
+
 	for i := range cols {
 		err = cols[i].Truncate()
 		s.NoError(err)
@@ -440,7 +442,6 @@ func (s *BondTestSuite) TestInheritedTx() {
 
 func (s *BondTestSuite) TestUnknownCollection() {
 	var err error
-
 	sess := s.Session()
 
 	err = sess.Save(nil)
diff --git a/internal/testsuite/sql_suite.go b/internal/testsuite/sql_suite.go
index 335930c..5cc7a37 100644
--- a/internal/testsuite/sql_suite.go
+++ b/internal/testsuite/sql_suite.go
@@ -112,9 +112,7 @@ func (s *SQLTestSuite) TestPreparedStatementsCache() {
 
 			// This query is different on each iteration and generates a new
 			// prepared statement everytime it's called.
-			res := sess.Collection("artist").
-				Find().
-				Select(db.Raw(fmt.Sprintf("count(%d)", i)))
+			res := sess.Collection("artist").Find().Select(db.Raw(fmt.Sprintf("count(%d)", i)))
 
 			var count map[string]uint64
 			err := res.One(&count)
@@ -190,12 +188,14 @@ func (s *SQLTestSuite) TestTruncateAllCollections() {
 }
 
 func (s *SQLTestSuite) TestQueryLogger() {
+	logLevel := db.Log().Level()
+
 	db.Log().SetLogger(logrus.New())
 	db.Log().SetLevel(db.LogLevelDebug)
 
 	defer func() {
 		db.Log().SetLogger(nil)
-		db.Log().SetLevel(db.LogLevelWarn)
+		db.Log().SetLevel(logLevel)
 	}()
 
 	sess := s.SQLBuilder()
@@ -1772,11 +1772,6 @@ func (s *SQLTestSuite) TestExhaustConnectionPool() {
 
 	sess := s.SQLBuilder()
 
-	db.Log().SetLevel(db.LogLevelDebug)
-	defer func() {
-		db.Log().SetLevel(db.LogLevelWarn)
-	}()
-
 	var wg sync.WaitGroup
 	for i := 0; i < 100; i++ {
 		tLogf("Tx %d: Pending", i)