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

Set recommend opcache.revalidate_freq #1718

Merged
merged 1 commit into from
Mar 23, 2022

Conversation

J0WI
Copy link
Contributor

@J0WI J0WI commented Mar 23, 2022

No description provided.

Signed-off-by: J0WI <J0WI@users.noreply.github.com>
@J0WI J0WI merged commit d27ec71 into nextcloud:master Mar 23, 2022
@J0WI J0WI deleted the opcache.revalidate_freq branch March 23, 2022 11:27
docker-library-bot added a commit to docker-library-bot/official-images that referenced this pull request Mar 23, 2022
Changes:

- https///github.com/nextcloud/docker/commit/fb33f15: Runs update.sh
- https///github.com/nextcloud/docker/commit/d27ec71: Set recommend opcache.revalidate_freq (https///github.com/nextcloud/docker/pull/1718)
- https///github.com/nextcloud/docker/commit/7d6837d: Remove Nextcloud 21 (EOL) (https///github.com/nextcloud/docker/pull/1719)
- https///github.com/nextcloud/docker/commit/04d461e: Runs update.sh
- https///github.com/nextcloud/docker/commit/6478d4d: Increase (double) opcache string buffer size (https///github.com/nextcloud/docker/pull/1702)
- https///github.com/nextcloud/docker/commit/df168e1: Runs update.sh
- https///github.com/nextcloud/docker/commit/3a5086d: Runs update.sh
- https///github.com/nextcloud/docker/commit/76b7969: Runs update.sh
- https///github.com/nextcloud/docker/commit/3fd3a40: Runs update.sh
- https///github.com/nextcloud/docker/commit/13eb696: doc: fix link to OpCache server doc (https///github.com/nextcloud/docker/pull/1703)
@te-online
Copy link

te-online commented Jul 5, 2022

Hey @J0WI 🙂

Can you put a few words to how this change is beneficial? I have no expertise with opcache. The only thing I can say is that I see some issues in regular usage. I can't find a related issue here on Github?

Take updating an app as an example: It can take up to 60 seconds until all file changes have settled. In this timeframe we can get weird results from php files being cached. This happens to me on an Ubuntu 22.04 system.

It's especially more noticeable when doing app development with the container: I need to wait up to 60 seconds after I saved my php file until php picks up the change. This happens to me using macOS 12.

I've made a custom Dockerfile overriding this change back to 1 and I can't see any obvious performance penalty, just php files being updated immediately.

Would be happy to learn if I'm doing something wrong here or if I can do anything to improve things 😊

@te-online
Copy link

te-online commented Jul 15, 2022

If I look at this comment, my understanding is that "the admin" in this case is you in extension of the docker-image users, pre-defining the config for many people using the docker image.

If the admin knows what it does then we are fine with a different value. [different then 1, that is]

With my limited knowledge, using the 60 value is not a good or bad option, same as 1 is not inherently bad. For me with a pretty average Nextcloud instance it causes issues and does not increase performance. If there is an internal cache invalidation, it does not work. I have to wait 60 seconds after app updates for them to take effect. In the meantime the log might be flooded with errors, because frontend and backend won't necessarily match.

You might disagree with 1 as a value and I can live with that, but I just want to advertise for reverting this change as it seems unnecessary to me and seems only recommended if you like to have it set to this value and you – as an admin – are aware of the effects and can take them into account when maintaining your Nextcloud instance.

@aligator
Copy link

@te-online
Thanks for that info, I will change that value to 1 in my installation to prevent future problems like I experienced with the timemanager app.
I think this "optimization" is only really beneficial for bigger nextcloud installations...

pathob pushed a commit to pathob/nextcloud-docker that referenced this pull request Jan 14, 2024
Signed-off-by: J0WI <J0WI@users.noreply.github.com>
ananace pushed a commit to ananace/docker-nextcloud that referenced this pull request May 10, 2024
Signed-off-by: J0WI <J0WI@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants