Skip to content

Commit

Permalink
*: move session.NewStore to store.New to achieve semantic accuracy (
Browse files Browse the repository at this point in the history
  • Loading branch information
lonng authored and ngaut committed Dec 10, 2018
1 parent 18b33fb commit 3042792
Show file tree
Hide file tree
Showing 6 changed files with 99 additions and 72 deletions.
5 changes: 3 additions & 2 deletions cmd/benchdb/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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, "|")
Expand Down Expand Up @@ -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)
Expand Down
48 changes: 0 additions & 48 deletions session/tidb.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package session

import (
"context"
"net/url"
"strings"
"sync"
"time"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
19 changes: 0 additions & 19 deletions session/tidb_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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()
Expand Down
72 changes: 72 additions & 0 deletions store/store.go
Original file line number Diff line number Diff line change
@@ -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)
}
20 changes: 20 additions & 0 deletions store/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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")
Expand Down Expand Up @@ -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))
}
7 changes: 4 additions & 3 deletions tidb-server/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}

Expand All @@ -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)
Expand Down

0 comments on commit 3042792

Please sign in to comment.