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

Direct restore fails with non-default blocksize #5196

Closed
2 tasks done
Jojo-1000 opened this issue May 17, 2024 · 7 comments · Fixed by #5201
Closed
2 tasks done

Direct restore fails with non-default blocksize #5196

Jojo-1000 opened this issue May 17, 2024 · 7 comments · Fixed by #5201

Comments

@Jojo-1000
Copy link
Contributor

  • I have searched open and closed issues for duplicates.
  • I have searched the forum for related topics.

Environment info

  • Duplicati version: 2.0.8.1
  • Operating system: Windows 10
  • Backend: -

Description

Direct restore does not save the detected blocksize from an existing backup. Instead, it tries to use the default value.

The detection works for the first partial recreate, which creates the latest version. However, the default blocksize is saved in the temporary database instead of the detected version.

Steps to reproduce

  1. Create a backup with --blocksize=100KB, --upload-unchanged-backups to a local folder
  2. Backup twice, to have at least 2 versions
  3. Direct restore from that folder
  4. Select an older version
  • Actual result:
    A warning is shown and the list of files is empty.
2024-05-17 18:39:20 +02 - [Warning-Duplicati.Library.Main.Operation.RecreateDatabaseHandler-FileProcessingFailed]: Failed to process file: duplicati-20240517T162243Z.dlist.zip
InvalidManifestException: Invalid manifest detected, the field Blocksize has value 102400 but the value 1048576 was expected
  • Expected result:
    The older version should show in the list and be restored without any problems.
@ts678
Copy link
Collaborator

ts678 commented May 17, 2024

Duplicati version: 2.0.8.1

Are you sure? New default went into master on April 21, which would have been between Experimental and Beta, so unlikely.
plus GitHub shows 2.0.8.1 has the old default. While it's a good thing to find the bug, I'm hoping it didn't just go into a Beta.

@Jojo-1000
Copy link
Contributor Author

Are you sure? New default went into master on April 21, which would have been between Experimental and Beta, so unlikely.

You are right, the reproduction is for the current master branch. You should be able to reproduce it with a blocksize of 1MB in the released version.

@Jojo-1000
Copy link
Contributor Author

Jojo-1000 commented May 17, 2024

The cause why it only fails for direct restore is here:

if (!updating)
Utility.VerifyParameters(restoredb, m_options, tr);

VerifyParameters() writes the detected blocksize to the database, but for partial backups updating=true, even when the first recreate is run. Here, it seems to be assumed that the database was already initialized properly to avoid registering the volumes and set the options:

// If we are updating, all files should be accounted for
foreach(var fl in remotefiles)
volumeIds[fl.File.Name] = updating ? restoredb.GetRemoteVolumeID(fl.File.Name) : restoredb.RegisterRemoteVolume(fl.File.Name, fl.FileType, fl.File.Size, RemoteVolumeState.Uploaded);
var hasUpdatedOptions = false;
if (updating)
{
Utility.UpdateOptionsFromDb(restoredb, m_options);
Utility.VerifyParameters(restoredb, m_options);
}

For the first run, this is called before the detection, so it saves the default blocksize in the database instead of the correct one.

355ef92 added an extra autoDetectBlockSize to override the updating assumption for blocksize detection:

if (!hasUpdatedOptions && (!updating || autoDetectBlockSize))
{
VolumeReaderBase.UpdateOptionsFromManifest(parsed.CompressionModule, tmpfile, m_options);
hasUpdatedOptions = true;
// Recompute the cached sizes
blocksize = m_options.Blocksize;
hashes_pr_block = blocksize / m_options.BlockhashSize;
}

c7e1ece added a fix because the volumeIds are all -1 initially due to the wrong assumptions. Despite what the commit says, updating is not set to true for repairs, only for partial recreate when restoring.

// Register the files we are working with, if not already updated
if (updating)
{
foreach(var n in filelists)
if (volumeIds[n.File.Name] == -1)
volumeIds[n.File.Name] = restoredb.RegisterRemoteVolume(n.File.Name, n.FileType, RemoteVolumeState.Uploaded, n.File.Size, new TimeSpan(0), tr);
}

As a side note, this will also fail when changing the hash algorithms. I don't know why the if is there to block saving the parameters for update, as far as I can tell it should be save to just always run VerifyParameters(). It would obviously be cleaner to rework the updating flag so it actually represents what it means (only true if the database already partially exists).

@ts678
Copy link
Collaborator

ts678 commented May 18, 2024

You should be able to reproduce it with a blocksize of 1MB in the released version.

Confirmed. I'm surprised it hasn't been seen before. Maybe need of old version helps.

I know increased blocksize is sometimes used, e.g. for performance on larger backups.

Wouldn't increased default blocksize on .NET 8 versions increase the visibility of issue?

Any workaround spotted yet?

@Jojo-1000
Copy link
Contributor Author

I'm surprised it hasn't been seen before. Maybe need of old version helps.

I guess most people don't use direct restore, but import the job and recreate the database fully. And those who do were probably only interested in the latest version.

Any workaround spotted yet?

You should be able to set the blocksize explicitly in advanced options underneath the password to fix it temporarily.

@ts678
Copy link
Collaborator

ts678 commented May 19, 2024

Confirmed, but not underneath the screen 1 Password, but underneath the screen 2 Passphrase. It's easiest if you know the size exactly in the somewhat picky option format, e.g. --blocksize=1MB works, but giving it --blocksize=1048576 gets Warnings:

Warnings 2 
2024-05-19 09:19:04 -04 - [Warning-Duplicati.Library.Main.Controller-OptionValidationError]: The size "1048576" supplied to --blocksize does not have a multiplier (b, kb, mb, etc). A multiplier is recommended to avoid unexpected changes if the program is updated.
2024-05-19 09:19:08 -04 - [Warning-Duplicati.Library.Main.Operation.RecreateDatabaseHandler-FileProcessingFailed]: Failed to process file: duplicati-20240518T105448Z.dlist.zip
InvalidManifestException: Invalid manifest detected, the field Blocksize has value 1048576 but the value 1073741824 was expected

If one doesn't know the blocksize, it seems like hiding dlist files of undesired later dates works, e.g. by changing prefix (if possible).

kenkendk added a commit that referenced this issue May 21, 2024
Fixed issue by changing order options are written and verified.
Some whitespace changes due to renaming a method
@kenkendk
Copy link
Member

It took a bit to figure out what is going on exactly.
It happens because the UI automatically fetches version=0.
This works fine, but writes the default values for blocksize etc into the database.
When changing the version, the logic re-uses the database (with default sizes) and this triggers the error.

I reworked the logic around when the values are written, so they are always written if anything has been parsed from a manifest file, and changed the logic so the default values are not written in this operation.

There is a bit of logic involved, but essentially, the operation can only work if there already exists a remote file list, and this will naturally provide the correct values. I think a later refactor should split the validation of parameters and the saving of parameters to make it more clear what is intended.

kenkendk added a commit that referenced this issue May 27, 2024
…ocksize-issue

Implemented test for reproducing issue #5196
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 a pull request may close this issue.

3 participants