Skip to content

Conversation

@MichaIng
Copy link
Member

@MichaIng MichaIng commented Jan 7, 2022

Fixes: #30532

@MichaIng MichaIng added bug 3. to review Waiting for reviews labels Jan 7, 2022
@MichaIng MichaIng added this to the Nextcloud 24 milestone Jan 7, 2022
@MichaIng MichaIng requested a review from solracsf January 7, 2022 18:43
@MichaIng
Copy link
Member Author

MichaIng commented Jan 7, 2022

/backport to stable23

@solracsf
Copy link
Member

solracsf commented Jan 7, 2022

Shouldn't it be !empty ? 🤔
Otherwise, you check if values are empty an then outputs a recommandation that they are full.

@MichaIng
Copy link
Member Author

MichaIng commented Jan 7, 2022

It's the free memory and the max cached keys (not the used memory/keys). In both cases, when they are null, nothing can be cached anymore, hence the warning as a performance recommendation makes sense. Not sure whether any max value can be actually set to 0 without disabling OPcache all together. However, it doesn't change something about the recommendation to always have 10% left free buffer, for interned strings, keys and overall OPcache size.


Did some tests:

  • opcache.memory_consumption cannot be set to 0, it will then use the minimum of 8 MiB.
  • Same with opcache.max_accelerated_files, where the minimum of 223 cached keys is applied then.
  • opcache.interned_strings_buffer=0 however is possible, which indeed disables the internet strings buffer completely so that $status['interned_strings_usage'] is not defined at all, which is probably the case with your setup @nursoda? It is now covered by this change.

Drone failure is unrelated.

@MichaIng MichaIng added the high label Jan 7, 2022
@MichaIng
Copy link
Member Author

MichaIng commented Jan 7, 2022

Another thing: Is there probably a flag for one of the CI checks/tools to fail on division by unverified variables?

@szaimen szaimen requested review from a team, PVince81, come-nc and skjnldsv and removed request for a team January 8, 2022 08:10
@solracsf solracsf removed their request for review January 8, 2022 14:53
@MichaIng MichaIng force-pushed the fix/avoid-zero-division branch from 4cfba89 to 21829d6 Compare January 10, 2022 13:52
@MichaIng MichaIng requested a review from come-nc January 10, 2022 13:53
@MichaIng
Copy link
Member Author

jsunit failure is unrelated, same as here: #30557

@szaimen
Copy link
Contributor

szaimen commented Jan 11, 2022

/rebase

@MichaIng MichaIng merged commit 796764a into master Jan 11, 2022
@MichaIng MichaIng deleted the fix/avoid-zero-division branch January 11, 2022 12:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Division by zero in file 'apps/settings/lib/Controller/CheckSetupController.php' line 503

5 participants