Hi all,
I have decided to put a post here concerning the work I have completed on an amendment/upgrade to the forum module that would permit subscription to individual discussions, rather than the entire forum.
This development adds three new options to the force-subscribe drop-down box:
4. Allow thread or forum subscriptions
5. Automatic thread-based subscriptions
6. Force thread-based subscriptions
And has been documented in http://moodle.org/mod/forum/discuss.php?d=115149 and in the tracker at MDL-1626, where the latest patch for 1.9.4 can be found.
Although 50 moodlers have voted for a solution to 1626, I have not yet received any feedback from other developers or administrators on the solution itself, but I had hoped that some would try it and tell me what they liked or disliked.
I have tested the changes extensively and I believe them to be sound, but you do need to run one update SQL statement...
ALTER TABLE `mdl_forum_subscriptions` ADD COLUMN `thread` BIGINT(10) UNSIGNED NOT NULL DEFAULT 0 AFTER `forum`, ADD INDEX `mdl_forusubs_thr_ix` (`thread`);
...before applying the patch.
The patch was created on a local CVS so you may need to point it at the correct file locations.
I would appreciate your comments.
Regards,
John
I'm afraid that getting core developers to review patches does take persistence.
This is not a good thing, because getting new contributors is one of the most important things for any open source project. On the other hand, all the core developers have a huge list of things to do for Moodle 2.0, reviewing code takes time, and writing your own code is normally more fun. So, posting a polite reminder here was a good thing to do. Please continue to be politely persistent.
However, having said that, I am afraid that I am not going to go and review your code right now because it is my bed time here. Hopefully someone else will take the bate.
This is not a good thing, because getting new contributors is one of the most important things for any open source project. On the other hand, all the core developers have a huge list of things to do for Moodle 2.0, reviewing code takes time, and writing your own code is normally more fun. So, posting a polite reminder here was a good thing to do. Please continue to be politely persistent.
However, having said that, I am afraid that I am not going to go and review your code right now because it is my bed time here. Hopefully someone else will take the bate.
In reply to John White
Re: Moodle Forum that allows subscription by discussion thread
by Dan Poltawski -
Hi John,
(I saw the bait and thought i'd offer comments)
This is quite an extensive patch, thanks for your work! Its great that you have been updating this patch and managed to do such work with such a large and complicated thing as the forum subscription code (braver than most on here!)
I have only skimmed it so far -
My immediate question is whether you've examined performance before/after your patch? It looks like you've continued using some of the php-based caches, but is this going to be a memory hog on places like moodle.org when we have thread-level granularity?
You've replaced a few tests with more complex tests in functions so this would be my major concern needing more detailed review. In particular I would be concerned to check if any of your new functions have the possibility of introducing the 'database query in a loop' problem (see Martin Langhoffs thread about constant db queries in his accesslib rework).
From a patch style point of view, it'd be better not to have comment lines which explain the patch itself but are not relevant when this is merged into core (like '//Edited to allow thread-based subscriptions'), but I understand your patch started off not-as a patch so I understand and this can be sorted if/when merged into core.
As such a big patch I personally don't think it'd be wise to apply this 1.9 as there is large scope for regressions (though, on the other hand, forum subscriptions are a sparsely tested feature in a development version due to the nature of developers working environments). Petr Škoda most recently did a quite extensive work on the forum subscription model for 1.9 - particularly in regard to performance, so I hope we can get him involved to find some time for review, having the most fresh mind on potential caveats
(I saw the bait and thought i'd offer comments)
This is quite an extensive patch, thanks for your work! Its great that you have been updating this patch and managed to do such work with such a large and complicated thing as the forum subscription code (braver than most on here!)
I have only skimmed it so far -
My immediate question is whether you've examined performance before/after your patch? It looks like you've continued using some of the php-based caches, but is this going to be a memory hog on places like moodle.org when we have thread-level granularity?
You've replaced a few tests with more complex tests in functions so this would be my major concern needing more detailed review. In particular I would be concerned to check if any of your new functions have the possibility of introducing the 'database query in a loop' problem (see Martin Langhoffs thread about constant db queries in his accesslib rework).
From a patch style point of view, it'd be better not to have comment lines which explain the patch itself but are not relevant when this is merged into core (like '//Edited to allow thread-based subscriptions'), but I understand your patch started off not-as a patch so I understand and this can be sorted if/when merged into core.
As such a big patch I personally don't think it'd be wise to apply this 1.9 as there is large scope for regressions (though, on the other hand, forum subscriptions are a sparsely tested feature in a development version due to the nature of developers working environments). Petr Škoda most recently did a quite extensive work on the forum subscription model for 1.9 - particularly in regard to performance, so I hope we can get him involved to find some time for review, having the most fresh mind on potential caveats
In reply to Dan Poltawski
Re: Moodle Forum that allows subscription by discussion thread
by John White -
Dan,
Thanks very much for your input. I am very pleased to receive the feedback.
You are certainly right about it being a big patch. When I started I hadn't envisaged it being quite so big, but that is partly because there are quite a few places in the forum code generally where the existing code is rather stringy (from my personal view) so that I had to make choices about how best to implement the functionality without massively re-writing what's there already. And partly because I failed to predict how many different issues would need to be considered!!!
Yes I am certainly very concerned indeed about performance, especially concerning the performance in very large site/forums. Indeed one of the great benefits of getting others involved would be to see what additional load this patch places on big systems! But we should also note that whilst the additional code will lay some extra overhead, the implementation may massively REDUCE the number of email transactions needed to service a forum like 'General Problems'. Conversely, over time it may actually INCREASE this number as more of us opt to have partial email traffic rather than NONE! So the dilemma, as I see it, is not just that the code process might be heavier, but that the forum might be more to users' liking!!! This is seriously problematic for me to test or predict.
There are three places in the code where it is likely to make an impact on performance, and I have made efforts to address each. (I should have taken timing readings before messing with any of it, but I could do so now by switching out thread subscription which switches out nearl all the code.) (1) When the forum view page is built, adding the yes/no buttons certainly creates some additional overhead. It is not for me to make policy, but I think unnecessary overheads can be created by trying to service the unservicable, in this case browsers with Javascript (and thus AJAX) turned off! But if this is appropriate to reach the widest audience then it may be dificult to slim this down. (2) When forum_cron() runs there is additional overhead in setting up the subscriptions to thread, but almost no additional overhead on collecting user data and the like as this is done once - and only once - for each user regardless of whether their subscription is to forum or to thread (only userid's are collected for separate thread subscriptions to cut this load right back). (3) When the user chooses to sort subscribed threads first. This is bound to be an issue, and I have placed an extensive comment in forum/lib.php (search for usort), because I believe there are two ways of doing this better than the method I have implemented, but requiring a substantial restructuring of the code, which I didn't feel I had the right to do upfront! Here I have suggested a double-pass method which would be ultra fast, requires minimal tests, and is suited to the situation where the data is already sorted except for one Boolean!
I've read Martin's post (phew!) and I think to some extent the patch addresses this type of problem, for example one new function in lib.php has a whopping list of nested simple query tests, because MOST cases will bail out very early in this testing process, so fewer queries need to run.
I take the point about the sorts of comments I've added, but I think they are best in for now, and i could take them out and re-patch if this is a go-er.
Before I started I checked carefully through the Moodle 2.0 planning sheet and Roadmap and saw no comments about the Forum module, have I been working in parallel? I certainly like to hear from Petr.
Again, many thanks,
John
PS Now I see lots of feedback from Eloy in the Tracker to consider too.
Thanks very much for your input. I am very pleased to receive the feedback.
You are certainly right about it being a big patch. When I started I hadn't envisaged it being quite so big, but that is partly because there are quite a few places in the forum code generally where the existing code is rather stringy (from my personal view) so that I had to make choices about how best to implement the functionality without massively re-writing what's there already. And partly because I failed to predict how many different issues would need to be considered!!!
Yes I am certainly very concerned indeed about performance, especially concerning the performance in very large site/forums. Indeed one of the great benefits of getting others involved would be to see what additional load this patch places on big systems! But we should also note that whilst the additional code will lay some extra overhead, the implementation may massively REDUCE the number of email transactions needed to service a forum like 'General Problems'. Conversely, over time it may actually INCREASE this number as more of us opt to have partial email traffic rather than NONE! So the dilemma, as I see it, is not just that the code process might be heavier, but that the forum might be more to users' liking!!! This is seriously problematic for me to test or predict.
There are three places in the code where it is likely to make an impact on performance, and I have made efforts to address each. (I should have taken timing readings before messing with any of it, but I could do so now by switching out thread subscription which switches out nearl all the code.) (1) When the forum view page is built, adding the yes/no buttons certainly creates some additional overhead. It is not for me to make policy, but I think unnecessary overheads can be created by trying to service the unservicable, in this case browsers with Javascript (and thus AJAX) turned off! But if this is appropriate to reach the widest audience then it may be dificult to slim this down. (2) When forum_cron() runs there is additional overhead in setting up the subscriptions to thread, but almost no additional overhead on collecting user data and the like as this is done once - and only once - for each user regardless of whether their subscription is to forum or to thread (only userid's are collected for separate thread subscriptions to cut this load right back). (3) When the user chooses to sort subscribed threads first. This is bound to be an issue, and I have placed an extensive comment in forum/lib.php (search for usort), because I believe there are two ways of doing this better than the method I have implemented, but requiring a substantial restructuring of the code, which I didn't feel I had the right to do upfront! Here I have suggested a double-pass method which would be ultra fast, requires minimal tests, and is suited to the situation where the data is already sorted except for one Boolean!
I've read Martin's post (phew!) and I think to some extent the patch addresses this type of problem, for example one new function in lib.php has a whopping list of nested simple query tests, because MOST cases will bail out very early in this testing process, so fewer queries need to run.
I take the point about the sorts of comments I've added, but I think they are best in for now, and i could take them out and re-patch if this is a go-er.
Before I started I checked carefully through the Moodle 2.0 planning sheet and Roadmap and saw no comments about the Forum module, have I been working in parallel? I certainly like to hear from Petr.
Again, many thanks,
John
PS Now I see lots of feedback from Eloy in the Tracker to consider too.
In reply to John White
Re: Moodle Forum that allows subscription by discussion thread
by John White -
...further to that, I have just seen a reason to retract the moan about support for non-javascript browsers... PDAs! (must be a very good reason for shelling out *loads* on Apple's offering!)
In any case the non-javascript support continues in this patch.
John
In any case the non-javascript support continues in this patch.
John
In reply to John White
Re: Moodle Forum that allows subscription by discussion thread
by Brad Smith -
I was just looking for information on this very feature, which I would really love to see. It sounds like things were progressing well, but the feature still seems to be missing in moodle 2.1 (or whatever the moodle.org forum uses). Might I inquire as to the status?
In reply to Brad Smith
Re: Moodle Forum that allows subscription by discussion thread
by Konrad Lorinczi -
I would also welcome, if I could subscribe just to a discussion thread on Moodle Forum, instead of the full course.
I want to get notified about replies to the thread I started or submitted message, but do not want to get all messages posted to the course (which are not important for me).
Waiting solution for subscription by discussion thread.
In reply to Brad Smith
Re: Moodle Forum that allows subscription by discussion thread
by sam marshall -
ForumNG (custom forum not part of core moodle), which I am lead developer for, has this feature. You can also subscribe by group (in a group forum, subscribe to group A but not B, C, or D - important if you have accessallgroups, but only one or two of the groups are particularly important to you. E.g. a tutor who wants to temporarily follow another tutor's group while they are on holiday, but doesn't want to follow all of the remaining 100 groups as well.)
There is some prospect of including ForumNG, or else using some of its ideas, in a future standard Moodle version. Since 2.2 is basically finished now, the earliest this could happen is 2.3, although 2.4 might be more likely. Some changes are required before ForumNG could be included in core; for example, at present it sends email 'manually' (like in 1.9) rather than using the new Moodle messaging system.
If you want to use ForumNG on your own moodle install you can try it (at your own risk) - it is currently beta-quality code. Source code is here. We might have a more stable version later.
--sam
In reply to sam marshall
Re: Moodle Forum that allows subscription by discussion thread
by Konrad Lorinczi -
I meant the moodle.org/mod/forum .
This forum allows only to subscribe to a whole forum (course).
I'm tired of getting a lot of irrelevant posts, when I want to watch only the thread I was posting...
Could be possible to use ForumNG on moodle.org?
In reply to Konrad Lorinczi
Re: Moodle Forum that allows subscription by discussion thread
by sam marshall -
Yes it would be possible to use ForumNG on moodle.org if HQ wanted to do that. I don't think it's very likely they would choose to do this unless it was firmly decided to use ForumNG in core moodle - and then most likely not ahead of time, i.e. only when updating moodle.org to that next version.
But, theoretically yes they could install it tomorrow.
--sam
In reply to sam marshall
Re: Moodle Forum that allows subscription by discussion thread
by Konrad Lorinczi -
Would be fine if someone could forward this request to Moodle.org developers.
In reply to Konrad Lorinczi
Re: Moodle Forum that allows subscription by discussion thread
by sam marshall -
Feel free...
(You can file tracker issues about the moodle.org sites.)
--sam