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

feature(engine): prevent shutdown events from delaying next page load #8061

Closed
wants to merge 1 commit into from
Closed

Conversation

beck24
Copy link
Member

@beck24 beck24 commented Mar 13, 2015

Allows for long-running background scripts to not interfere with UX

Allows for long-running background scripts to not interfere with UX
@ewinslow
Copy link
Contributor

perf?

@beck24
Copy link
Member Author

beck24 commented Mar 13, 2015

yep, it should go under the perf label
I'll change it before merge once I hear back about any changes/objections

@beck24
Copy link
Member Author

beck24 commented Mar 13, 2015

I should probably note that this is basically pulling https://community.elgg.org/plugins/1222696 into core
I use it on every project now, I can't even think of an exception.

@jdalsem
Copy link
Member

jdalsem commented Mar 13, 2015

Regular elgg will probably not get any benefits from this i guess. What usecases are there to have scripts run indefinitely are there in your projects? With set_time_limit(0) these processes will run forever without any ability to close those sessions. Should we limit them at least to 24 hours or something?

// Ignore user aborts and allow the script to run forever
ignore_user_abort(true);
// Prevent an APC session bug: https://bugs.php.net/bug.php?id=60657
session_write_close();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need docs that devs shouldn't write to session during shutdown. Better if there were a way to catch it.

@brettp
Copy link
Member

brettp commented Mar 13, 2015

This was developed at MITRE. Just a caveat, we don't use it any more because we found that it doesn't work in a number of situations depending on server and PHP settings, and our primary reason for it (notifications) was resolved in core. For example, regardless of PHP's output buffer setting, if any output is > 2048k it holds instead of closing the connection.

@mrclay
Copy link
Member

mrclay commented Mar 13, 2015

Seems better to make it easier to use the queue Cash added.

@beck24
Copy link
Member Author

beck24 commented Mar 13, 2015

Thanks @brettp, I haven't noticed any issues with it in my uses, that's good feedback

My use cases are varied, but usually have to do with large groups or large numbers of friends. The most recent examples are scenarios where users have settings to automatically become friends with people who join their groups. When a user joins a group that already has 800 people, that's a long-running loop to make them all friends. It works nicely to defer this function to shutdown and have it continue in the background.

Another one is a group calendar that syncs events to a personal calendar. A similarly large group adds an event, that event now needs have relationships created for each members calendar.

I have a plugin to fix the friends access issue by making everyone have their own real access collection consisting of all of their friends. When friendship gets updated I have it rebuild the access_collection (probably overkill, but as a plugin it's possible for it to get out of sync if the plugin is disabled for a time). For a user with a lot of friends this again ends up potentially taking a while.

The solr plugin uses this to handle large reindexing tasks. When the admin triggers a reindex, it hits the action, the action registers the shutdown functions, the admin is returned with a system message stating that reindexing is underway, and it does it's thing. As the script progresses it writes to a log file which is displayed via ajax to the admin on the index page to view real-time progress without having to wait for it complete or keep the page open.

On 1.8 we didn't have the queue which is why I've been leaning on this so much, but it's been working well for me.
On 1.9+ with a high activity site with these kinds of operations happening - is the queue going to keep up? My concern is that there would be a noticeable time-lag between the action (eg. user becoming friends) and the result (eg. user friends acl rebuilt).

Thoughts?

@brettp
Copy link
Member

brettp commented Mar 13, 2015

On 1.9+ with a high activity site with these kinds of operations happening - is the queue going to keep up? My concern is that there would be a noticeable time-lag between the action (eg. user becoming friends) and the result (eg. user friends acl rebuilt).

This is a very valid concern. The queue only runs every minute. We've noticed some considerable (3 minute) delays in receiving email notifications in large groups. For notifications, this isn't much, but for something like joining groups or ACLs, it would be.

@hypeJunction
Copy link
Contributor

As Evan mentioned in another thread, it seems it would make sense to
replace some of these batch operations with computed values. Perhaps we
should go over some of the bottlenecks and see how we can perform these
operations on demand rather than running long scripts. In the case of
comments, I would almost say plugins have no business in altering the
permissions of individual comments, it seems that we want to sure permanent
sync, so why let plugins mess with that logic. A single SQL update would do
the trick, and we would save a lot of cycles.
Is there any other way to process queues without cron, some daemon worker
perhaps?

On Fri, Mar 13, 2015 at 8:52 PM, Brett Profitt notifications@github.com
wrote:

On 1.9+ with a high activity site with these kinds of operations happening

  • is the queue going to keep up? My concern is that there would be a
    noticeable time-lag between the action (eg. user becoming friends) and the
    result (eg. user friends acl rebuilt).

This is a very valid concern. The queue only runs every minute. We've
noticed some considerable (3 minute) delays in receiving email
notifications in large groups. For notifications, this isn't much, but for
something like joining groups or ACLs, it would be.


Reply to this email directly or view it on GitHub
#8061 (comment).

@beck24
Copy link
Member Author

beck24 commented Mar 14, 2015

So the consensus here is that we don't like this, but we do need a way to facilitate doing things in the background in a timely manner that the user doesn't need to wait for between page loads right?

@ewinslow
Copy link
Contributor

I think that about captures it, yes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants