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

Fix #10427 - APC does not work for SuiteCRM 7.14, PHP8 #10428

Open
wants to merge 1 commit into
base: hotfix
Choose a base branch
from

Conversation

gunnicom
Copy link
Contributor

See #10427

@SuiteBot
Copy link

This pull request has been mentioned on SuiteCRM. There might be relevant details there:

https://community.suitecrm.com/t/apc-cache-not-working/69261/2

@simonr44
Copy link
Contributor

Hi Gunnicom,

Thanks for contributing to SuiteCRM.

I have tested your changes and they work well.

Having looked at the code, there are some related changes that may be worthwhile.

  • apc_clear_cache is used in include/SugarCache/SugarCache.php around lines 119
  • apc_delete_file is used in include/SugarCache/SugarCache.php around lines 157

The check && ini_get('apc.stat') == 0 may no longer be required on the above also?

In addition it may be worth lowering the priority in include/SugarCache/SugarCacheAPC.php line 49
Changing protected $_priority = 940; to protected $_priority = 890; will ensure APCu is prioritised over other caches that may be available but not fully configured such as memcached.

@gunnicom
Copy link
Contributor Author

gunnicom commented May 16, 2024

@simonr44 At first I thought about the file include/SugarCache/SugarCache.php, too, but on second look I assumed, that that is only for OPcache part of APC, that vanished long ago. So I decided to not include that here.
The real solution would be to remove that part completely.

@simonr44
Copy link
Contributor

Agreed yes APCu no longer includes opcode caches, however clearing the APCu cache when this function is called may still have benefits. Perhaps the function name is no longer too relevant but it may still have value in one form or another.

@simonr44
Copy link
Contributor

On further inspection apc_delete_file looks obsolete now, so 157 - 159 could go?

apcu_clear_cache() may still be worth calling in cleanOpcodes()

@gunnicom
Copy link
Contributor Author

@simonr44 That could all be discussed in a different issue/PR. Keep this PR to fixing APCu functionality, and leave cleanup of old obsolete APC OPcache to a different PR.

@simonr44
Copy link
Contributor

There is a good chance we would look to bring these changes in together and test together so we'll make a new PR to cover those.

Thanks again for your contribution.

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