TYPO3 Core

I'm a colleague of Justus.
We have commented the source code out, and the fileadmin still goes offline.

We are still investigating whether something from outside of TYPO3 generates the issue, as we can't find any other place inside of TYPO3 that can do the trick.

We have the same problem with all storages, not only fileadmin, using TYPO3 6.2.19.
There are no server logs indicating access errors to fileadmin directory.
Waiting for more information, we had to make a task to set all storages online recurrently.
I wonder why storages can't be set online automatically when they are offline.

Patch set 2 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/49798

Patch set 3 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/49798

Patch set 4 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/49798

Patch set 5 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/49798

Patch set 6 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/49798

change has been reverted here: https://review.typo3.org/#/c/50192/

Is it by intention in typo3/sysext/core/Classes/Resource/ResourceStorage.php public function isOnline() that $this->isOnline is SET in this method (under some circumstances)?

Could that be a possible cause for this "offline" problem?

IMHO the isOnline() function should only return true/false but never manipulate an internal variable. Is there some "hidden magic" why this is done?

Any ideas how to cover this issue with some tests?

Patch set 2 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/50371

IMO the "feature" that the ResourceStorage sets the storage automatically permanent offline doesn't make sense.
Also temporary disabling doesn't make sense for the LocalDriver provided by the core as it is just a single file system call that determine the storage isn't available.

In general just look at it like our main DB connection. You also don't want the DB to be marked offline by 1 glitch, not even for a few seconds.

Sure there are drivers where it makes sense to disable it for a few seconds/minutes to not DDOS a remote service. But the only part of FAL that can now such a requirement is the Driver. So if such a behaviour is desired by a Driver this should by handled by the driver as only the driver knows what timeouts are reasonable for it type. This could for instance by implemented in the processConfiguration() method of the driver.

I propose the following changes:

1. Remove the whole "automatically" permanent disabling of a storage by the ResourceStorage but only set it offline for current request
   so call markAsTemporaryOffline() instead of markAsPermanentlyOffline() and let it only set $this->isOnline = false;
2. Add a check to the BE form to check if the storage is online after save
   Similar check like is done for is_public in \TYPO3\CMS\Core\Resource\Service\UserStorageCapabilityService.
   Create a userFunc renderIsOnline that checks if this storage is temporary offline and if so disables the checkbox is_online and presents the admin with a flash message like is done for is_public
   Maybe build the driver like is done in the ResourceStorage constructor and to catch and display the exception in the flashmessage
3. Deprecate ResourceStorage::markAsPermanentlyOffline()
4. Cleanup ResourceStorage to make the whole online check better readable

This should be 2 different patches. 1 with the first 2 points with a backport to 7.6, a second with the cleanup.

I second 1), 3) and 4).

Regarding 2)... if we disable the checkbox is_online we have the same issue as now; in fact ResourceStorage::markAsPermanentlyOffline() does exact this: uncheck the is_online checkbox and cause several trouble with this.

For local resources IMHO we don't need any temporary or even permanent "offline mode" as the check for availability is really cheap. For remote resources this may be different.

Quick test earlier today caused some errors in the backend if the (unavailable) resource isn't marked as offline at all; I'll spend some testing time to isolate this.

If you look at \TYPO3\CMS\Core\Resource\Service\UserStorageCapabilityService you see that for is_public only the checkbox is unchecked. This value is only persisted when you save the form again. So it still isn't really disabled automatically. But the flashmessage is the important part here. That will tell the admin what is going on so he can react on it.

So the is_online value is only persisted by action of the admin, not by just visiting the backend

Patch set 3 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/50371

Hmm. I'm quite not really happy with this solution; as this would also introduce an UI/UX-change (which might to be discussed to be useful or not; but I would like to see this discussion in another ticket to do not block this (stand-alone) bugfix, which is likely overdue.

I have pushed another patchset https://review.typo3.org/#/c/50371/ to be reviewed; in this patchset the resource storage is neither set offline temporary or permanent; so some flaky resources are back in the system as soon they are reachable again; this is important esp. in NFS-scenarios, where some files(ystems) are sometimes for short time not available (and the old code may hit such a timeframe and set the resource offline forever).

I would like to see a discussion on the given patch; as follow-up to these patch (if accepted) there should be some cleanup, esp. deprecation of the markAsTemporaryOffline() and markAsPermaentOffline() methods as they are no longer used in the core (and likely not in any other extension).

If some drivers needs to set resource storages offline for more than it's really not available, this behavior should be configurable in the driver and/or the resource storage; the actual standard is not suitable really good for most cases (and therefore abandoned in the named patch).

Also to be discussed in the follow-up discussion to this patch is the UI/UX behavior which should occur - IMHO there is not even one reason to set a storage offline except the admin does this especially for maintenance reasons (which works). A useful reaction to a non-reachable resource could be a flash-message, an email to an administrator, an entry into syslog or a combination of these. But I think, we should reduce complexity for the bugfix issue and not increase it by adding additional functionality (which of course can be done in another ticket).

Patch set 4 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/50371

Patch set 5 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/50371

Patch set 6 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/50371

Patch set 7 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/50371

Patch set 8 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/50371

Patch set 9 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/50371

Patch set 10 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/50371

Patch set 11 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/50371

Patch set 1 for branch TYPO3_7-6 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/50816

Patch set 1 for branch TYPO3_7-6 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/50837