Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The AUTO_INCREMENT has been changed #381

Closed
ghost opened this issue Mar 3, 2017 · 9 comments
Closed

The AUTO_INCREMENT has been changed #381

ghost opened this issue Mar 3, 2017 · 9 comments
Assignees

Comments

@ghost
Copy link

ghost commented Mar 3, 2017

This is the place to report a bug, ask a question, or suggest an enhancement.

When we delete some data, and the max value of AUTO_INCREMENT columns is not match the AUTO_INCREMENT of the table. Then we use ghost to do some alter .The AUTO_INCREMENT of the table have be changed.

This is also the place to make a discussion before creating a PR.

If this is a bug report, please provide a test case (e.g., your table definition and gh-ost command) and the error output.
Please use markdown to format code or SQL: https://guides.github.com/features/mastering-markdown/

CREATE TABLE ghost_test(
    -> `id` bigint(20) unsigned NOT NULL AUTO_INCREMENT,
    -> val int(11) NOT NULL DEFAULT '0',
    -> PRIMARY KEY (`id`)
    -> )ENGINE=InnoDB;

we insert 3 rows:

>select * from ghost_test;
+----+-----+
| id | val |
+----+-----+
|  1 |   1 |
|  2 |   2 |
|  3 |   3 |
+----+-----+
3 rows in set (0.00 sec)

delete 1 row

>delete from ghost_test where id =3;
Query OK, 1 row affected (0.00 sec)

>select * from ghost_test;
+----+-----+
| id | val |
+----+-----+
|  1 |   1 |
|  2 |   2 |
+----+-----+
>show create table ghost_test\G
*************************** 1. row ***************************
       Table: ghost_test
Create Table: CREATE TABLE `ghost_test` (
  `id` bigint(20) unsigned NOT NULL AUTO_INCREMENT,
  `val` int(11) NOT NULL DEFAULT '0',
  PRIMARY KEY (`id`)
) ENGINE=InnoDB AUTO_INCREMENT=4 DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci
1 row in set (0.00 sec)

Then we used gh-ost to do "Alter table ghost_test engine=innodb"

show create table ghost_test\G
*************************** 1. row ***************************
       Table: ghost_test
Create Table: CREATE TABLE `ghost_test` (
  `id` bigint(20) unsigned NOT NULL AUTO_INCREMENT,
  `val` int(11) NOT NULL DEFAULT '0',
  PRIMARY KEY (`id`)
) ENGINE=InnoDB AUTO_INCREMENT=3 DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci
1 row in set (0.00 sec)

Please label the issue on the right (bug, enhancement, question, etc.).
My business friend try to fix the bug.
He add the AUTO_INCREMENT to the SQL which create the new table .

There is his code

func (this *Applier) ModifyGhostTable() (string, error) {
	SourceStr := fmt.Sprintf(`show create table %s.%s`,
		sql.EscapeName(this.migrationContext.DatabaseName),
		sql.EscapeName(this.migrationContext.OriginalTableName),
	)
	ResponseSource, err := sqlutils.QueryResultData(this.db, SourceStr)
	if err != nil {
		return "", err
	}
	SourceStrip := strings.Replace(ResponseSource[0][1].String, "`", "", -1)
	NewGhostTableName := this.migrationContext.DatabaseName + "." + this.migrationContext.GetGhostTableName()
	NewGhostTableSQL := strings.Replace(SourceStrip, this.migrationContext.OriginalTableName, NewGhostTableName, -1)
	return NewGhostTableSQL, nil
}

// CreateGhostTable creates the ghost table on the applier host
func (this *Applier) CreateGhostTable() error {
	NewGhostTableSQL, err := this.ModifyGhostTable()
	if err != nil {
		return err
	}
	log.Infof(NewGhostTableSQL)
	if _, err := sqlutils.ExecNoPrepare(this.db, NewGhostTableSQL); err != nil {
		return err
	}
	log.Infof("Ghost table created")
	return nil
}

And please understand if this issue is not addressed immediately or in a timeframe you were expecting.

Thank you!

@parham-pythian
Copy link

JiaqingXu, can you share the gh-ost statement and flags you used? I would like to try to reproduce the issue related to another test I'm running.

@shlomi-noach
Copy link
Contributor

@jiaqingxu thank you, and it took me three reads to realize this Issue also offers a patch. Are you able to submit it as a pull request please?

@shlomi-noach
Copy link
Contributor

shlomi-noach commented Apr 13, 2017

I believe there's a race condition that cannot be solved unless locking down the original table.

In the patch offered above, the app issues a show create table, then somehow (the code needs to be fixed, it uses a naive assumption about the creation options, I believe) parses the auto_increment=... value, and applies that onto the ghost table creation.

However, a row can be deleted in between these two statements. Actually, it can be also deleted after the creation of the ghost table and the problem will remain the same.

Migration only begins well after the ghost table was created and inspected. Rows deleted until such time will have the same affect on having a wrong auto_increment.

As I see it, to solve the problem we'd need to lock tables original_table write while setting up migration initial position, listener, range, then alter table _***_gho_ auto_increment=....

I'm not sure I'm happy with this path.

@dikang123
Copy link

I don't think this is a big problem, even a record with the id=3 inserted, eventually. @jiaqingxu
But it is a indiscoverable issue ,really! haha:)

@qiuker521
Copy link

qiuker521 commented Jun 14, 2018

+1 for this issue

Under some conditions, the auto_increment id is important.

// CreateGhostTable creates the ghost table on the applier host
func (this *Applier) CreateGhostTable() error {
	query := fmt.Sprintf(`create /* gh-ost */ table %s.%s like %s.%s`,
		sql.EscapeName(this.migrationContext.DatabaseName),
		sql.EscapeName(this.migrationContext.GetGhostTableName()),
		sql.EscapeName(this.migrationContext.DatabaseName),
		sql.EscapeName(this.migrationContext.OriginalTableName),
	)
	log.Infof("Creating ghost table %s.%s",
		sql.EscapeName(this.migrationContext.DatabaseName),
		sql.EscapeName(this.migrationContext.GetGhostTableName()),
	)
	if _, err := sqlutils.ExecNoPrepare(this.db, query); err != nil {
		return err
	}
	log.Infof("Ghost table created")
	return nil
}

We found the same issue a day ago:

create table a like b does not copy b's auto_increment id.

So we must do other things to avoid this.

😂😂😂😂😂😂

@shlomi-noach shlomi-noach self-assigned this Jun 14, 2018
@jiangnanora
Copy link

We are also facing the same problem, does the author plan to fix it?

@shlomi-noach
Copy link
Contributor

While I do not maintain this repository anymore, I'm interested in looking into this.

@shlomi-noach
Copy link
Contributor

Please see openark#12 for suggested solution.

@shlomi-noach shlomi-noach mentioned this issue Jan 5, 2021
2 tasks
@timvaillancourt
Copy link
Collaborator

This should be fixed by the merging of #967 which is a copy of openark#12

Please test with the latest master branch and re-open an issue if you see problems

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

No branches or pull requests

6 participants