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

Folder size not calculated (state 'pending') on 32-bit systems #28275

Closed
bcutter opened this issue Jun 30, 2017 · 61 comments · Fixed by #28649
Closed

Folder size not calculated (state 'pending') on 32-bit systems #28275

bcutter opened this issue Jun 30, 2017 · 61 comments · Fixed by #28649

Comments

@bcutter
Copy link

bcutter commented Jun 30, 2017

Expected behaviour

Folder size gets calculated correctly for every (sub)folder

Actual behaviour

Since upgrade from v9.1.6 I was wondering why desktop client showed "0 KB is used" in account tab.

First, running occ files:scan made some improvement; now "only" two subfolders aren´t calculated - state "pending". Running occ files:scan for affected user account or even using "--path=/username/files/affected/path" option didn´t fix it.

In MySQL db "select COUNT(fileid) from oc_filecache where size like "-1"" gives 290 entries; most of them exactly in those two affected subfolders.

How can I fix that? Do I need to edit those database rows manually? Can I re-create them, only for the two affected subfolders? As already mentioned, occ files:scan with "--path=/username/files/affected/path" option doesn´t work.

Server configuration

Operating system:
Raspbian Jessie
Web server:
nginx
Database:
MySQL
PHP version:
5.6
ownCloud version: (see ownCloud admin page)
10.0.2.1
Updated from an older ownCloud or fresh install:
Updated from 9.1.6 to 10.0.2.1
Where did you install ownCloud from:
native package from website (no OS repositories)
Signing status (ownCloud 9.0 and above):
few errors

https://gist.github.com/bcutter/d43ca098412a1aed3bdc9170c31ffbcc

Are you using external storage, if yes which one: local/smb/sftp/...
No, everything local in /owncloud/data/
Are you using encryption: yes/no
no

@bcutter
Copy link
Author

bcutter commented Jul 1, 2017

Guys I need a fix for this. Even a (temporary) workaround would be good at the moment. What is the problem here? How does file and folder size calculation work internally in owncloud? What about the database I mentioned above?

@PVince81
Copy link
Contributor

PVince81 commented Jul 3, 2017

Guys I need a fix for this

Asking busy people for fast and free support ?

http://owncloud.org/support

@bcutter
Copy link
Author

bcutter commented Jul 3, 2017

Yes, that's what we do here right? OwnCloud central sent me over here. I already put in a lot of research and testing time, without success. So the last alternative is to ask the experts. That's why we're here.

Similar but not exactly same problems: #16742, #13876

@PVince81
Copy link
Contributor

PVince81 commented Jul 3, 2017

Can you link to your posts on central if they contain information about your research ?

@bcutter
Copy link
Author

bcutter commented Jul 3, 2017

@ghost
Copy link

ghost commented Jul 11, 2017

Asking busy people for fast and free support ?

That's unnecessarily rude and non-constructive. The man is desperate and in the time you wrote this reaction, you could also have overlooked the problem quickly and made a small suggestion like you did afterwards. If you don't like this platform and its users, quit GitHub and open-source developing. If you don't want to lose the battle against NextCloud, keep playing on this field and try to be nice.

Anyway, on topic: I might have the same problem.

  • Table oc_filecache seems to be fine here, all sizes seems to be correct
  • It only happens on large folders, it seems > 1 GB
  • It was not happening on OC9, I upgraded to OC10 and suddenly some large folders stated 'Pending'
  • I even have one folder with a 'Pending' state, but the summary within that folder shows the correct size!

@PVince81
Copy link
Contributor

@FCTURNER were this old folders that suddenly switched to "Pending" ? Or are these newly added big folders ?

Are you running on a 32-bit system ? (to exclude potential "large folder handling" issues)

Normally for existing local folders (assuming no shares not federated shares), the state should never switch back to "Pending" after the value was already set to something. The code that updates the value runs when modifying the contents of the folder and only adds or substracts integers from that column. So not sure what could cause this.

You could check the DB before and after the update: select * from oc_filecache where size=-1 (size -1 is the "pending" value)

@PVince81
Copy link
Contributor

and... apologies for my brief moment of weakness.

@bcutter
Copy link
Author

bcutter commented Jul 11, 2017

No problem @PVince81, we all are humans (most of us :-) ) and I really believe you guys have a lot of work to do here - and I´m happy you do this work for us (users).

In my case, OC is running on a Raspberry Pi 2. To add further information, here some of external posts (from OwnCloud Central), so we have all possibly relevant information here:

Same problem with v10.0.2.1. Since upgrade from v9.1.6 I was wondering why desktop client showed "0 KB is used" in account tab.

Running occ files:scan made some improvement; now "only" two subfolders aren´t calculated - state "pending". Running occ files:scan for affected user account or even using "--path=/username/files/affected/path" option didn´t fix it.

In MySQL db "select COUNT(fileid) from oc_filecache where size like "-1"" gives 290 entries; most of them exactly in those two affected subfolders.

@PVince81
Copy link
Contributor

@bcutter ok so in your case it's about a SMB external storage.

@FCTURNER can you confirm whether you observe the problem only on external storages or also local storage ?

@bcutter there is a cron / background job that should process the "size = -1" items in the database in small batches. You could try running php cron.php or sudo -uwww-data php cron.php a few times and see if the number of "size = -1" entries in the DB is getting down.

I do find it strange that occ files:scan with path doesn't help. Did you try adding -vvvvvv at the end of the command ? It should show you what folders it's scanning into, see if it goes into the "Pending" folder at all.

@bcutter
Copy link
Author

bcutter commented Jul 11, 2017

Thanks for your input, I can work on that.

  1. No it´s local storage, in standard data directory (/.../owncloud/data).
  2. I will try running the cron a few times, having a look at "sudo watch -n1 'mysql select statement'".
  3. I didn´t use the -vvvvvv (I guess 'verbose' = debug?) option so far; I will do so and come back with the results in a few hours.

@PVince81
Copy link
Contributor

No it´s local storage, in standard data directory (/.../owncloud/data).

If the folders were created over Webdav or OC web UI, they should already have the correct size, not "Pending". Are you modifying/adding folders into "owncloud/data" outside of the ownCloud interfaces ? Usually it's not recommanded to use OC that way as OC has no way to detect such changes and you need to resort to occ files:scan. Let's see what the verbose mode says...

@bcutter
Copy link
Author

bcutter commented Jul 11, 2017

@PVince81: The affected folders are pretty old (static folder structure) and I´m pretty sure they were created using OC Desktop-Client = WebDAV.

Ha, good point, there might be an event related to that scenario ("modifying/adding folders into "owncloud/data" outside of the ownCloud interfaces"): A few days after the update to OC v10 there was a full system (file and folder structure 1:1 from tar backup) and MySQL database restore. Both things (files + database) were exactly the same as before (so files and oc_filecache table fit together). And - important! - I already detected the pending state after the update from OC v9.1.6 to v10.0.2.1, so I´m pretty sure the backup restore isn´t relevant. But to be fair I of course want to provide all information available.

So, to summarize: No, adding/modifying/deleting files from the user´s data folders not using standard channels like web or OC desktop client is NOT performed.

@ghost
Copy link

ghost commented Jul 29, 2017

@PVince81 Awesome, thanks.

@FCTURNER can you confirm whether you observe the problem only on external storages or also local storage ?

No, actually local storage only. But I found out that it has nothing to with the oc_filescache table, there are no incorrect file and folder size. The problem lies in the sum of those records; when it exceeds 1 GB, the state of the parent folder will be Pending, although the calculation of the sums are fine (column size of a parent folder is correct in oc_filescache). It's also not a JavaScript error, but a PHP error, since both the OC iOS app and the desktop client show incorrect quota's.

It happened as soon as I upgraded from the latest OC 9 to OC 10.01. I didn't edit any file of folder, but after the update all folders with > 1 GB in total showed Pending. Rescanned with occ command, but it didn't help. Which lib files are used for summing up the child records of a folder? Then I can look to those to investigate further.

edit: In addition: calculated folder sizes in pending folders are shown in the web GUI as for example -1270505944 B and -468866787 B.

@PVince81
Copy link
Contributor

Hmmm but the size column is a bigint, so it's 64 bits, it shouldn't be able to wrap back to negative values. That is, unless it's your PHP version that is wrapping back to negative values ? Are you using a 32-bit PHP version by any chance ? There used to be issues with values bigger than 2 GB.

@ghost
Copy link

ghost commented Jul 31, 2017 via email

@PVince81
Copy link
Contributor

@FCTURNER there was already a fix in place for 32 bit sizes, not sure why it stopped working: #5365, especially this bit here https://github.com/owncloud/core/pull/5365/files#diff-01acafc8091c51ac67a0be326eadfbd4R92.

@bcutter
Copy link
Author

bcutter commented Aug 2, 2017

Seems like this is the root cause - I guess a Raspberry (running on Raspbian) is 32 bit too (because ARM is 32 bit based).

@PVince81: File structure changed since then, can´t find any of those files.

Just to finish my TODOs:

  • Running cron manually several times didn´t change anything (still 290 rows in oc_filecache with size = -1).
  • Running "sudo -u www-data php /var/www/owncloud/occ files:scan --path="/Username/files/MainFolder/SubFolder/SubSubFolder/AffectedFolder" -vvvvvv" (verbose/debug output) returned nothing interesting: occ goes through all files and folders, just lists "File /.../.../filename" or "Folder /.../.../foldername" etc. with a summary:
    +---------+-------+--------------+
    | Folders | Files | Elapsed time |
    +---------+-------+--------------+
    | 153 | 1859 | 00:01:56 |
    +---------+-------+--------------+

So working on the 32 bit / size issue might make some progress. What can I / can we (@FCTURNER + me) do?

@PVince81
Copy link
Contributor

PVince81 commented Aug 2, 2017

If you have time, setup a second OC instance from git and then do a git bisect between OC 9 (whichever version still worked) and OC 10 (whichever version is broken) to find what change caused the problem.

The second approach would be trying to dig into the code and find where the problem appears. It's a bit more complicated because it might require adding more logging in several places to find out what size values are being passed around.

@bcutter
Copy link
Author

bcutter commented Aug 2, 2017

Update/in addition:
I renamed one of the affected folders (sub-sub-sub-sub...-folder) on a Windows client and waited for the sync (OC desktop client). Sync finished, but no change. I checked the folder on the server: it doesn´t exist there!

Desktop client: "Error transferring ... server replied: Internal Server Error"

owncloud.log:

Error | PHP | fopen(/var/www/owncloud/data/Chris/files/Datastore/Studium/Software/Citavi/Projects/Information  Literacy (Citavi 5-Demo)/Citavi Attachments): failed to open stream: No  such file or directory at  /var/www/owncloud/apps/files_locking/lib/lock.php#93
-- | -- | --

==> Why the hell is this folder missing on the server? Of course the cron can´t calculate the size if the folder doesn´t exist on the server. Helpless 😞

I´ll try to make a directory comparison (endpoint vs. server) next.

@PVince81
Copy link
Contributor

PVince81 commented Aug 2, 2017

@bcutter likely a different issue. Could be a PHP timeout issue during the move. Run occ files:scan to make your folder reappear. It is likely that it was moved on disk but oc_filecache was not updated due to timeout.

@bcutter
Copy link
Author

bcutter commented Aug 2, 2017

OK, but now this specific folder is existing on the server (on an endpoint: moved it to outside OC sync folder, put it back in, forced sync on desktop client). BUT: the appropriate row in oc_filecache is still saying size = -1. So either this has nothing to do with oc_filecache table or those affected rows/folders aren´t updated in oc_filecache. Makes some headache meanwhile...

@bcutter
Copy link
Author

bcutter commented Aug 2, 2017

Enough testing and hacking for today. Another 3 hours spent without making any progress.

Let´s come back to #28275 (comment)

@ghost
Copy link

ghost commented Aug 5, 2017

Alright, first quick fix:

Replace 0 + to (int) in the file lib/private/Files/Cache/Cache.php.

For example:

$sum = 0 + $sum;

will become:

$sum = (int)$sum;

Now all Pending states are gone, but the maximum of 2 GB per folder is shown. So folders with a total size beyond 2 GB will still be shown as 2 GB. But the upside is: no more negative folder size values, and the possibilty to add files again, because the quota is being calculated again (but underestimated, of course).

I'll keep searching for a better solution.

@bcutter
Copy link
Author

bcutter commented Aug 5, 2017

All parts with "0 +"? I count four.

@ghost
Copy link

ghost commented Aug 5, 2017 via email

@ghost
Copy link

ghost commented Aug 5, 2017

I tried $... = (float)$...; by the way to circumvent using (int). Didn’t help, breaks it again and sets states back to Pending.

@bcutter
Copy link
Author

bcutter commented Aug 5, 2017

Well, not really/fully: Now the pending state is gone and every affected folder simply shows "2 GB" as folder size, but: number of rows with size =-1 in oc_filecache table is still unchanged (290) after several manual cron runs.

mysql> select count(fileid) as AffectedElements from oc_filecache where size = -1;
+------------------+
| AffectedElements |
+------------------+
|              290 |
+------------------+
1 row in set (0.21 sec)

So it seems like an optical fix (for the web UI and desktop client folder view), unfortunately not a functional :-(

I´m wondering why nobody else puts some work on this (especially your input @FCTURNER, might be a hot lead). I´m only a (technical) user, not a developer. :-(

Note for me: I reverted those changes in Cache.php because of fear of any unknown side effects.

@ghost
Copy link

ghost commented Aug 6, 2017 via email

@ghost
Copy link

ghost commented Aug 6, 2017 via email

@mrow4a
Copy link
Contributor

mrow4a commented Aug 7, 2017

Ye, we can maybe change the phpdoc and say it returns string, which in fact is int.. but it is a bit workaround, so needs proper explanation. @DeepDiver1975 thoughts on this?

@individual-it
Copy link
Member

to check if the size is valid we should use is_numeric() http://php.net/manual/en/function.is-numeric.php
that should also work for big numbers on a 32bit system http://php.net/manual/en/function.is-numeric.php#102011

@mrow4a
Copy link
Contributor

mrow4a commented Aug 7, 2017

Makes sense. Motivation for that change was that the interface defined return as int, so we just need to change it to string and if not numeric throw a exception or sth.

@PVince81
Copy link
Contributor

PVince81 commented Aug 7, 2017

@mrow4a can you take care of the revert + phpdoc changes ?

@mrow4a
Copy link
Contributor

mrow4a commented Aug 7, 2017

Ok, so the decision is to change interface to return numeric string (verified by is_numeric()) in phpdoc?

@PVince81
Copy link
Contributor

PVince81 commented Aug 7, 2017

If you insist on verifying, then use is_numeric(). Else just adjust the PHPDocs alone.

@individual-it
Copy link
Member

@bcutter could you please confirm if reverting #27389 does fix your problem?

@mrow4a
Copy link
Contributor

mrow4a commented Aug 8, 2017

Guys, reverting is not a good idea.. at least I dont want to sign my name over an interface which returns int or string based on the weather or humor.

@mrow4a
Copy link
Contributor

mrow4a commented Aug 8, 2017

#28605

@PVince81
Copy link
Contributor

PVince81 commented Aug 8, 2017

well, PHP returns a string instead of an int based on the platform for big numbers... so much for consistency. Doesn't make it possible to have consistent interfaces.

@mrow4a
Copy link
Contributor

mrow4a commented Aug 8, 2017

I mean, the solution proposed was returning numeric string, checked by is_numeric(), as interface in PR describes.

@phil-davis
Copy link
Contributor

Whenever an is_numeric() string is put into a calculation, it is made into a number and the calculation is done. If small enough to fit into the platform MAXINT it happily works like an int. If bigger, then it goes in a float/double. If it overflows double then the answer is double(INF)
So an is_numeric() string all the time should work just as well as it being a proper "int" when small and a "string" when big. I guess there will be a little bit of performance difference in what is mostly an int being converted to a string to be passed back and forth, then automagically type-converted to int when used in a calculation.

@jvillafanez
Copy link
Member

I think the best solution is to return numeric strings as @mrow4a has said. For calculations with sizes, we could try the BC Math extension: http://www.php.net/manual/en/book.bc.php

Note that anyway, I expect a lot of changes.

@phil-davis
Copy link
Contributor

bcadd() works nicely - feed it 2 is_numeric() strings or int in any combination and length/size and it gives you back the sum as an is_numeric() string
when you need to store the number in a database, then you have to think how best to do it - packed-BCD anyone :)
and there would be some performance overhead for 99.99% of cases where there is 64-bit int available, or a 32-bit system with a small install.

@jvillafanez
Copy link
Member

If the performance loss of using the BC match extension is negligible in every system, I'd just use it without any check.

We can create a FileSizeOperations class, or something like that, to wrap all of the size operations and work with the BC match extension transparently. If it really make sense, we can optimize the class to use integers under several circumstances, but anyway, the class would work with string and would return string.
Note that working with integers there might be an overoptimization that might not be worthy: the performance gain we could have by using integers could be lost due to the extra checks we'll need to do. In addition, the code will be more complex and difficult to maintain.

@individual-it
Copy link
Member

well, PHP returns a string instead of an int based on the platform for big numbers... so much for consistency. Doesn't make it possible to have consistent interfaces.

I thought Doctrine would always return strings regardless of the system (32/64bit) if we use BIGINT in the database https://doctrine-orm.readthedocs.io/projects/doctrine-dbal/en/stable/reference/types.html#bigint
so that getSize() would always return a string

@PVince81
Copy link
Contributor

PVince81 commented Aug 9, 2017

We can create a FileSizeOperations class

this might go a bit too far just for quick 32-bit compatibility

@PVince81
Copy link
Contributor

PVince81 commented Aug 9, 2017

especially for something that worked before but somehow was just inconsistent with the interfaces.

It seems that with all this type weirdness it is not possible to have a clean typed interface and at the same type 32-bit compatibility.

@PVince81
Copy link
Contributor

Revert PR here #28649 for the short term

@bcutter
Copy link
Author

bcutter commented Sep 1, 2017

I just wanted to confirm: I changed the FileInfo.php manually. The PR revert worked, everything back normal now.

I´m just wondering what happens on the update to OC v10.0.3... is this revert merged to master or will I need to do the modification of FileInfo.php again after next update? Thank you all so far for your efforts.

@jvillafanez
Copy link
Member

There is a backport to stable10 so it will be for 10.0.3, no need to change anything.

@felixboehm felixboehm removed this from the triage milestone Apr 10, 2018
@lock
Copy link

lock bot commented Jul 30, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jul 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants