*: shard implicit row ID #5513

coocood · 2017-12-27T07:02:25Z

Shard the implicit row ID to avoid hot spot.

winkyao · 2017-12-27T11:13:21Z

ddl/ddl_api.go

@@ -828,8 +828,22 @@ func handleTableOptions(options []*ast.TableOption, tbInfo *model.TableInfo) {
 			tbInfo.Charset = op.StrValue
 		case ast.TableOptionCollate:
 			tbInfo.Collate = op.StrValue
+		case ast.TableOptionShardRowID:
+			if !hasAutoIncrementColumn(tbInfo) {
+				//tbInfo.ShardRowID = op.UintValue != 0


remove this?

winkyao · 2017-12-27T11:51:04Z

executor/show.go

@@ -623,6 +623,10 @@ func (e *ShowExec) fetchShowCreateTable() error {
 		}
 	}

+	if tb.Meta().ShardRowID {
+		buf.WriteString(" SHARD_ROW_ID=1")


if we enable shard_row_id by SHARD_ROW_ID=2?

It's enabled too.

winkyao · 2017-12-27T11:51:51Z

parser/parser.y

@@ -5272,6 +5273,10 @@ TableOption:
 	{
 		$$ = &ast.TableOption{Tp: ast.TableOptionStatsPersistent}
 	}
+|	"SHARD_ROW_ID" EqOpt LengthNum


is EqOpt LengthNum necessary?

So we can disable it after enabled.

coocood · 2018-01-02T09:22:13Z

@ngaut @shenli PTAL

coocood · 2018-01-02T09:24:32Z

@zhangjinpeng1987 PTAL

shenli · 2018-01-02T09:29:22Z

ddl/ddl_api.go

@@ -932,6 +956,36 @@ func (d *ddl) RebaseAutoID(ctx context.Context, ident ast.Ident, newBase int64)
 	return errors.Trace(err)
 }

+func (d *ddl) ShardRowID(ctx context.Context, ident ast.Ident, uVal uint64) error {


add comment for this function

shenli · 2018-01-02T09:29:35Z

ddl/ddl_api.go

+	return errors.Trace(err)
+}
+
+func (d *ddl) createJobByIdent(ident ast.Ident) (*model.Job, error) {


createJobByName is a better name?

ident has both Schema and TableName, it's not a single name.

createJobForTable

createJobForTable is a better name.

shenli · 2018-01-02T09:30:55Z

table/tables/tables.go

+	var buf [8]byte
+	binary.LittleEndian.PutUint64(buf[:], startTS)
+	hashVal := int64(murmur3.Sum32(buf[:]))
+	return (hashVal & (1<<t.meta.ShardRowIDBits - 1)) << (64 - t.meta.ShardRowIDBits - 1)


Could we make the (1<<t.meta.ShardRowIDBits - 1)) << (64 - t.meta.ShardRowIDBits - 1) a const?

No, t.meta.ShardRowIDBits is different for each table.

zimulala · 2018-01-02T10:15:37Z

ddl/ddl_api.go

+	}
+	t, err := is.TableByName(ident.Schema, ident.Name)
+	if err != nil {
+		return nil, errors.Trace(infoschema.ErrTableNotExists.GenByArgs(ident.Schema, ident.Name))


Can we remove this trace ？

zimulala · 2018-01-02T10:35:42Z

ddl/ddl_api.go

+				switch opt.Tp {
+				case ast.TableOptionShardRowID:
+					if opt.UintValue > 15 {
+						opt.UintValue = 15


Can we define a constant as 15? We use it in multiple places.

coocood · 2018-01-03T07:50:46Z

@zimulala @shenli PTAL

zimulala · 2018-01-03T08:37:27Z

executor/ddl_test.go

+		if (h >> 56) > 0 {
+			hasShardedID = true
+		}
+		count++


Can we put this line to line 355？ Then we can make sure all IDs are sharded ID.

No, sharded ID still include shard 0, then (h >> 56) == 0.

zimulala · 2018-01-03T08:40:10Z

executor/ddl_test.go

+	c.Assert(tk.Se.NewTxn(), IsNil)
+	err = tbl.IterRecords(tk.Se, tbl.FirstKey(), nil, func(h int64, rec []types.Datum, cols []*table.Column) (more bool, err error) {
+		c.Assert(h, GreaterEqual, int64(0))
+		if (h >> 56) > 0 {


Why there is 56? Can we use a temporary variable to replace it? Then make it more readable.

coocood · 2018-01-03T11:35:53Z

@shenli @zimulala PTAL

shenli · 2018-01-03T12:48:02Z

ddl/ddl_api.go

+	return errors.Trace(err)
+}
+
+func (d *ddl) createJobByIdent(ident ast.Ident) (*model.Job, error) {


createJobForTable is a better name.

shenli · 2018-01-03T12:49:17Z

meta/autoid/autoid.go

@@ -27,7 +27,7 @@ import (
 )

 // Test needs to change it, so it's a variable.
-var step = int64(5000)
+var step = int64(30000)


We should add a metrics for auto-id allocation latter.

shenli · 2018-01-03T12:51:02Z

executor/show.go

@@ -623,6 +623,10 @@ func (e *ShowExec) fetchShowCreateTable() error {
 		}
 	}

+	if tb.Meta().ShardRowIDBits > 0 {
+		buf.WriteString(fmt.Sprintf(" SHARD_ROW_ID_BITS=%d", tb.Meta().ShardRowIDBits))


We should also use special-comment for show create table. Or users will meet error when migrating data from TiDB to MySQL.

zimulala · 2018-01-03T15:00:46Z

sessionctx/binloginfo/binloginfo.go

+	}
+	upperQuery := strings.ToUpper(ddlQuery)
+	reg, err := regexp.Compile(`SHARD_ROW_ID_BITS\s*=\s*\d+`)
+	terror.Log(err)


If err isn't nil, reg may be nil.

It's just added to pass errcheck, it never returns nil

zimulala · 2018-01-04T02:32:00Z

LGTM

shenli · 2018-01-04T07:17:28Z

LGTM
Please resolve the conflicts.

…w-id Conflicts: ast/ddl.go parser/parser.y

coocood · 2018-01-04T07:22:40Z

/run-all-tests

*: shard implicit row ID

7229ce6

coocood added the DNM label Dec 27, 2017

winkyao reviewed Dec 27, 2017

View reviewed changes

shenli mentioned this pull request Dec 27, 2017

[DNM]*: Add a new syntax to randomize the rowid #5430

Closed

coocood added 4 commits December 29, 2017 11:22

*: change option to shard row ID bits.

2a6a84c

*: use the same shard value for a transaction.

4178cd7

*: increase auto ID step.

2d40fec

*: use hash instead of rand to avoid global lock and add test

09be751

coocood removed the DNM label Jan 2, 2018

shenli reviewed Jan 2, 2018

View reviewed changes

*: add comments

9ba07f9

zimulala reviewed Jan 2, 2018

View reviewed changes

*: address comment

32b42ac

zimulala reviewed Jan 3, 2018

View reviewed changes

*: add special comment for ddl binlog.

a290d98

*: address comment and fix ci

7e4df19

coocood force-pushed the shard-row-id branch from 64dcd06 to 7e4df19 Compare January 3, 2018 11:43

*: check error

0a42ecf

shenli reviewed Jan 3, 2018

View reviewed changes

zimulala reviewed Jan 3, 2018

View reviewed changes

*: address comment

3976397

zimulala added the status/LGT1 Indicates that a PR has LGTM 1. label Jan 4, 2018

coocood added 2 commits January 4, 2018 15:21

Merge commit '01f132ed39a8c1fc9775d963c7b0b50415f7d024' into shard-ro…

2c02909

…w-id Conflicts: ast/ddl.go parser/parser.y

Merge branch 'master' into shard-row-id

b5a5479

coocood added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Jan 4, 2018

shenli approved these changes Jan 4, 2018

View reviewed changes

coocood merged commit 04ef7d7 into pingcap:master Jan 4, 2018

coocood deleted the shard-row-id branch January 4, 2018 08:32

zimulala mentioned this pull request Jan 30, 2018

ddl: fix shard show id bug #5751

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

*: shard implicit row ID #5513

*: shard implicit row ID #5513

coocood commented Dec 27, 2017

winkyao Dec 27, 2017

winkyao Dec 27, 2017

coocood Dec 27, 2017

winkyao Dec 27, 2017

coocood Dec 27, 2017

coocood commented Jan 2, 2018

coocood commented Jan 2, 2018

shenli Jan 2, 2018

shenli Jan 2, 2018

coocood Jan 2, 2018

shenli Jan 3, 2018

shenli Jan 3, 2018

shenli Jan 2, 2018

coocood Jan 2, 2018

zimulala Jan 2, 2018

zimulala Jan 2, 2018

coocood commented Jan 3, 2018

zimulala Jan 3, 2018

coocood Jan 3, 2018

zimulala Jan 3, 2018

coocood commented Jan 3, 2018

shenli Jan 3, 2018

shenli Jan 3, 2018

shenli Jan 3, 2018

zimulala Jan 3, 2018

coocood Jan 3, 2018

zimulala commented Jan 4, 2018

shenli commented Jan 4, 2018

coocood commented Jan 4, 2018

*: shard implicit row ID #5513

*: shard implicit row ID #5513

Conversation

coocood commented Dec 27, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coocood commented Jan 2, 2018

coocood commented Jan 2, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coocood commented Jan 3, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coocood commented Jan 3, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zimulala commented Jan 4, 2018

shenli commented Jan 4, 2018

coocood commented Jan 4, 2018