Skip to content
This repository has been archived by the owner on Jul 24, 2024. It is now read-only.

Support backup & restore views and sequences #242

Merged
merged 15 commits into from
May 25, 2020

Conversation

kennytm
Copy link
Collaborator

@kennytm kennytm commented Apr 22, 2020

What problem does this PR solve?

Fix #145, and also support backup/restore sequences.

What is changed and how it works?

Thanks to #216, views and sequences can be naturally restored.

The only problems left are

  1. Views don't have auto IDs. This PR skips the auto-ID rebase when the table is a view.
  2. Value of sequence needs to be backed up. This PR stores the current sequence value into the AutoIncID field, and performs setval() on restore.

Check List

Tests

  • Integration test

Code changes

Side effects

Related changes

  • Need to be included in the release note

    • BR now supports views and sequences.

@codecov
Copy link

codecov bot commented Apr 23, 2020

Codecov Report

Merging #242 into master will decrease coverage by 2.33%.
The diff coverage is 87.80%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #242      +/-   ##
==========================================
- Coverage   74.27%   71.94%   -2.34%     
==========================================
  Files          49       48       -1     
  Lines        5890     5054     -836     
==========================================
- Hits         4375     3636     -739     
+ Misses       1022      959      -63     
+ Partials      493      459      -34     
Impacted Files Coverage Δ
pkg/restore/db.go 59.34% <84.84%> (-2.98%) ⬇️
pkg/backup/client.go 72.50% <100.00%> (-7.90%) ⬇️
pkg/task/restore.go 54.07% <0.00%> (-9.11%) ⬇️
pkg/restore/client.go 70.21% <0.00%> (-6.81%) ⬇️
pkg/restore/range.go 79.48% <0.00%> (-3.13%) ⬇️
pkg/restore/util.go 73.38% <0.00%> (-3.10%) ⬇️
pkg/restore/pipeline_items.go
pkg/restore/split_client.go 61.13% <0.00%> (+0.37%) ⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d44f5b1...07778c7. Read the comment docs.

@kennytm
Copy link
Collaborator Author

kennytm commented Apr 23, 2020

Sequences actually have 2 associated metadata: the sequence value (SID:n) and sequence cycle (SequenceCycle:n). There is no exposed SQL to configure the latter value. Is it fine if the number of rounds is wrong (zero) for the cyclic sequence when restored? cc @AilinKid

@AilinKid
Copy link

AilinKid commented May 20, 2020

Sequences actually have 2 associated metadata: the sequence value (SID:n) and sequence cycle (SequenceCycle:n). There is no exposed SQL to configure the latter value. Is it fine if the number of rounds is wrong (zero) for the cyclic sequence when restored? cc @AilinKid

To address this problem, the cycle value will influence the nextval computation formula.

there some way manually
if a sequence isn't cycle-type or it hasn't been in the cycle, it is ok to let it be 0.
if a cycle-typed sequence is in the cycle, you can just setval(MAX_VALUE) / setval(MIN_VALUE) if negative, then use nextval(seq) to trigger cycle, then setval(target_num) to set value in cycle round.

now we only just cycle>0 to know it's in cycle, so there is no difference in 1,2,3... if you don't care the real cycle round times.

@3pointer
Copy link
Collaborator

Sequences actually have 2 associated metadata: the sequence value (SID:n) and sequence cycle (SequenceCycle:n). There is no exposed SQL to configure the latter value. Is it fine if the number of rounds is wrong (zero) for the cyclic sequence when restored? cc @AilinKid

To address this problem, the cycle value will influence the nextval computation formula.

there some way manually
if a sequence isn't cycle-type or it hasn't been in the cycle, it is ok to let it be 0.
if a cycle-typed sequence is in the cycle, you can just setval(MAX_VALUE) / setval(MIN_VALUE) if negative, then use nextval(seq) to trigger cycle, then setval(target_num) to set value in cycle round.

now we only just cycle>0 to know it's in cycle, so there is no difference in 1,2,3... if you don't care the real cycle round times.

I got two questions for this.

  1. how to know this sequence value is in cycle round > 0 when backup?

  2. I feel like the nextval of this sequence is same no matter how many rounds in cycle, did I miss something?

@kennytm
Copy link
Collaborator Author

kennytm commented May 21, 2020

After discussion with the designers of the sequence function, we conclude that

  • TiDB needs to add two internal APIs for getting and setting the cycle value.

  • TiDB's result of nextval() after setval() depends on the cycle count, which is observable with this:

    create sequence seq start 2 minvalue 1 maxvalue 999999 increment 2 cycle;
    do setval(seq, 10);
    select nextval(seq);

    In the first cycle, the "base" of the sequence is start, so nextval() will always return an even number.

    In the future cycles, the "base" of the sequence is minvalue, so nextval() will always return an odd number.


setval() is not an SQL Standard function. (ISO/IEC 9075-2 only specified ALTER SEQUENCE seq RESTART WITH 12345; with DDL behavior and is blocking, but setval() is not blocking). Thus, the behavior of setval() is incompatible between different SQL dialects.

System setval behavior after cycle get cycle count set cycle count
TiDB Success; rebase on minvalue rather than start unimplemented unimplemented
MariaDB Becomes no-op, unless explicitly setting the round argument to the cycle count: setval(seq, 12345, true, 67) select cycle_count from seq; do setval(seq, value, true, @cycle_count)
PostgreSQL No concept of cycle count, and setval will succeed even if the value is decreased N/A N/A
MS SQL Server / IBM Informix / SQL standard No setval() function; ALTER SEQUENCE RESTART WITH is supported and can decrease values N/A N/A
Oracle Need to drop and create the sequence to change its value. Boo. N/A N/A

The TiDB behavior is designed to keep the same pace among all nodes within the same cluster. In any case, both MariaDB and TiDB's setval disallow decreasing the sequence number, and thus the cycle count must be recorded.

@3pointer
Copy link
Collaborator

/run-all-tests

setValFormat := fmt.Sprintf("do setval(%s.%s, %%d);",
utils.EncloseName(table.Db.Name.O),
utils.EncloseName(table.Info.Name.O))
if table.Info.Sequence.Cycle {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Collaborator

@3pointer 3pointer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@3pointer
Copy link
Collaborator

manual test success.

create sequence seq start 2 minvalue 1 maxvalue 999999 increment 2 cycle;
do setval(seq, 10);
select nextval(seq);

before backup we got sequence with even number:

mysql> select nextval(seq);
+--------------+
| nextval(seq) |
+--------------+
|           12 |
+--------------+

after restore we got sequence with odd number:

mysql> select nextval(seq);
+--------------+
| nextval(seq) |
+--------------+
|         2013 |
+--------------+

Copy link
Member

@overvenus overvenus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kennytm kennytm added status/LGT2 LGTM2 and removed status/LGT1 LGTM1 labels May 25, 2020
@kennytm
Copy link
Collaborator Author

kennytm commented May 25, 2020

/merge

@sre-bot
Copy link
Contributor

sre-bot commented May 25, 2020

/run-all-tests

@sre-bot sre-bot merged commit b25f71f into pingcap:master May 25, 2020
@sre-bot
Copy link
Contributor

sre-bot commented May 25, 2020

cherry pick to release-4.0 failed

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support backup/restore VIEW
5 participants