-
-
Save p/8b8e98d22dd9c9b2ea31 to your computer and use it in GitHub Desktop.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
(15:54:33) bantu: nn-: you fixed some locking problem with flock() we had in 3.0.x earlier, right? | |
(15:54:48) bantu: have you considered just switching to db locking? | |
(15:56:14) nn-: it's on the todo list i'm sure | |
(15:56:49) nn-: i suppose we could do db locking | |
(15:56:55) nn-: abuse config table for it | |
(15:57:05) bantu: we do have a lock_db class in 3.1.x | |
(15:57:09) nn-: do you want it in 3.0? | |
(15:57:22) bantu: I'm not sure. | |
(15:57:25) nn-: hmm really, what does that do | |
(15:57:33) bantu: nn-: uses config table ;-) | |
(15:58:28) nn-: says here it was done for cron | |
(15:58:38) bantu: yup, probably | |
(15:58:49) bantu: but it's reusable | |
(15:59:26) bantu: You apperently knew that there can be problems with flock (e.g. on NFS). | |
(15:59:39) bantu: So that's why I am wondering whether you considered using the DB for locking. | |
(15:59:52) nn-: i think that was the plan long term | |
(15:59:58) nn-: i'm sure it was written down somewhere | |
(16:00:04) bantu: oh okay | |
(16:00:07) bantu: guess that makes sense | |
(16:00:12) nn-: for queue we're doing it this way because it is currently done that way | |
(16:00:21) nn-: also, the queue change predates cron | |
(16:02:05) bantu: well yeah, that might be the case | |
(16:02:07) nn-: this lock db class looks prettyp fancy | |
(16:02:25) nn-: if it works we should probably use it :) | |
(16:02:27) bantu: as far as I remember the changes for flock were quite complicated | |
(16:02:52) bantu: so we must have considered using db for locking at that time | |
(16:03:06) nn-: cron ought to lock per task | |
(16:03:18) nn-: having that many lock files would have been complicated | |
(16:03:55) nn-: not to mention they don't actually work in all environments | |
(16:03:56) bantu: Isn't it just one big lock for cron right now? I don't know | |
(16:04:04) nn-: i hope not | |
(16:04:26) nn-: * @param string $config_name A config variable to be used for locking | |
(16:04:43) nn-: ah | |
(16:04:53) bantu: as for db locking for queue, I'd say 3.1 is fine | |
(16:05:02) nn-: so we still have my proposal of adding a flag file in 3.0 if we stay with flock | |
(16:05:28) nn-: hmm | |
(16:05:32) nn-: phpBB/cron.php:$cron_lock = new phpbb_lock_db('cron_lock', $config, $db); | |
(16:06:38) bantu: so global locking | |
(16:06:42) nn-: yep | |
(16:06:47) nn-: lock db uses __construct | |
(16:07:01) nn-: does php4 have private and friends? | |
(16:07:07) bantu: no | |
(16:07:22) bantu: it does also not have the ocnfig class | |
(16:07:32) bantu: nn-: is there an actual problem with the queue code in 3.0. | |
(16:07:39) bantu: it seems to be owrking fine for most people | |
(16:07:58) nn-: you need to have queue interval set to exceed the interval between message submissions | |
(16:08:39) nn-: if you have periods when that is not true queued mail will be sent, albeit with a delay | |
(16:08:57) nn-: on a very active forum with a long-ish queue interval this should break | |
(16:09:04) nn-: this is all based on reading the code, i haven't tested it | |
(16:10:43) nn-: the fix for 3.0 should be 10 lines of code or so | |
(16:11:22) nn-: 1 file existence check, 1 mtime check, 1 fopen/touch and 1 unlink | |
(16:12:13) nn-: another 5 lines if we want to be careful with the unlinking and try to only unlink the lock file that we created | |
(16:12:16) nn-: or rather the flag file | |
(16:17:12) nn-: for 3.1 we should use lock_db for queue | |
(16:17:34) nn-: this should actually be rather straightforward | |
(16:19:32) bantu: nn-: sounds good to me |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment