*: implement the sequence allocator logic. #14829

AilinKid · 2020-02-18T07:01:14Z

What problem does this PR solve?

implement the sequence allocator logic
this PR implements the complete functionality of TiDB sequence.
For example:

create sequence seq start 1 increment -2 minvalue -10 maxvalue 10 cache 3 cycle.
select nextval(seq) will get: 
[1, -1, -3]   [-5, -7, -9]   [10, 8, 6] ...

setval(seq, num) will change the sequence value to a certain position.
1: num previous to now base, return NULL,
2: return the num, and rebase the cache(in cache) / rebase the meta (invalid the cache).

`lastval(seq)` will show the last allocated value in your session. 
1: if `nextval` hasn't been called, return NULL, 
2: otherwise, return the last value.

What is changed and how it works?

1： do sequence cache in tables, a sequence table can be easily notified with cache empty and write the binlog.
2: implement the sequence positive-growth & negative-growth allocation under MIN, MAX limitation.
3: add the allocSeqCache interface to allocator, cause it differs from autoid a lot.
4: add some tests at SQL and KV level.

Check List

Tests

Unit test
Integration test

Code changes

Has interface methods change

Related changes

Need to update the documentation

Release note

implement the sequence allocator logic.

table/column.go

Deardrops · 2020-02-18T08:34:17Z

meta/autoid/autoid.go

+	startTime := time.Now()
+	err := kv.RunInNewTxn(alloc.store, true, func(txn kv.Transaction) error {
+		m := meta.NewMeta(txn)
+		currentEnd, err1 := getAutoIDByAllocType(m, alloc.dbID, tableID, alloc.allocType)


The variable name err is ok at here.

meta/autoid/autoid.go

Deardrops · 2020-02-18T08:47:22Z

meta/autoid/autoid.go

+	return m.SetSequenceCycle(dbID, tableID, round)
+}
+
+// getSequenceCycleRound is used to get whether the sequence is already in cycle.


Suggested change

// getSequenceCycleRound is used to get whether the sequence is already in cycle.

// getSequenceCycleRound is used to get the flag `round`, which indicates whether the sequence is already in cycle.

crazycs520 · 2020-02-18T11:05:36Z

meta/autoid/autoid_test.go

+			Name:     model.NewCIStr("seq"),
+			Sequence: seq,
+		}
+		if seq.Increment >= 0 {


seq.Increment >= 0 is always true?

In this test, it is. addressed.

crazycs520

Is this expect?

AilinKid · 2020-02-18T12:35:14Z

Is this expect?

yes, in TiDB set value will not change the next valid value.
Mariadb: next value = set value + increment
TiDB: next value is still in the formula: offset + increment * n, we ganna find the next valid value.

AilinKid · 2020-02-19T10:54:20Z

/run-unit-test

meta/autoid/autoid.go

table/tables/tables.go

Deardrops · 2020-02-21T12:25:33Z

table/tables/tables.go

+	// base < end when increment > 0.
+	// base > end when increment < 0.
+	end  int64
+	base int64


How about naming them low and high? high is always greater than low.

not readlly, if use low/high, that will mean low < high when increment > 0, but low > high when increment < 0, which is weird.

meta/autoid/autoid.go

meta/autoid/autoid_test.go

Deardrops · 2020-02-21T12:44:49Z

meta/autoid/autoid_test.go

+		wg.Add(1)
+		go func(num int) {
+			defer wg.Done()
+			time.Sleep(time.Duration(num%10) * time.Microsecond)


Could we remove sleep here?

This tried to simulate to allocate sequence cache at different start-time currently.
The sleep duration is small with a few Microsecond, won't postpone the test time.

meta/autoid/autoid_test.go

Deardrops · 2020-02-21T12:50:36Z

meta/autoid/autoid_test.go

+				if _, ok := m[i]; ok {
+					errCh <- fmt.Errorf("duplicate id:%v", i)
+					errFlag = true
+					mu.Unlock()


Duplicated unlock here?

Not readlly, errFlag = true will break twice, which will skip the normal unlock.

Deardrops · 2020-02-21T12:59:55Z

ddl/sequence_test.go

+	s.tk.MustExec("select setval(seq, 100)")
+	s.tk.MustExec("insert into t values(lastval(seq)),(-1),(nextval(seq))")
+	s.tk.MustQuery("select * from t").Check(testkit.Rows("14", "-1", "101"))
+}


Add some test for using nextval(), setval(), lastval() in generated columns' expression.

We store sequence value by int64. Could you add some tests for integer overflow?

nice suggestion.

AilinKid · 2020-02-25T09:17:49Z

/build

AilinKid · 2020-02-25T11:24:05Z

/run-all-tests

table/tables/tables.go

AilinKid · 2020-02-26T05:18:03Z

/run-all-tests

AilinKid · 2020-02-26T05:26:33Z

/run-unit-test

tangenta

LGTM

AilinKid · 2020-02-26T07:33:32Z

/merge

sre-bot · 2020-02-26T07:33:38Z

/run-all-tests

sre-bot · 2020-02-26T07:36:39Z

@AilinKid merge failed.

AilinKid · 2020-02-26T07:38:19Z

/build

AilinKid · 2020-02-26T07:41:38Z

/merge

sre-bot · 2020-02-26T07:42:08Z

/run-all-tests

AilinKid added 5 commits February 18, 2020 14:41

implement the sequence allocator

16e7ab5

check dev

9302c9d

.

a99f318

change the sequence run out error

f905d31

new allocator few parameter

2bb466f

AilinKid requested review from tangenta, crazycs520 and zimulala February 18, 2020 07:28

AilinKid requested a review from bb7133 February 18, 2020 07:30

AilinKid added require-LGT3 Indicates that the PR requires three LGTM. type/new-feature labels Feb 18, 2020

Deardrops reviewed Feb 18, 2020

View reviewed changes

crazycs520 reviewed Feb 18, 2020

View reviewed changes

AilinKid and others added 2 commits February 19, 2020 10:45

address comment

13be98a

Merge branch 'master' into seq_alloc

1544c36

Merge branch 'master' into seq_alloc

78d3bd9

crazycs520 reviewed Feb 19, 2020

View reviewed changes

meta/autoid/autoid.go Outdated Show resolved Hide resolved

meta/autoid/autoid.go Outdated Show resolved Hide resolved

crazycs520 reviewed Feb 19, 2020

View reviewed changes

meta/autoid/autoid.go Outdated Show resolved Hide resolved

AilinKid and others added 2 commits February 21, 2020 10:48

address comment

5caaa4d

Merge branch 'master' into seq_alloc

9547089

AilinKid removed the require-LGT3 Indicates that the PR requires three LGTM. label Feb 21, 2020

AilinKid added this to the v4.0.0-beta.1 milestone Feb 21, 2020

Deardrops self-requested a review February 21, 2020 04:00

Deardrops reviewed Feb 21, 2020

View reviewed changes

AilinKid mentioned this pull request Feb 24, 2020

sql: add sequence doc pingcap/docs-cn#2282

Merged

1 task

use bigInt to compute sequence seek formula to avoid overflow

d6b538d

AilinKid added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Feb 25, 2020

address comment

5697d3f

tangenta reviewed Feb 26, 2020

View reviewed changes

table/tables/tables.go Show resolved Hide resolved

AilinKid and others added 4 commits February 26, 2020 10:03

address comment

7da16c1

make seek fast

4aaa222

sequence increment shouldn't set as 0

c5f3d19

Merge branch 'master' into seq_alloc

654a5e5

AilinKid added the status/all tests passed label Feb 26, 2020

tangenta approved these changes Feb 26, 2020

View reviewed changes

tangenta added status/LGT3 The PR has already had 3 LGTM. and removed status/LGT2 Indicates that a PR has LGTM 2. labels Feb 26, 2020

Merge branch 'master' into seq_alloc

aadafd8

sre-bot added the status/can-merge Indicates a PR has been approved by a committer. label Feb 26, 2020

sre-bot merged commit 0bbf1d9 into pingcap:master Feb 26, 2020

AilinKid mentioned this pull request Mar 5, 2020

sql: fix sequence doc pingcap/docs-cn#2380

Merged

4 tasks

lysu mentioned this pull request Mar 18, 2020

SELECT nextval reset after kill tidb #15441

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

*: implement the sequence allocator logic. #14829

*: implement the sequence allocator logic. #14829

AilinKid commented Feb 18, 2020

Deardrops Feb 18, 2020

AilinKid Feb 19, 2020

Deardrops Feb 18, 2020

crazycs520 Feb 18, 2020

AilinKid Feb 19, 2020

crazycs520 left a comment

AilinKid commented Feb 18, 2020

AilinKid commented Feb 19, 2020

Deardrops Feb 21, 2020

AilinKid Feb 24, 2020

Deardrops Feb 21, 2020

AilinKid Feb 24, 2020

Deardrops Feb 21, 2020

AilinKid Feb 24, 2020 •

edited

Loading

Deardrops Feb 21, 2020

Deardrops Feb 21, 2020

AilinKid Feb 24, 2020

AilinKid commented Feb 25, 2020

AilinKid commented Feb 25, 2020

AilinKid commented Feb 26, 2020

AilinKid commented Feb 26, 2020

tangenta left a comment

AilinKid commented Feb 26, 2020

sre-bot commented Feb 26, 2020

sre-bot commented Feb 26, 2020

AilinKid commented Feb 26, 2020

AilinKid commented Feb 26, 2020

sre-bot commented Feb 26, 2020

	// getSequenceCycleRound is used to get whether the sequence is already in cycle.
	// getSequenceCycleRound is used to get the flag `round`, which indicates whether the sequence is already in cycle.

*: implement the sequence allocator logic. #14829

*: implement the sequence allocator logic. #14829

Conversation

AilinKid commented Feb 18, 2020

What problem does this PR solve?

What is changed and how it works?

Check List

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

crazycs520 left a comment

Choose a reason for hiding this comment

AilinKid commented Feb 18, 2020

AilinKid commented Feb 19, 2020

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

AilinKid Feb 24, 2020 • edited Loading

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

AilinKid commented Feb 25, 2020

AilinKid commented Feb 25, 2020

AilinKid commented Feb 26, 2020

AilinKid commented Feb 26, 2020

tangenta left a comment

Choose a reason for hiding this comment

AilinKid commented Feb 26, 2020

sre-bot commented Feb 26, 2020

sre-bot commented Feb 26, 2020

AilinKid commented Feb 26, 2020

AilinKid commented Feb 26, 2020

sre-bot commented Feb 26, 2020

AilinKid Feb 24, 2020 •

edited

Loading