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

include files without blockset in broken files (issue #4988) #4990

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gpatel-fr
Copy link
Contributor

No description provided.

@Jojo-1000
Copy link
Contributor

If I understand this change correctly, it was caused because empty files/folders don't have a blockset, but can still be broken due to metadata.

I guess the BROKEN_FILE_IDS subquery correctly finds the files, but they were discarded in the external query? Did you check all usages that they can handle B.Length=null, or should it be set to zero instead?

@ts678
Copy link
Collaborator

ts678 commented Jul 10, 2023

Thanks for fast response, all. I'll stay pretty quiet since I think your SQL is better than mine.
I followed the general left join idea but didn't want to try to ask questions about the NULL.

My higher-level wonder was how well purge-broken-files purges a folder that's found bad.
Possibly that requires a build with the proposed change and a fancier backup to test with...

@gpatel-fr
Copy link
Contributor Author

@Jojo-1000
normal files have 2 blocksets, one for data, one for metadata, directories have one blockset for metadata.
Yes the internal query finds bad files (it's named for that in LocalListBrokenFilesDatabase.cs) and it's working fine as far as I understand it.
I don't think the Null value is really used elsewhere. A quick test shows it displaying -1 for the length. Not very cute but it's better that not displaying anything.

@ts678
I don't think that this is used to purge broken files. From browsing (quickly) the code in PurgeBrokenFileHandler.cs, the purge is not using the database at all. That's probably the reason it was working in your case while the list is not.

@Jojo-1000
Copy link
Contributor

@gpatel-fr
I made a test database with an empty file and it does have a blockset with size 0 in the database, so this is only an issue for folders and symlinks. I think for those we can keep the -1 size, this is consistent with the result from list files.
Otherwise you could use IFNULL(B.Length,A.BlocksetID) to output -100 for folders and -200 for symlinks, if it is necessary to distinguish the two.

@ts678
Copy link
Collaborator

ts678 commented Jul 11, 2023

I don't think that this is used to purge broken files.

Purge needs to figure it out too. Stack might look like:

public IEnumerable<Tuple<DateTime, long, long>> GetBrokenFilesets(DateTime time, long[] versions, System.Data.IDbTransaction transaction)
{
var query = string.Format(BROKEN_FILE_SETS, FOLDER_BLOCKSET_ID, SYMLINK_BLOCKSET_ID, RemoteVolumeType.Blocks.ToString());

public static Tuple<DateTime, long, long>[] GetBrokenFilesetsFromRemote(string backendurl, BasicResults result, Database.LocalListBrokenFilesDatabase db, System.Data.IDbTransaction transaction, Options options, out List<Database.RemoteVolumeEntry> missing)
{
missing = null;
var brokensets = db.GetBrokenFilesets(options.Time, options.Version, transaction).ToArray();

using (var db = new Database.LocalListBrokenFilesDatabase(m_options.Dbpath))
using (var tr = db.BeginTransaction())
{
if (db.PartiallyRecreated)
throw new UserInformationException("The command does not work on partially recreated databases", "CannotPurgeOnPartialDatabase");
var sets = ListBrokenFilesHandler.GetBrokenFilesetsFromRemote(m_backendurl, m_result, db, tr, m_options, out missing);

That's probably the reason it was working in your case while the list is not.

I'm not sure how well it was working, but I updated the issue with output after remembering --console-log-level must be raised, otherwise all the useful details get tossed. This is an unpleasant regression (IMO), introduced by the change of the logging code.

also change the message in purge-broken-files about last fileset to a warning.
@gpatel-fr
Copy link
Contributor Author

@ts678

I have changed the message log level to a Warning. It seems appropriate because asking for a purge that would remove the last fileset without specifying that one wants to do it is something of a mistake (while not being quite an error).

@duplicatibot
Copy link

This pull request has been mentioned on Duplicati. There might be relevant details there:

https://forum.duplicati.com/t/find-and-purge-have-different-results/16994/3

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.

None yet

4 participants