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

fix restore auto increment ID overflow - #458

Open
3pointer wants to merge 13 commits into
pingcap:masterfrom
3pointer:fix_restore_overflow
Open

fix restore auto increment ID overflow#458
3pointer wants to merge 13 commits into
pingcap:masterfrom
3pointer:fix_restore_overflow

Conversation

@3pointer

Copy link
Copy Markdown
Collaborator

What problem does this PR solve?

when auto_increment id reached max.Int64, it can backs up success. but failed when restore. we should handle this situation.

What is changed and how it works?

if autoIncID overflowed. change is to max.Int64, to keep consistency with backup cluster.

Check List

Tests

  • Integration test

Code changes

  • Has exported function/method change
  • Has exported variable/fields change
  • Has interface methods change
  • Has persistent data change

Side effects

  • Possible performance regression
  • Increased code complexity
  • Breaking backward compatibility

Related changes

  • Need to cherry-pick to the release branch

Release Note

  • No release note

Comment threadpkg/restore/db.go Outdated
// auto inc id overflow
autoIncID := table.Info.AutoIncID
if table.Info.AutoIncID < 0 {
autoIncID = math.MaxInt64

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

what about bigint unsigned columns?

createtablea (id bigint unsigned primary key auto_increment);
insert into a values (12345678901234567890);

@3pointer3pointerAug 14, 2020

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

I was wondering that the allocator can work well 🤔️,it set the allocator with unsigned=false https://github.com/pingcap/br/blob/master/pkg/backup/client.go#L261

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

@kennytmkennytmAug 16, 2020

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm not sure what's the purpose of subtracting 1 😅

Comment threadpkg/restore/db.go Outdated
Comment threadpkg/restore/db.go Outdated
@overvenus

Copy link
Copy Markdown
Member

Any update?

@3pointer

Copy link
Copy Markdown
CollaboratorAuthor

/run-integration-tests

@3pointer
3pointerforce-pushed the fix_restore_overflow branch from 37ba16c to 36e8045CompareMarch 28, 2021 07:24

@kennytmkennytm left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

rest LGTM

Comment threadpkg/restore/db.go
Comment on lines +178 to 181
autoIncID = uint64(table.Info.AutoIncID)
} else {
setValSQL = fmt.Sprintf(setValFormat, table.Info.Sequence.MaxValue)
autoIncID = uint64(table.Info.AutoIncID)
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

the two branches are the same? so this is simply

 }
autoIncID=uint64(table.Info.AutoIncID)

?

Sign up for freeto 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.

6 participants

@3pointer@overvenus@kennytm@glorv@lichunzhu@ti-chi-bot