From 30427927f6f278d664a766180d8adf8c5e853e7a Mon Sep 17 00:00:00 2001 From: Lonng Date: Mon, 10 Dec 2018 20:53:15 +0800 Subject: [PATCH] *: move `session.NewStore` to `store.New` to achieve semantic accuracy (#8617) --- cmd/benchdb/main.go | 5 +-- session/tidb.go | 48 ----------------------------- session/tidb_test.go | 19 ------------ store/store.go | 72 ++++++++++++++++++++++++++++++++++++++++++++ store/store_test.go | 20 ++++++++++++ tidb-server/main.go | 7 +++-- 6 files changed, 99 insertions(+), 72 deletions(-) create mode 100644 store/store.go diff --git a/cmd/benchdb/main.go b/cmd/benchdb/main.go index e3f4b247705e9..1585cabe8c766 100644 --- a/cmd/benchdb/main.go +++ b/cmd/benchdb/main.go @@ -24,6 +24,7 @@ import ( "github.com/pingcap/parser/terror" "github.com/pingcap/tidb/session" + "github.com/pingcap/tidb/store" "github.com/pingcap/tidb/store/tikv" "github.com/pingcap/tidb/util/logutil" log "github.com/sirupsen/logrus" @@ -58,7 +59,7 @@ func main() { Level: *logLevel, }) terror.MustNil(err) - err = session.RegisterStore("tikv", tikv.Driver{}) + err = store.Register("tikv", tikv.Driver{}) terror.MustNil(err) ut := newBenchDB() works := strings.Split(*runJobs, "|") @@ -94,7 +95,7 @@ type benchDB struct { func newBenchDB() *benchDB { // Create TiKV store and disable GC as we will trigger GC manually. - store, err := session.NewStore("tikv://" + *addr + "?disableGC=true") + store, err := store.New("tikv://" + *addr + "?disableGC=true") terror.MustNil(err) _, err = session.BootstrapSession(store) terror.MustNil(err) diff --git a/session/tidb.go b/session/tidb.go index de9dacbd3a8dd..f94ca2ea76227 100644 --- a/session/tidb.go +++ b/session/tidb.go @@ -19,7 +19,6 @@ package session import ( "context" - "net/url" "strings" "sync" "time" @@ -90,7 +89,6 @@ var ( domap = &domainMap{ domains: map[string]*domain.Domain{}, } - stores = make(map[string]kv.Driver) // store.UUID()-> IfBootstrapped storeBootstrapped = make(map[string]bool) storeBootstrappedLock sync.Mutex @@ -247,52 +245,6 @@ func GetRows4Test(ctx context.Context, sctx sessionctx.Context, rs sqlexec.Recor return rows, nil } -// RegisterStore registers a kv storage with unique name and its associated Driver. -func RegisterStore(name string, driver kv.Driver) error { - name = strings.ToLower(name) - - if _, ok := stores[name]; ok { - return errors.Errorf("%s is already registered", name) - } - - stores[name] = driver - return nil -} - -// NewStore creates a kv Storage with path. -// -// The path must be a URL format 'engine://path?params' like the one for -// session.Open() but with the dbname cut off. -// Examples: -// goleveldb://relative/path -// boltdb:///absolute/path -// -// The engine should be registered before creating storage. -func NewStore(path string) (kv.Storage, error) { - return newStoreWithRetry(path, util.DefaultMaxRetries) -} - -func newStoreWithRetry(path string, maxRetries int) (kv.Storage, error) { - storeURL, err := url.Parse(path) - if err != nil { - return nil, errors.Trace(err) - } - - name := strings.ToLower(storeURL.Scheme) - d, ok := stores[name] - if !ok { - return nil, errors.Errorf("invalid uri format, storage %s is not registered", name) - } - - var s kv.Storage - err = util.RunWithRetry(maxRetries, util.RetryInterval, func() (bool, error) { - log.Infof("new store") - s, err = d.Open(path) - return kv.IsRetryableError(err), err - }) - return s, errors.Trace(err) -} - var queryStmtTable = []string{"explain", "select", "show", "execute", "describe", "desc", "admin"} func trimSQL(sql string) string { diff --git a/session/tidb_test.go b/session/tidb_test.go index e47b935b924a1..8a7baaabeed52 100644 --- a/session/tidb_test.go +++ b/session/tidb_test.go @@ -23,7 +23,6 @@ import ( "time" . "github.com/pingcap/check" - "github.com/pingcap/errors" "github.com/pingcap/parser/auth" "github.com/pingcap/tidb/domain" "github.com/pingcap/tidb/kv" @@ -51,12 +50,6 @@ type testMainSuite struct { dom *domain.Domain } -type brokenStore struct{} - -func (s *brokenStore) Open(schema string) (kv.Storage, error) { - return nil, errors.New("try again later") -} - func (s *testMainSuite) SetUpSuite(c *C) { testleak.BeforeTest() s.dbName = "test_main_db" @@ -111,18 +104,6 @@ func (s *testMainSuite) TestTrimSQL(c *C) { } } -func (s *testMainSuite) TestRetryOpenStore(c *C) { - begin := time.Now() - RegisterStore("dummy", &brokenStore{}) - store, err := newStoreWithRetry("dummy://dummy-store", 3) - if store != nil { - defer store.Close() - } - c.Assert(err, NotNil) - elapse := time.Since(begin) - c.Assert(uint64(elapse), GreaterEqual, uint64(3*time.Second)) -} - func (s *testMainSuite) TestSysSessionPoolGoroutineLeak(c *C) { store, dom := newStoreWithBootstrap(c, s.dbName+"goroutine_leak") defer dom.Close() diff --git a/store/store.go b/store/store.go new file mode 100644 index 0000000000000..b51c10bad92f4 --- /dev/null +++ b/store/store.go @@ -0,0 +1,72 @@ +// Copyright 2018 PingCAP, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// See the License for the specific language governing permissions and +// limitations under the License. + +package store + +import ( + "net/url" + "strings" + + "github.com/pingcap/errors" + "github.com/pingcap/tidb/kv" + "github.com/pingcap/tidb/util" + log "github.com/sirupsen/logrus" +) + +var stores = make(map[string]kv.Driver) + +// Register registers a kv storage with unique name and its associated Driver. +func Register(name string, driver kv.Driver) error { + name = strings.ToLower(name) + + if _, ok := stores[name]; ok { + return errors.Errorf("%s is already registered", name) + } + + stores[name] = driver + return nil +} + +// New creates a kv Storage with path. +// +// The path must be a URL format 'engine://path?params' like the one for +// session.Open() but with the dbname cut off. +// Examples: +// goleveldb://relative/path +// boltdb:///absolute/path +// +// The engine should be registered before creating storage. +func New(path string) (kv.Storage, error) { + return newStoreWithRetry(path, util.DefaultMaxRetries) +} + +func newStoreWithRetry(path string, maxRetries int) (kv.Storage, error) { + storeURL, err := url.Parse(path) + if err != nil { + return nil, errors.Trace(err) + } + + name := strings.ToLower(storeURL.Scheme) + d, ok := stores[name] + if !ok { + return nil, errors.Errorf("invalid uri format, storage %s is not registered", name) + } + + var s kv.Storage + err = util.RunWithRetry(maxRetries, util.RetryInterval, func() (bool, error) { + log.Infof("new store") + s, err = d.Open(path) + return kv.IsRetryableError(err), err + }) + return s, errors.Trace(err) +} diff --git a/store/store_test.go b/store/store_test.go index b5d8283cdb258..cfada40925455 100644 --- a/store/store_test.go +++ b/store/store_test.go @@ -21,8 +21,10 @@ import ( "sync" "sync/atomic" "testing" + "time" . "github.com/pingcap/check" + "github.com/pingcap/errors" "github.com/pingcap/tidb/kv" "github.com/pingcap/tidb/store/mockstore" "github.com/pingcap/tidb/util/logutil" @@ -35,6 +37,12 @@ const ( indexStep = 2 ) +type brokenStore struct{} + +func (s *brokenStore) Open(schema string) (kv.Storage, error) { + return nil, errors.New("try again later") +} + func TestT(t *testing.T) { CustomVerboseFlag = true logLevel := os.Getenv("log_level") @@ -637,3 +645,15 @@ func (s *testKVSuite) TestIsolationMultiInc(c *C) { }) c.Assert(err, IsNil) } + +func (s *testKVSuite) TestRetryOpenStore(c *C) { + begin := time.Now() + Register("dummy", &brokenStore{}) + store, err := newStoreWithRetry("dummy://dummy-store", 3) + if store != nil { + defer store.Close() + } + c.Assert(err, NotNil) + elapse := time.Since(begin) + c.Assert(uint64(elapse), GreaterEqual, uint64(3*time.Second)) +} diff --git a/tidb-server/main.go b/tidb-server/main.go index 0d0b945e9aacd..61d468947c8ff 100644 --- a/tidb-server/main.go +++ b/tidb-server/main.go @@ -40,6 +40,7 @@ import ( "github.com/pingcap/tidb/sessionctx/binloginfo" "github.com/pingcap/tidb/sessionctx/variable" "github.com/pingcap/tidb/statistics" + kvstore "github.com/pingcap/tidb/store" "github.com/pingcap/tidb/store/mockstore" "github.com/pingcap/tidb/store/tikv" "github.com/pingcap/tidb/store/tikv/gcworker" @@ -150,10 +151,10 @@ func main() { } func registerStores() { - err := session.RegisterStore("tikv", tikv.Driver{}) + err := kvstore.Register("tikv", tikv.Driver{}) terror.MustNil(err) tikv.NewGCHandlerFunc = gcworker.NewGCWorker - err = session.RegisterStore("mocktikv", mockstore.MockDriver{}) + err = kvstore.Register("mocktikv", mockstore.MockDriver{}) terror.MustNil(err) } @@ -164,7 +165,7 @@ func registerMetrics() { func createStoreAndDomain() { fullPath := fmt.Sprintf("%s://%s", cfg.Store, cfg.Path) var err error - storage, err = session.NewStore(fullPath) + storage, err = kvstore.New(fullPath) terror.MustNil(err) // Bootstrap a session to load information schema. dom, err = session.BootstrapSession(storage)