Repository: pingcap/dumpling
Pull request #336 (2021-08-24 03:34:38)

prepare: filter databases before show table status (#324)
This is an automated cherry-pick of #324

<!--
Thank you for contributing to Dumpling! Please read the [CONTRIBUTING](https://github.com/pingcap/dumpling/blob/master/CONTRIBUTING.md) document **BEFORE** filing this PR.

If you want to open the **Challenge Program** pull request, please use the following template:
https://raw.githubusercontent.com/pingcap/.github/master/.github/PULL_REQUEST_TEMPLATE/challenge-program.md

You can use it with query parameters: https://github.com/pingcap/dumpling/compare/master...${you branch}?template=challenge-program.md
-->

### What problem does this PR solve? <!--add issue link with summary if exists-->
In https://github.com/pingcap/dumpling/pull/305, we have switched the method to get table info to `SHOW TABLE STATUS`. To reduce the access to databases, we'd better filter databases before we executing `SHOW TABLE STATUS`.

### What is changed and how it works?
Filter databases before `SHOW TABLE STATUS` through `conf.Filter`.

### Check List <!--REMOVE the items that are not applicable-->

Tests <!-- At least one of them must be included. -->

 - Unit test
 - Integration test
 - Manual test (add detailed scripts or steps below)
Dump databases and check the log. The databases are filtered before `SHOW TABLE STATUS`.

Related changes

 - Need to cherry-pick to the release branch

### Release note

<!-- bugfixes or new feature need a release note, must in the form of a list, such as

- support -T/--tables-list argument

or if no need to be included in the release note, just add the following line

- No release note
-->
- No release note

ti-chi-bot: @ti-chi-bot: This cherry pick PR is for a release branch and has not yet been approved by release team.
Adding the `do-not-merge/cherry-pick-not-approved` label.

To merge this cherry pick, it must first be approved by the collaborators.

**AFTER** it has been approved by collaborators, please ping the release team in a comment to request a cherry pick review.


<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
</details>
ti-chi-bot: [REVIEW NOTIFICATION]

This pull request has not been approved.


To complete the [pull request process](https://book.prow.tidb.io/#/en/workflows/pr), please ask the reviewers in the [list](https://prow.tidb.io/tichi/repos/pingcap/dumpling/pulls/336/owners) to review by filling `/cc @reviewer` in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the [list](https://prow.tidb.io/tichi/repos/pingcap/dumpling/pulls/336/owners) by filling  `/assign @committer` in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found [here](https://prow.tidb.io/command-help?repo=pingcap%2Fdumpling).

<details>

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.
</details>

<!--Review Notification Identifier-->
lichunzhu: /merge
ti-chi-bot: This pull request has been accepted and is ready to merge. <details>Commit hash: c23069e800ab597449b6b6a6b3ab95ec9b811bed</details>

Patch:
diff --git a/v4/export/block_allow_list.go b/v4/export/block_allow_list.go
index 381bea6..194be0e 100644
--- a/v4/export/block_allow_list.go
+++ b/v4/export/block_allow_list.go
@@ -8,6 +8,23 @@ import (
 	tcontext "github.com/pingcap/dumpling/v4/context"
 )
 
+func filterDatabases(tctx *tcontext.Context, conf *Config, databases []string) []string {
+	tctx.L().Debug("start to filter databases")
+	newDatabases := make([]string, 0, len(databases))
+	ignoreDatabases := make([]string, 0, len(databases))
+	for _, database := range databases {
+		if conf.TableFilter.MatchSchema(database) {
+			newDatabases = append(newDatabases, database)
+		} else {
+			ignoreDatabases = append(ignoreDatabases, database)
+		}
+	}
+	if len(ignoreDatabases) > 0 {
+		tctx.L().Debug("ignore database", zap.Strings("databases", ignoreDatabases))
+	}
+	return newDatabases
+}
+
 func filterTables(tctx *tcontext.Context, conf *Config) {
 	filterTablesFunc(tctx, conf, conf.TableFilter.MatchTable)
 }
diff --git a/v4/export/dump.go b/v4/export/dump.go
index cd9a86b..0f1fb57 100755
--- a/v4/export/dump.go
+++ b/v4/export/dump.go
@@ -852,7 +852,7 @@ func extractTiDBRowIDFromDecodedKey(indexField, key string) (string, error) {
 }
 
 func prepareTableListToDump(tctx *tcontext.Context, conf *Config, db *sql.Conn) error {
-	databases, err := prepareDumpingDatabases(conf, db)
+	databases, err := prepareDumpingDatabases(tctx, conf, db)
 	if err != nil {
 		return err
 	}
diff --git a/v4/export/prepare.go b/v4/export/prepare.go
index e995f8a..2a6d7a1 100644
--- a/v4/export/prepare.go
+++ b/v4/export/prepare.go
@@ -10,6 +10,8 @@ import (
 	"text/template"
 
 	"github.com/pingcap/errors"
+
+	tcontext "github.com/pingcap/dumpling/v4/context"
 )
 
 const (
@@ -75,10 +77,14 @@ func ParseOutputFileTemplate(text string) (*template.Template, error) {
 	return template.Must(DefaultOutputFileTemplate.Clone()).Parse(text)
 }
 
-func prepareDumpingDatabases(conf *Config, db *sql.Conn) ([]string, error) {
+func prepareDumpingDatabases(tctx *tcontext.Context, conf *Config, db *sql.Conn) ([]string, error) {
 	databases, err := ShowDatabases(db)
+	if err != nil {
+		return nil, err
+	}
+	databases = filterDatabases(tctx, conf, databases)
 	if len(conf.Databases) == 0 {
-		return databases, err
+		return databases, nil
 	}
 	dbMap := make(map[string]interface{}, len(databases))
 	for _, database := range databases {


Test patch:
diff --git a/v4/export/block_allow_list_test.go b/v4/export/block_allow_list_test.go
index fddc771..845bcfb 100644
--- a/v4/export/block_allow_list_test.go
+++ b/v4/export/block_allow_list_test.go
@@ -17,6 +17,7 @@ var _ = Suite(&testBWListSuite{})
 type testBWListSuite struct{}
 
 func (s *testBWListSuite) TestFilterTables(c *C) {
+	tctx := tcontext.Background().WithLogger(appLogger)
 	dbTables := DatabaseTables{}
 	expectedDBTables := DatabaseTables{}
 
@@ -36,7 +37,11 @@ func (s *testBWListSuite) TestFilterTables(c *C) {
 		TableFilter: tableFilter,
 	}
 
+	databases := []string{filter.InformationSchemaName, filter.PerformanceSchemaName, "xxx", "yyy"}
+	c.Assert(filterDatabases(tctx, conf, databases), DeepEquals, databases)
+
 	conf.TableFilter = tf.NewSchemasFilter("xxx")
+	c.Assert(filterDatabases(tctx, conf, databases), DeepEquals, []string{"xxx"})
 	filterTables(tcontext.Background(), conf)
 	c.Assert(conf.Tables, HasLen, 1)
 	c.Assert(conf.Tables, DeepEquals, expectedDBTables)
diff --git a/v4/export/prepare_test.go b/v4/export/prepare_test.go
index acf0691..6644eeb 100644
--- a/v4/export/prepare_test.go
+++ b/v4/export/prepare_test.go
@@ -20,7 +20,8 @@ func (s *testPrepareSuite) TestPrepareDumpingDatabases(c *C) {
 	db, mock, err := sqlmock.New()
 	c.Assert(err, IsNil)
 	defer db.Close()
-	conn, err := db.Conn(context.Background())
+	tctx := tcontext.Background().WithLogger(appLogger)
+	conn, err := db.Conn(tctx)
 	c.Assert(err, IsNil)
 
 	rows := sqlmock.NewRows([]string{"Database"}).
@@ -31,7 +32,7 @@ func (s *testPrepareSuite) TestPrepareDumpingDatabases(c *C) {
 	mock.ExpectQuery("SHOW DATABASES").WillReturnRows(rows)
 	conf := defaultConfigForTest(c)
 	conf.Databases = []string{"db1", "db2", "db3"}
-	result, err := prepareDumpingDatabases(conf, conn)
+	result, err := prepareDumpingDatabases(tctx, conf, conn)
 	c.Assert(err, IsNil)
 	c.Assert(result, DeepEquals, []string{"db1", "db2", "db3"})
 
@@ -40,12 +41,12 @@ func (s *testPrepareSuite) TestPrepareDumpingDatabases(c *C) {
 		AddRow("db1").
 		AddRow("db2")
 	mock.ExpectQuery("SHOW DATABASES").WillReturnRows(rows)
-	result, err = prepareDumpingDatabases(conf, conn)
+	result, err = prepareDumpingDatabases(tctx, conf, conn)
 	c.Assert(err, IsNil)
 	c.Assert(result, DeepEquals, []string{"db1", "db2"})
 
 	mock.ExpectQuery("SHOW DATABASES").WillReturnError(fmt.Errorf("err"))
-	_, err = prepareDumpingDatabases(conf, conn)
+	_, err = prepareDumpingDatabases(tctx, conf, conn)
 	c.Assert(err, NotNil)
 
 	rows = sqlmock.NewRows([]string{"Database"}).
@@ -55,7 +56,7 @@ func (s *testPrepareSuite) TestPrepareDumpingDatabases(c *C) {
 		AddRow("db5")
 	mock.ExpectQuery("SHOW DATABASES").WillReturnRows(rows)
 	conf.Databases = []string{"db1", "db2", "db4", "db6"}
-	_, err = prepareDumpingDatabases(conf, conn)
+	_, err = prepareDumpingDatabases(tctx, conf, conn)
 	c.Assert(err, ErrorMatches, `Unknown databases \[db4,db6\]`)
 	c.Assert(mock.ExpectationsWereMet(), IsNil)
 }