Skip to content

Point-in-time-recovery#4857

Closed
deepthi wants to merge 20 commits intovitessio:masterfrom
planetscale:ds-pitr-tablet
Closed

Point-in-time-recovery#4857
deepthi wants to merge 20 commits intovitessio:masterfrom
planetscale:ds-pitr-tablet

Conversation

@deepthi
Copy link
Copy Markdown
Collaborator

@deepthi deepthi commented May 7, 2019

Partial fix for #4886

  • CreateKeyspace now takes three additional (optional) parameters
    • keyspace_type: NORMAL/SNAPSHOT
    • base_keyspace: keyspace that you are trying to recover
    • snapshot_time: for SNAPSHOT keyspaces, the time for which it is being recovered
  • when creating a keyspace, copy vschema from base_keyspace
  • Vttablet recovery mode
    • init_keyspace: this should be a SNAPSHOT keyspace
    • snapshot_time will be used as a starting point to find a backup to restore (most recent backup from snapshot_time)
    • replication will not be setup when restoring a snapshot keyspace
  • Vtgate changes
    • tables and vindexes of SNAPSHOT keyspaces are not added to the uniqueTables and uniqueVindexes maps when building vschema. This is sufficient to hide them from the global routing.
  • It is not necessary to bring up a master tablet for PITR. All tablets can be replicas.
  • If a master is desired for any reason, it should be initialized using TabletExternallyReparented, not InitShardMaster or PlannedReparentShard.
  • How to access recovered data
    • use @replica followed by select ... from ks.table
    • use ks@replica or use ks:shard@replica followed by select .. from table
  • added unit tests for CreateKeyspace
  • integration test to test recovery of vttablet + vtgate routing

Signed-off-by: deepthi deepthi@planetscale.com

@deepthi deepthi requested a review from sougou as a code owner May 7, 2019 19:36
deepthi added 3 commits May 14, 2019 14:23
  - keyspace_type: NORMAL/SNAPSHOT
  - snapshot_time: for SNAPSHOT keyspaces, the time for which it is
being recovered
  - also when creating a keyspace, create an empty vschema
* Vttablet recovery mode
  - recovery_keyspace: which keyspace to recover
  - init_keyspace: this should be a SNAPSHOT keyspace
  - snapshot_time will be used as a starting point to find a backup to
restore (most recent backup from snapshot_time)
* Vtgate changes
  - tables and vindexes of SNAPSHOT keyspaces are not added to the
uniqueTables and uniqueVindexes maps when building vschema
* integration test to test recovery of vttablet

Signed-off-by: deepthi <deepthi@planetscale.com>
Signed-off-by: deepthi <deepthi@planetscale.com>
…ime is not set

Signed-off-by: deepthi <deepthi@planetscale.com>
deepthi added 2 commits May 29, 2019 16:04
…ion for CreateKeyspace

call RebuildVSchemaGraph after copying vschema for snapshot keyspace

Signed-off-by: deepthi <deepthi@planetscale.com>
Signed-off-by: deepthi <deepthi@planetscale.com>
@deepthi deepthi changed the title WIP: Features needed to facilitate Point-in-time-recovery Features needed to facilitate Point-in-time-recovery May 30, 2019
Signed-off-by: deepthi <deepthi@planetscale.com>
@deepthi deepthi changed the title Features needed to facilitate Point-in-time-recovery Point-in-time-recovery May 30, 2019
deepthi added 3 commits May 30, 2019 17:31
Signed-off-by: deepthi <deepthi@planetscale.com>
Signed-off-by: deepthi <deepthi@planetscale.com>
…nd which gtid we restored to

Signed-off-by: deepthi <deepthi@planetscale.com>
Copy link
Copy Markdown
Contributor

@sougou sougou left a comment

Choose a reason for hiding this comment

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

Still need to do a deeper dive. Have one high level comment so far.

dir := fmt.Sprintf("%v/%v", tablet.Keyspace, tablet.Shard)
name := fmt.Sprintf("%v.%v", time.Now().UTC().Format("2006-01-02.150405"), topoproto.TabletAliasString(tablet.Alias))
returnErr := mysqlctl.Backup(ctx, agent.Cnf, agent.MysqlDaemon, l, dir, name, concurrency, agent.hookExtraEnv())
backupTime := time.Now().UTC()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think the backup time should be pulled from the server's timestamp. Ideally, as of the snapshot time. But I can see someone make an argument for the current time as seen by vttablet. @rafael: thoughts?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

that is a good point. until now this time was only used to make the backup directory, so it didn't matter, but for purposes of knowing where to replay binlogs from, it should be server time.

deepthi and others added 9 commits June 20, 2019 10:46
Signed-off-by: deepthi <deepthi@planetscale.com>
distribute backup tests among jobs

Signed-off-by: deepthi <deepthi@planetscale.com>
Signed-off-by: deepthi <deepthi@planetscale.com>
Signed-off-by: deepthi <deepthi@planetscale.com>
Signed-off-by: deepthi <deepthi@planetscale.com>
Signed-off-by: deepthi <deepthi@planetscale.com>
@deepthi deepthi requested a review from enisoc July 3, 2019 16:26

log.Infof("Restoring latest backup from directory %v", backupDir)
restorePos, err := mysqlctl.Restore(ctx, mycnf, mysqld, backupDir, *concurrency, extraEnv, map[string]string{}, logutil.NewConsoleLogger(), true, dbName)
restorePos, _, err := mysqlctl.Restore(ctx, mycnf, mysqld, backupDir, *concurrency, extraEnv, map[string]string{}, logutil.NewConsoleLogger(), true, dbName, time.Unix(0, 0).UTC())
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Traditionally we use the zero value time.Time{} to represent "unspecified". Is there any reason we can't make Restore() accept that?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

no reason, I just didn't know there was a convention. I will change it.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Still need to change this to time.Time{} (and a few more places below). Also, I noticed that you were calling .UTC() in a few places. That should not be necessary because time.Unix() will return the correct value whether it's UTC or not.

// to convert times.
message Time {
int64 seconds = 1;
int32 nanoseconds = 2;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd say go with int64. There's no great benefit to using int32.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

this is an existing message that I moved into a new file. Is it safe to change the data type?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

we'll keep this as int32 because changing the type will not be backwards-compatible


log.Infof("Restoring latest backup from directory %v", backupDir)
restorePos, err := mysqlctl.Restore(ctx, mycnf, mysqld, backupDir, *concurrency, extraEnv, map[string]string{}, logutil.NewConsoleLogger(), true, dbName)
restorePos, _, err := mysqlctl.Restore(ctx, mycnf, mysqld, backupDir, *concurrency, extraEnv, map[string]string{}, logutil.NewConsoleLogger(), true, dbName, time.Unix(0, 0).UTC())
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Still need to change this to time.Time{} (and a few more places below). Also, I noticed that you were calling .UTC() in a few places. That should not be necessary because time.Unix() will return the correct value whether it's UTC or not.

Signed-off-by: deepthi <deepthi@planetscale.com>
Signed-off-by: deepthi <deepthi@planetscale.com>
@deepthi deepthi requested a review from derekperkins as a code owner August 13, 2019 23:46
@deepthi
Copy link
Copy Markdown
Collaborator Author

deepthi commented Sep 4, 2019

Closing this in favor of #5160.
@enisoc if you have a review in progress on this, please go ahead and submit it and I will look at it.

@deepthi deepthi closed this Sep 4, 2019
@deepthi deepthi deleted the ds-pitr-tablet branch July 6, 2020 19:01
@arindamnayak arindamnayak mentioned this pull request Jul 18, 2020
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

Successfully merging this pull request may close these issues.

3 participants