The 2026 Drupal Business Survey is open. Your response will help shape the future of the Drupal business ecosystem.I had a serious bug, all PHP threads where on my system right after I raised manually the PHP CLI memory limit.
After commenting your two lines that raise the memory limit, it worked fine.
You should let an option to ensure that site admin can set their own memory limit themself, which disable Datasync module to set it.
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | 536512_memory_limit.patch | 593 bytes | andrewlevine |
| #1 | datasync-6.x-1.0-alpha3-php_memory_limit.patch | 2.63 KB | pounard |
Comments
Comment #1
pounardDid a simple patch, file attached.
Comment #2
andrewlevine commentedI'm not sure I understand exactly what went wrong and why the patch fixes it. The patch doesn't seem to do anything different except add a textfield to select a memory limit?
Appreciate your help.
Comment #3
pounardThe fact is, on my system, when I let DataSync do its ini_set stuff itself, PHP threads goes to . It's a really strange bug, I think linked with my environment.
Beside, I think DataSync should not set the ini_set() if the site admin don't want to.
The patch just skip the ini_set() in case the user let a blank value. It also give a tuning option to site admin.
Comment #4
andrewlevine commentedI agree that ini_set should not be called unnecessarily. By default right now it just sets the memory_limit to the current memory limit. I'm not sure it deserves a spot in the interface though (it is settable by changing the variable in the datasync_variable table). If anything it might be in the host configuration. What do you think of this patch?
Comment #5
pounardRight, but why do you use a variable if no one can set it?
Comment #6
andrewlevine commentedIn this patch the variable can be set by calling ds_variable_set('datasync_run_memory_limit', $bytes); Unless you meant that no one will know to set it. The reason I'm hesitant to expose this in the interface is that it is a system-wide setting when it really should be a per-host setting. What do you think?
Comment #7
pounardHmm, it's up to you. I think you just should get rid of this piece of code, in my company, sysadmin will say "Let the admin do its job", which is not wrong. They also could say "the sysadmin should never have to click on the frontend".
So I think this setting should only relies on the system.
Furthermore, there is chances that a lot of systems are configured to disallow ini_set() in php code (just like suhosin patches for exemple), it should not be the application which control the system.
It's only an opinion, you should ask some more people to be sure.
Comment #8
andrewlevine commentedHey. Removing this code seems to make sense to me for now. Might add it back to the host configuration at a later time. I also added a note about the memory limit in INSTALL.txt. Thanks for bringing the issue up.
http://cvs.drupal.org/viewvc.py/drupal/contributions/modules/datasync/da...