occ files_external:notify does not propogate changes to clients.

[ariselseng]

Steps to reproduce

1. Mount a windows share in nextcloud
2. Let a desktop client sync the share.
3. Run occ files_external:notify -v <mount_id>
4. Make some changes remote, directly to the windows share by other means.

Expected behaviour

The desktop clients should propogate the changes that happened directly to the SMB share.

Actual behaviour

I get the changes listed out in the terminal output of the occ files_external:notify command, but nothing else happens.

Server configuration

Operating system:
Ubuntu Server 16.04
Web server:
Nginx 1.10

Database:
Mariadb 10
PHP version:
7.0/7.1 (tried both)
Nextcloud version: (see Nextcloud admin page)
11.0.1 and 11.0.2 (tried both)

Updated from an older Nextcloud/ownCloud or fresh install:
Upgrade.
Where did you install Nextcloud from:
Wonderfall/nextcloud:latest

Client configuration

Operating system:
Elementary with nextcloud-client 2.2.4

[nickvergessen]

@icewind1991

[ymolinet]

I have the same issue.
Any idea to fix it ?

Update : I can not see the reference of the new file in the table oc_filecache

[ymolinet]

This issue is a major problem for us because file change by a local SMB Share are not propagate to the nextcloud client. We can help you to fix this issue if we can understand how the sync process works and how the SMB notify works in Nextcloud.

[nickvergessen]

@icewind1991

[ymolinet]

up ?

[ymolinet]

I'm working on this issue, and I have a question.
In apps\files_external\lib\Command\Notify.php, I can found the function that it call by the notify command.
In the execute function (at line 90), I can found only the request to update a files already register in file_cache table, but no method to add or remove files....

[ymolinet]

If I understand the process correctly, another issue is possible.
External_Storage are register as mount point in oc_mounts.
Each user that use an external storage refer it by a storage id.
The storage id is use to refer the file in oc_filecache.

So, if a user push a file (from web ou client), it is refer in his storage, but all other are not updated ...

[ymolinet]

Anyone could explained how NextCloud get file data to store it in the filecache table?
especially mimetype list, mimepart, etag, permissions

[icewind1991]

The notify command doesn't write the new info to the filecache itself.

It marks a folder as out-of-date (setting it's size to -1), the background scanner (cron) will then detect that and handle updating the cache.

If you're running into issues with the cache not being updated while the notify command works fine, please verify that nextcloud's cron job is running correctly

[ymolinet]

So notify works only if a file or folder is changed, what's happen if we add a file ? In addition, scan must have the credential to access to the share especially if we use credential session (ldap user).

[icewind1991]

For new files the folder is also marked as outdated and the scanner will find the new files.

session credentials might indeed cause issues, not sure, will investigate later

[ymolinet]

If we use the session credential, scan can occur only if the user is connected to the web interface.
It could be interresting to allow scan when the user is connected throw the windows client (could be an option to enable in the admin panel).

[ymolinet]

If I correctly understand, the windows client use WebDAV to connect with NextCloud.
it could be interresting to invoque the cron (like ajax cron) when a user log on by Nextcloud client.

[ariselseng]

is it possible to run two cron jobs? one every minute that only for checking if a folder has been marked as updated? and the other for everything else. I have my cron time set to 15min, so it takes up to 15 minutes before a change is synced.

[ymolinet]

I think you can not solve this problem with two cron.
Your second job need to have the user credential store somewhere. If (like me) you used the credential in the session, NextCloud don't have the login/password to connect to the remote share.

[nickvergessen]

Multiple parallel cron jobs are supported since NC 10 I think

[ymolinet]

Up ?

[ymolinet]

I think that the best way to fix this issue is to create a "new" way to scan the files.
A scan method, running with a remote account can read permissions on the remote share and associate it with nextcloud user ... (LDAP Account).

[ymolinet]

Is it possible to have informations about the file_cache table (column description and waiting values)

[ymolinet]

I think a problem exist with the markParentAsOutdated function because it use dirname() php method.
under Windows, this method use / and \ to parse the directory, but under Linux it use only /.
So when Nextcloud is running under linux and check a Windows path, the parent directory is not correctly parse ...

[ymolinet]

In addition, path is return from notify with / at the beginning, but files in filecache don't have this (path column).

[philippe-opendsi]

Hi
We are experiencing the same problem. Using v12.04. Update and delete directly made in the filesystem (mounted in nextcloud via CIFS) are not correctly propagated to all users via sync desktop.

[MorrisJobke]

cc @icewind1991

[icewind1991]

Using 'Login credentials, save in database' instead of 'Login credentials, save in session' should make the file scanner pickup the changes correctly.

As for decreasing the "sync interval" you can run occ files:scan --unscanned --all as a separate cron job to pick up new changes before the "normal cron job" does it.

[MorrisJobke]

@icewind1991 Could you add this to the documentation for the notify command? Thanks

[ariselseng]

Is this working now with official nextcloud docker image + smbclient?

[ariselseng]

@theroch Is things working for you with your patch now?
I am now trying the official dockeri image + smbclient and when testing, and it seems some files do not get synced, even though they are listed during occ files_external:notify, but not during occ files:scan --unscanned.

[theroch]

@cowai I'm using now nextcloud 15.04 with desktop client 2.5.1. The sync works in this versions now much better, so I didn't test it with my patch.
Which files do you have problems with, in what depth of subdirectories do they lie?

[ariselseng]

@theroch Same versions here.
It seems to be quite random. In some tests its the file which resides in the 9th level that gets synced. Next time its the first level. I am testing by appending text to 3 files. Why Nextcloud still has problems with this after years is beyond me.
Try this a couple of times:
$ date >> SHARE/1th/2th/3th/4th/5th/testfile1.txt ;sleep 1;date >> SHARE/1th/2th/3th/testfile2.txt ;sleep 1;date >> SHARE/1th/testfile3.txt

Do each of the files show up in your client? For mine its totally random.

[ariselseng]

@MorrisJobke is this a known working feature now?

[MorrisJobke]

@MorrisJobke is this a known working feature now?

Yep - we have quite some instances where this is actively used for big external storages and there it propagates properly the changed files :/

[ariselseng]

@MorrisJobke @icewind1991 I have tried multiple times the last couple of years. Maybe there is something wrong/different with my setup?

The smb server is using Windows Server 2012 R2 Standard.

The main bug/problem I have is this:

The "occ files:scan --unscanned --all" command does not always list every change that the occ files_external:notify command did.

This is the procedure to reproduce:

1. use latest 15.0.4-apache official docker image
2. enable external storage
3. mount the smb storage on a machine for testing. ( a local cifs mount)
4. add a global smb mount
5. run php occ files_external:notify 1 -vv
6. setup a desktop client to sync a specific folder on smb. Let it finish syncing.
7. run this to check for unscanned:

$ while true;do php occ files:scan --unscanned --all;sleep 15;done

9. run this on the machine where you have the cifs share locally mounted:

$ mkdir -p SHARE/1th/2th/3th/4th/5th
$ date >> SHARE/1th/2th/3th/4th/5th/testfile1.txt && sleep 1 && date >> SHARE/1th/2th/3th/testfile2.txt 
 && sleep 1 && date >> SHARE/1th/testfile3.txt

10. watch as the occ files_external:notify command lists the changes successfully.
11. wait a couple of minutes.
12. check that the file tree on the share and on the desktop client is the same. It will not be.

[ariselseng]

@icewind1991 @MorrisJobke
I have confirmed that after doing some changes on smb the parent folders size is set to -1.
But if there is more than 1 file changed that shares a subset of the path tree, only one of the files is scanned correctly. This must be a bug. Note, I am running latest official docker image.

In my test 33 bytes are added to testfile1.txt, testfile2.txt and testfile3.txt
Here is output from sql query:

select path,size from oc_filecache where path like "SHARE/%"

#before doing changes:
SHARE/1th|198
SHARE/1th/2th|132
SHARE/1th/2th/3th|132
SHARE/1th/2th/3th/4th|66
SHARE/1th/2th/3th/4th/5th|66
SHARE/1th/2th/3th/4th/5th/testfile1.txt|66
SHARE/1th/testfile3.txt|66
SHARE/1th/2th/3th/testfile2.txt|66

#after changes, parent folders are marked with size -1 correctly:
SHARE/1th|-1
SHARE/1th/2th|132
SHARE/1th/2th/3th|-1
SHARE/1th/2th/3th/4th|66
SHARE/1th/2th/3th/4th/5th|-1
SHARE/1th/2th/3th/4th/5th/testfile1.txt|66
SHARE/1th/testfile3.txt|66
SHARE/1th/2th/3th/testfile2.txt|66

#after running occ files:scan --unscanned --all, note wrong sizes for testfile3.txt and testfile2.txt:
SHARE/1th|231
SHARE/1th/2th|165
SHARE/1th/2th/3th|165
SHARE/1th/2th/3th/4th|99
SHARE/1th/2th/3th/4th/5th|99
SHARE/1th/2th/3th/4th/5th/testfile1.txt|99
SHARE/1th/testfile3.txt|66
SHARE/1th/2th/3th/testfile2.txt|66

#after full scan, now have correct sizes:
SHARE/1th|297
SHARE/1th/2th|198
SHARE/1th/2th/3th|198
SHARE/1th/2th/3th/4th|99
SHARE/1th/2th/3th/4th/5th
SHARE/1th/2th/3th/4th/5th/testfile1.txt|99
SHARE/1th/testfile3.txt|99
SHARE/1th/2th/3th/testfile2.txt|99

[ariselseng]

@icewind1991 @MorrisJobke
Sorry for the spam here.

FINALLY I got to the bottom of this.
When I make getIncomplete() in /lib/private/Files/Cache/Cache.php return an array with all entries with -1 as size instead of just 1, and then for loop over each one in backgroundScan() in /lib/private/Files/Cache/Scanner.php, it works fine!

I got the hint here where there was a "FIXME" line: https://github.com/nextcloud/server/blob/master//lib/private/Files/Cache/Scanner.php#L523

It seems that for some reason the sorting of getIncomplete makes a difference. I believe this bug only happens when the entry with the highest fileid is further down in the tree then the other changes.
After backgroundScan() has processed the folders with -1 size that was not returned in getIncomplete() suddenly has its old wrong size back.

@icewind1991 I am not sure if making getIncomplete() returning an array is the right solution to all this. I am willing to help if you have any thoughts. If you are not convinced this is not a bug or I am not making myself understood clearly, please tell :)

[ariselseng]

Okay so the bug only presents it self if there is more than folder with size -1 + that they are in the same tree + that the folder created last (highest id) and is the one deepest into the tree.

Like test1/test.txt and test1/subfolder/test2.txt
Where "subfolder" has a higher id than "test1".

A quick change of ordering in getIncomplete() fixes the problem it seems.
@icewind1991 @MorrisJobke Is this a safe fix? Is the "order by fileid DESC" worth it?

diff --git a/Cache.php b/nextcloud/lib/private/Files/Cache/Cache.php
index 43e48e5..7d5ed90 100644
--- a/Cache.php
+++ b/nextcloud/lib/private/Files/Cache/Cache.php
@@ -841,7 +841,7 @@ class Cache implements ICache {
         */
        public function getIncomplete() {
                $query = $this->connection->prepare('SELECT `path` FROM `*PREFIX*filecache`'
-                       . ' WHERE `storage` = ? AND `size` = -1 ORDER BY `fileid` DESC', 1);
+                       . ' WHERE `storage` = ? AND `size` = -1 ORDER BY `path` ASC', 1);
                $query->execute([$this->getNumericStorageId()]);
                if ($row = $query->fetch()) {
                        return $row['path'];

[MorrisJobke]

@icewind1991 @MorrisJobke Is this a safe fix? Is the "order by fileid DESC" worth it?

Looking at the code it shouldn't make a difference. But maybe @icewind1991 knows more?

[ariselseng]

There could also be a potential fix related to and in calculateFolderSize() if it somehow did not calculate size for parent folders more than 1 level up, that also had size -1. That is the core of the issue but I am not sure how to fix that. Just returning a sorted result in getIncomplete() where top most folder is returned it also avoids the issue with wrong calculated folder sizes.

[icewind1991]

I think I remembered the reasoning behind the DESC, if it uses ASC it means that when the background scanner runs, it's will run on the top most folder that is marked incomplete (since child id's are generally higher that the parent one), this is also likely the folder that will take the longest to scan.

sorting DESC makes it so we run the background scanner in the smallest "chunks" possible.

Now I'm not sure if this is still a relevant "advantage", I remember there being logic somewhere that ran the background scanner one "chunk" at a time, but I can't find it so it might have been removed.

I think switching to ASC will be safe to try for 16

[ariselseng]

@theroch
Care to checkout these changes? And do a proper test to try and get sync issues? Like do changes and check with rsync --stats --dry-run --checksum or something.

A couple of PRs here will probably help for the issues listed in this thread:

#14425 (Fixes where some folders to not get scanned properly)
#14456 (Fixes where no folders get scanned)

@MorrisJobke
@icewind1991

There is still also this issue that makes the very first registered change in occ files_external:notify not be registered: #14441

With all these bugs it makes me wonder if I am the only one actually using the SMB share outside of Nextcloud :D

[klada]

To avoid running into performance issues we definitely need a db index on path, which is used in the ORDER BY clause.

[ariselseng]

@klada You're right.
I have updated my PR to avoid this though. #14425

[theroch]

@cowai I'will try your changes this week.