Improvements wrt unpublishing sites with the Course Site Removal job
Description
is depended on by
Activity
Brian Baillargeon October 7, 2019 at 2:03 PMEdited
Yeah that makes sense to me - the terms aren't being added to the sql in their proper order because the propertyRestriction / propertyCriteria blocks should be swapped. I think I ran into a messy merge conflict on this one, and I missed this. I'm working on stuff related to the Assignments conversion for the time being, but I've added it to my queue to fix and test this when I find the time
Sam Ottenhoff October 7, 2019 at 1:30 PMEdited
> Ultimately the propertyCriteria terms should fall into this first query
The code I referenced above deals with propertyCriteria after restrictions. Seems like they are pretty clearly swapped. I'm not sure how this is working correctly for anyone.
I have created
Brian Baillargeon October 7, 2019 at 1:12 PMEdited
We last ran the job in production on July 30th, and there were no issues. It's possible that an issue was introduced while I was contributing the code, but I'm pretty sure I had tested it without issue. Ultimately the propertyCriteria terms should fall into this first query, and the propertyRestrictions terms should fall into this second query; if that's not the case, then there's a bug
Sam Ottenhoff October 7, 2019 at 11:22 AMEdited
Right, the paste seems off, but the order of criteria and restrictions still seems wrong. The SQL is including sites with the "set" property and then excluding sites in the term.
Is this job working correctly for anyone using Sakai 19+?
Commit 3519a410d4c9ba3f3394c7b4f26678218c2b8116 added propertyRestrictions before propertyCriteria in DbSiteService -> getSitesFields
Brian Baillargeon October 7, 2019 at 11:08 AM
Are you sure you've pasted that correctly? I don't believe that's syntactically correct, particularly:
... = '0' and select SAKAI_SITE.SITE_ID ...
I suspect the first chunk was repeated, and the query is actually:
Details
Details
Priority
MajorAffects versions
Fix versions
Components
Assignee
Brian BaillargeonReporter
Brian Baillargeon
The Course Site Removal job's default configuration is to unpublish sites.
In our seven years of working on Sakai at Western, it's safe to say that this piece has the... 'least optimal' performance. Once, we ran this in our QA environment and got sidetracked; it completed after 12 days of execution.
A big part of this is that it invokes siteService.save(site) has a lot of slow performing hooks (as once might expect, you're saving a whole site afterall!). These hooks include:
triggering SiteAdvisors
deleting all pages, tools, properties, etc., associated with the site, then inserting them all back with any modifications
handling authz group changes
notifying ContextObservers to do their own work related to the site modification
triggering EventTrackingService
These are important hooks, but generally, all except the EventTrackingService are not necessary when the job's intention is to simply flip the "published" flag to 0.
So I introduced a sakai.property which allows all those hooked to be bypassed; default is false:
course_site_removal_service.silently.unpublish=false (default)
We also have a use case to handle sites that are crosslisted with sections in different terms.
For instance, consider a course that has two sections attached - CompSci101FW18A, and CompSci101FW18B. Those two sections can be in different terms.
If CompSci101FW18A is associated with an Academic Session whose end date is December, and CompSci101B is associated with an Academic Session whose end date is in May, the job currently only considers one of these end dates (the first one that was attached if I'm not mistaken). But we don't want to unpublish this site until both sections' academic sessions' end dates fall before the grace period, so the job needs to consider all the attached sections' Academic Session end dates before it can decide whether it can unpublish the site.
I've introduced a sakai.property for this too that is also off by default:
course_site_removal_service.handle.crosslisting=false