feat: Forward sizeDifference on update

[artonge]

And remove unneeded oldSize hack.

Reason

This will speed up update operations on S3 as primary storage by preventing the recomputation of the size of each folder higher up in the hierarchy.

Rational

When using S3 as primary storage, the Scanner is replaced with the ObjectStoreScanner. This means that we are not able to benefit from the oldSize property set in the Scanner.

• Scanner sets the oldSize property on the cache object:
  

  server/lib/private/Files/Cache/Scanner.php

  Line 248 in 2152650

  $data['oldSize'] = ($cacheData && isset($cacheData['size'])) ? $cacheData['size'] : 0;

• Updater tries to access the oldSize property
  

  server/lib/private/Files/Cache/Updater.php

  Lines 134 to 136 in 2152650

  	if (isset($data['oldSize']) && isset($data['size'])) {
  	$sizeDifference = $data['size'] - $data['oldSize'];
  	}

• ObjectStoreScanner just does nothing:
  

  server/lib/private/Files/ObjectStore/ObjectStoreScanner.php

  Lines 38 to 40 in 2152650

  	public function scan($path, $recursive = self::SCAN_RECURSIVE, $reuse = -1, $lock = true) {
  	return [];
  	}

[come-nc]

Do we have tests for this?

[artonge]

Do we have tests for this?

No, I would like to validate the approach with @icewind1991 first.

[icewind1991]

Does this even work? Since the storage operation (with object store) would have already updated the filecache, the size you get in the new View::basicOperation logic would be the new size, not the old one. So I would expect the calculated difference to always be 0 here.

Also, this would only make the sizeDifference available to calls trough the View::basicOperation code path, where the current approach will work for all calls to Updater::update.

It will also add an additional filecache read for every file change.

[icewind1991]

I'm tempting to have the object store do more of the filecache management "in house" and also have it perform the size/etag propagation when it writes to the filecache

[icewind1991]

this needs a better approach

[tobiasKaminsky]

@icewind1991 @artonge what is the status here?
Have Robin's remarks been resolved?

[artonge]

Summary from the discussion with Robin:

Overall, the issue is that the ObjectStoreScanner does not return the data needed by the Updater to use the oldSize optimisation. This is because ObjectStoreScanner->scan*() methods have nothing to scan, so they return null.

One solution would be to create an ObjectStoreUpdater and to return it in ObjectStoreStorage->getUpdater()
That new updater would mostly do nothing as ObjectStoreStorage already does a lot of cache management by itself.

[tobiasKaminsky]

@artonge what does this now mean

• dev-wise
• time-wise

I read it like that the PR is not working, but will the solution work? And how long would it need to implement?

[artonge]

I should have something working before the 20th of June, if not, Robin will take over.