Federation timeout to low to getShareInfo() at big Federation link

[rinal13]

At big federation links (11,3 GB / 6653 files / 170 folders) the timeout set at
(/var/www/nextcloud/apps/files_sharing/lib/External/Storage.php)
public function getShareInfo()
of 10 seconds is too low. It takes approx. 20 seconds to get ShareInfo.
After timeout increased to 30 seconds, Federation is working slow but working.

According the error log the data transmitted is only 269393 bytes.
Why it takes so long I can't judge, as I don't know.

[patagonaa]

I also ran into this issue today, I tried to do a federated share with 178GB / 70.000 files and also ran into timeouts (and high CPU/Memory usage on the instance where the files are hosted).
I've have had a quick look at the code and it seems like the federation ShareInfo endpoint seems to be a bit insane as it tries to iterate through all items (files and directories) in a shared folder:

server/apps/files_sharing/lib/Controller/ShareInfoController.php

Lines 102 to 124 in ad6e975

	private function parseNode(Node $node, int $permissionMask) {
	if ($node instanceof File) {
	return $this->parseFile($node, $permissionMask);
	}
	return $this->parseFolder($node, $permissionMask);
	}
	private function parseFile(File $file, int $permissionMask) {
	return $this->format($file, $permissionMask);
	}
	private function parseFolder(Folder $folder, int $permissionMask) {
	$data = $this->format($folder, $permissionMask);
	$data['children'] = [];
	$nodes = $folder->getDirectoryListing();
	foreach ($nodes as $node) {
	$data['children'][] = $this->parseNode($node, $permissionMask);
	}
	return $data;
	}

This doesn't work with large shares (obviously) and because the "receiving" instance seems to retry this request if it fails, the "sending" instance is effectively unusable until the receiving instance is stopped or the share is unshared.

[rinal13]

May the recursion depth can be limited to the next useful folder level to improve this?
How it's made with i.e external SMB shares, or is it a general issue?

[nickvergessen]

From our instance, requests of a single user on a single share within the first 12h of the day:

 Hits       h% Vis.      v% Tx. Amount Mtd      Proto    Data
 ----- ------- ---- ------- ---------- -------- -------- ----
 51010 100.00%    1 100.00% 361.06 MiB GET      HTTP/1.1 /status.php
 25503  50.00%    1 100.00% 262.36 MiB POST     HTTP/1.1 /index.php/apps/files_sharing/shareinfo?t=…
 23316  45.71%    1 100.00% 110.62 MiB PROPFIND HTTP/1.1 /public.php/webdav/
 15931  31.23%    1 100.00% 123.44 MiB GET      HTTP/1.1 /ocs-provider/
  3828   7.50%    1 100.00%   8.05 MiB PROPFIND HTTP/1.1 /public.php/webdav/Level1-1/Level2-1
  3828   7.50%    1 100.00%   8.09 MiB PROPFIND HTTP/1.1 /public.php/webdav/Level1-2/Level2-2
  1740   3.41%    1 100.00%   3.65 MiB PROPFIND HTTP/1.1 /public.php/webdav/Level1-2/Level2-3
  1706   3.34%    1 100.00%   3.56 MiB PROPFIND HTTP/1.1 /public.php/webdav/Level1-1
  1044   2.05%    1 100.00%   2.20 MiB PROPFIND HTTP/1.1 /public.php/webdav/Level1-1/Level2-4
  1044   2.05%    1 100.00%   2.22 MiB PROPFIND HTTP/1.1 /public.php/webdav/Level1-1/Level2-5
   696   1.36%    1 100.00%   1.46 MiB PROPFIND HTTP/1.1 /public.php/webdav/Level1-1/Level2-6
   696   1.36%    1 100.00%   1.47 MiB PROPFIND HTTP/1.1 /public.php/webdav/Level1-2/Level2-7
   348   0.68%    1 100.00% 746.91 KiB PROPFIND HTTP/1.1 /public.php/webdav/Level1-2/Level2-8
   174   0.34%    1 100.00%   1.23 MiB GET      HTTP/1.1 /public.php/webdav/Level1-1/Level2-9

[PVince81]

maybe the code should stop using the "shareinfo" which retrieves the whole cache at once and switch to PROPFIND with Depth: 1, and do multiple calls there.

having multiple calls might be a bit slower but would prevent the remote to time out

I'm not sure if having just snippets of the remote cache would work correctly.
@icewind1991 do you remember why we had to resort to a full transfer back then instead of relying on the default scanner behavior in https://github.com/nextcloud/server/blob/master/apps/files_sharing/lib/External/Scanner.php#L43 ?

also, slightly related: here we're querying the whole thing but discarding the body: https://github.com/nextcloud/server/blob/master/apps/files_sharing/lib/External/Storage.php#L216
the latter could be optimized by passing in an additional arg to shareinfo to tell that we only want to check availability and are not interested in the cache entries (if we are to keep "shareinfo" as a mechanism for scanning)

[patagonaa]

@PVince81 I assume (I haven't looked into the code for this) this was done for performance reasons as it is very likely a lot faster (and less CPU-intensive) if you have more than a few (but not a huge number of) directories.

So in that case (say 100 directories with 100 files each) doing everything in a single request and letting the remote server do the scanning is very likely a lot faster than doing a request per directory.

Now, if you have 1000 directories with 1000 files each, that's a different story, and scanning and transferring everything in one go is too much.

The fastest way would probably be something inbetween. So if the directory has say, less than 10 000 entries, the entire thing could be transferred at once and if there where more than 10 000 entries ShareInfo would signal that (either by a specific error code or by returning only a subset of the requested data, maybe only a certain depth?) and the scanner would scan each directory in a seperate request. This would likely add a lot of complexity though, so it would probably be better if something simpler also worked.

[PVince81]

here's a PR that removes the call to shareinfo: #30499

I've tested it and it worked fine with a simple tree (not massive), so functionally it should be fine.

this doesn't address https://github.com/nextcloud/server/blob/master/apps/files_sharing/lib/External/Storage.php#L216 which needs more changes on both sides of the endpoint