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

[Bug]: Disabling the preview does not stop the creation of the folder tree #45697

Closed
4 of 8 tasks
devsamt opened this issue Jun 6, 2024 · 5 comments · Fixed by #45866
Closed
4 of 8 tasks

[Bug]: Disabling the preview does not stop the creation of the folder tree #45697

devsamt opened this issue Jun 6, 2024 · 5 comments · Fixed by #45866
Labels

Comments

@devsamt
Copy link

devsamt commented Jun 6, 2024

⚠️ This issue respects the following points: ⚠️

Bug description

I am encountering an issue: my NextCloud instance is installed on an ext4 partition with limited inode resources. The preview feature generates a very large number of folders that consume all available inodes and block the entire system. Despite disabling the preview feature in the configuration file, this folder tree continues to be created empty. Could you please fix the problem quickly?

For more information:
https://help.nextcloud.com/t/the-preview-feature-consumes-too-many-inodes/194129

Steps to reproduce

  1. sudo -u www-data php /var/www/nextcloud/occ files:scan-app-data
  2. df -i /dev/sda1
  3. systemctl stop apache2
  4. rm -rf /var/www/nextcloud/data/appdata_xxx/preview/*
  5. sudo -u www-data php /var/www/nextcloud/occ files:scan-app-data
  6. df -i /dev/sda1
  7. systemctl start apache2

Expected behavior

Disabling the preview feature stops the creation of all files and folders, not just files.

Installation method

None

Nextcloud Server version

29

Operating system

Debian/Ubuntu

PHP engine version

None

Web server

None

Database engine version

None

Is this bug present after an update or on a fresh install?

None

Are you using the Nextcloud Server Encryption module?

None

What user-backends are you using?

  • Default user-backend (database)
  • LDAP/ Active Directory
  • SSO - SAML
  • Other

Configuration report

No response

List of activated Apps

No response

Nextcloud Signing status

No response

Nextcloud Logs

No response

Additional info

No response

@devsamt devsamt added 0. Needs triage Pending check for reproducibility or if it fits our roadmap bug labels Jun 6, 2024
@kesselb kesselb added 1. to develop Accepted and waiting to be taken care of and removed 0. Needs triage Pending check for reproducibility or if it fits our roadmap labels Jun 6, 2024
@kesselb
Copy link
Contributor

kesselb commented Jun 6, 2024

True.

With enabled_previews = false, no preview providers are registered and therefore not previews are generated. But currently some code before, like requesting a generator instance, creating the folder structure, BeforePreviewFetchedEvent still runs.

The patch below should improve it.

Wdyt @st3iny @come-nc?

Index: lib/private/PreviewManager.php
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/lib/private/PreviewManager.php b/lib/private/PreviewManager.php
--- a/lib/private/PreviewManager.php	(revision 858ba27835ed3e71ad34c9cc933467e98db14adf)
+++ b/lib/private/PreviewManager.php	(date 1717704971130)
@@ -50,6 +50,7 @@
 	private IServerContainer $container;
 	private IBinaryFinder $binaryFinder;
 	private IMagickSupport $imagickSupport;
+	private bool $enablePreviews;
 
 	public function __construct(
 		IConfig                  $config,
@@ -73,6 +74,7 @@
 		$this->container = $container;
 		$this->binaryFinder = $binaryFinder;
 		$this->imagickSupport = $imagickSupport;
+		$this->enablePreviews = $config->getSystemValueBool('enable_previews', true);
 	}
 
 	/**
@@ -86,7 +88,7 @@
 	 * @return void
 	 */
 	public function registerProvider($mimeTypeRegex, \Closure $callable): void {
-		if (!$this->config->getSystemValueBool('enable_previews', true)) {
+		if (!$this->enablePreviews) {
 			return;
 		}
 
@@ -101,7 +103,7 @@
 	 * Get all providers
 	 */
 	public function getProviders(): array {
-		if (!$this->config->getSystemValueBool('enable_previews', true)) {
+		if (!$this->enablePreviews) {
 			return [];
 		}
 
@@ -158,6 +160,7 @@
 	 * @since 11.0.0 - \InvalidArgumentException was added in 12.0.0
 	 */
 	public function getPreview(File $file, $width = -1, $height = -1, $crop = false, $mode = IPreview::MODE_FILL, $mimeType = null) {
+		$this->throwIfPreviewsDisabled();
 		$previewConcurrency = $this->getGenerator()->getNumConcurrentPreviews('preview_concurrency_all');
 		$sem = Generator::guardWithSemaphore(Generator::SEMAPHORE_ID_ALL, $previewConcurrency);
 		try {
@@ -181,6 +184,7 @@
 	 * @since 19.0.0
 	 */
 	public function generatePreviews(File $file, array $specifications, $mimeType = null) {
+		$this->throwIfPreviewsDisabled();
 		return $this->getGenerator()->generatePreviews($file, $specifications, $mimeType);
 	}
 
@@ -191,7 +195,7 @@
 	 * @return boolean
 	 */
 	public function isMimeSupported($mimeType = '*') {
-		if (!$this->config->getSystemValueBool('enable_previews', true)) {
+		if (!$this->enablePreviews) {
 			return false;
 		}
 
@@ -216,7 +220,7 @@
 	 * Check if a preview can be generated for a file
 	 */
 	public function isAvailable(\OCP\Files\FileInfo $file): bool {
-		if (!$this->config->getSystemValueBool('enable_previews', true)) {
+		if (!$this->enablePreviews) {
 			return false;
 		}
 
@@ -452,4 +456,13 @@
 			});
 		}
 	}
+
+	/**
+	 * @throws NotFoundException if preview generation is disabled
+	 */
+	private function throwIfPreviewsDisabled(): void {
+		if (!$this->enablePreviews) {
+			throw new NotFoundException('Previews disabled');
+		}
+	}
 }

@st3iny
Copy link
Member

st3iny commented Jun 7, 2024

@kesselb I like it. I also tested it locally and it works fine. Do you mind opening a PR?

@didierm
Copy link

didierm commented Jun 7, 2024

I applied the patch to 29.0.1, and it works beautifully.

We were hit by this in our use case :
small (< 2GB) nextcloud data volumes (no local storage, many TBs of external (local network) storage), filling up with inodes (512 bytes/inode).

Thanks !

@pbuergi
Copy link

pbuergi commented Jun 10, 2024

same problem here. Is there a fix coming for normal patching? (stable)

@susnux
Copy link
Contributor

susnux commented Jun 13, 2024

@kesselb would you mind creating a PR from your patch?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants