br icon indicating copy to clipboard operation
br copied to clipboard

when using BR backup/restore a database is empty , it will not be restored

Open wangbincmsss opened this issue 4 years ago • 12 comments

Question

	if len(dbInfo.Tables) == 0 {
		log.Warn("It's not necessary for backing up empty database",
			zap.Stringer("db", dbInfo.Name))
		continue
	}

As I see code in br/pkg/backup/client.go We have a application request restored empty database but now when a database is empty , it will not be restored could I pull a PR about add a Flag , determine whether restore a empty database or not

thanks

wangbincmsss avatar Mar 09 '21 05:03 wangbincmsss

cc #337.

kennytm avatar Mar 10 '21 10:03 kennytm

@kennytm when a empty database is used backup and restore , i have nothing to do ? i think add a param flag will be better

wangbincmsss avatar Mar 15 '21 03:03 wangbincmsss

@YuJuncen hi could I pull a PR code to accomplish this

wangbincmsss avatar Mar 16 '21 01:03 wangbincmsss

@YuJuncen hi could I pull a PR code to accomplish this

Of course. Due to the Schema structure (see below), maybe leave table field empty would be OK.

https://github.com/pingcap/kvproto/blob/a02e2549f82fe0f35455717fcb0a107ab264c486/proto/backup.proto#L69-L72

But this may introduce new compatibility problems. (e.g. Backups with newer version BR and this flag cannot be restored in an older version of BR), is that OK? @overvenus @kennytm

YuJuncen avatar Mar 16 '21 03:03 YuJuncen

yep i think if backup empty database. then should restore empty database, if not, should not restore empty database.@YuJuncen

wangbincmsss avatar Mar 16 '21 04:03 wangbincmsss

@wangbinhe3db Yes, and the current problem is, if we generate a backupmeta with empty databases, I'm afraid that older versions of BR might have no way to recognize them.

YuJuncen avatar Mar 16 '21 07:03 YuJuncen

i think the following behavior is acceptable:

  • old backup → any restore = empty db not backup, and thus skipped
  • new backup → old restore = creating empty db is "optional": the old BR can either ignore the new field (thanks to protobuf) or actually create the empty db via DDL.
  • new backup → new restore = create the empty db.

kennytm avatar Mar 16 '21 08:03 kennytm

@kennytm You mean, following https://github.com/pingcap/br/issues/337#issuecomment-640359151 or https://github.com/pingcap/br/issues/337#issuecomment-640363405?

YuJuncen avatar Mar 16 '21 08:03 YuJuncen

the second one.

kennytm avatar Mar 16 '21 08:03 kennytm

@wangbinhe3db And, eh, I'm not sure whether I fully understand what you are wanting us to do. @kennytm's solution has been ready to be working with.

If you meaning you want to provide a PR for us(thanks for that!), I can give you a recipe then.

Or you're just want us to give a hot fix? If so, I'm ready to do that.

UPDATE: This would be blocked by backupmetaV2(Not documented yet.).

YuJuncen avatar Mar 16 '21 08:03 YuJuncen

I want to provide a PR for community, to learn about this , could you give me some tips , thks @YuJuncen

wangbincmsss avatar Mar 16 '21 09:03 wangbincmsss

Notes:

For solving #818, a new version of backupmeta is developing, if possible, it would be better to start your development after that have been done, because related codes would be probably changed after new version of backupmeta get done.


@wangbinhe3db OK, so as #337 said, current version of backupmeta doesn't support backing up empty schemas at all. Therefore, firstly, you should update the backupmeta type at the kvproto repository.

You might would like to add a field like repeated bytes empty_schemas(bytes for storing the marshaled dbInfo JSON). After this get done, you can update backup and restore procedure of BR, to read and write this field.

For the backing up side, you can edit the function BuildBackupRangeAndSchema, letting it returns dbInfo of empty schemas, and then marshal and write them to the new field you just added.

https://github.com/pingcap/br/blob/f628f4ea420aee5dcc743806fb1d890d18bec22d/pkg/backup/client.go#L268-L274

For restoring side, you can edit the LoadBackupTables, read and unmarshal those dbInfo from the backup meta. Then, return them.

https://github.com/pingcap/br/blob/6e1a8ca1399ba15e9b6314d505db2e4978231d98/pkg/utils/schema.go#L71-L78

Finally, you can change the test case br_backup_empty. Adding a new test case to make sure empty databases can be backed up.

YuJuncen avatar Mar 17 '21 07:03 YuJuncen