databacker / mysql-backup

image to enable automated backups of mysql databases in containers
648 stars 185 forks source link

Restore specific schema from archive #236

Closed AxelGuignard closed 1 year ago

AxelGuignard commented 1 year ago

Use the DB_NAMES variable without SINGLE_DATABASE to specify one or more databases to restore from a backup archive generated with DB_DUMP_BY_SCHEMA.

I am not a bash expert, so feel free to test and review my code. I tested it for simple cases without errors. Also, as I made it, there can be issues when schema names are overlapping.

AxelGuignard commented 1 year ago

It answers to issue https://github.com/databacker/mysql-backup/issues/121

deitch commented 1 year ago

I find this somewhat confusing.

* `DB_NAMES`: can be used in two distinct cases:
   * `SINGLE_DATABASE` has been set to `true`: in that case it specifies the name of the database to restore to, and it must contain exactly one database name.
   * `SINGLE_DATABASE` has not been set to `true` and `DB_DUMP_BY_SCHEMA=true` was used for backup: in that case it specifies which schemas to restore. It can contain one or more schema names separated with spaces.

DB_DUMP_BY_SCHEMA=true is an option during dump (and in the completely revamped version, close to release, this will be the only way to dump). So I think what you are saying it, "the compressed dump file has one file per database, which was set during dump via DB_DUMP_BY_SCHEMA=true," but I am not 100% sure.

Can we build a table here of all the possible scenarios? I still am confused, which means users likely will be as well.

AxelGuignard commented 1 year ago

@deitch Thank you for your feedback! I'm saying what you understood. Basically, I just check if there are multiple files in the compressed archive, which is what happen when you dump with DB_DUMP_BY_SCHEMA=true. But I also take into account when the param SINGLE_DATABASE is set to true while restoring to not override the default behavior (SINGLE_DATABASE + DB_NAMES). We can change the README if that part isn't clear.

deitch commented 1 year ago

This really will be easier if all dumps are DB_DUMP_BY_SCHEMA, which is what happens in the new version (blocked on one final missing thing in the native SMB/CIFS library).

So what are all of the possible permutations?

AxelGuignard commented 1 year ago

We check if there are multiple files in the archive, so it doesn't really matter whether the backup was made with DB_DUMP_BY_SCHEMA or not.

From what I understood it's not possible to restore multiple schemas if SINGLE_DATABASE is set to true, that's why it falls back to old behavior if that's the case. I can't really think of any other problematic cases.

AxelGuignard commented 1 year ago

I updated the README, tell me if it's clearer now

deitch commented 1 year ago

Let's see. When restoring:

Is that it? It is a bit confusing.

AxelGuignard commented 1 year ago

Yes, that's how it should work. Do you think we should use a variable other than DB_NAMES to make it less ambiguous? Something like ONLY=database1 database2 perhaps?

deitch commented 1 year ago

I still find it confusing, but have yet to come up with something better. DB_NAMES is the list of names of databases to backup when in dump mode, so it makes total sense that it is the list of databases to restore in restore mode.

That means:

I think I would just add a small section to the readme under the restore section describing the above, and call it a day

AxelGuignard commented 1 year ago

Another day, another try to make my explication clearer Please tell me what you think of this new enhanced version of README 😄

deitch commented 1 year ago

Yeah, that looks good. I am a real stickler for documentation (maybe because I have been tripped up so many times). Let's let CI run.

AxelGuignard commented 1 year ago

I totally understand your point. When writing documentation, you often write things that make sense to you as the person who made the change, but it isn't always helpful for others. I like to make things right, so your feedbacks were really appreciated.