Hi Dave,

The throttle field in cron jobs will only work if ALL actions in the cron action have the same throttle. I am thinking about fixing this (as I want it for my Notifications module - i'll send it to you when I tidy it up) by adding a new table with a nid/fbu -> time_cron lookup.

A few quesations:
- Is it ok to have two tables in a .install file (I assume so)
- Is this something that you will add to the official release (if not I will do it in a seperate module so that my merges of official releases will go easier)
- do you think that apikey should go in this new table (since you are checking it in this particular code), or do you think it can be just ignored?
- I am tempted to have something else in this table other than nid/fbu as keys becacuse what if you want to be able to do something that doesn't actually have a nid (ie a user_to_user motification which is just an adhoc kind of thing and probably wouldn't be related to a nid). But Im not sure exactly what to do... any ideas?

Thanks
Nigel

Comments

Dave Cohen’s picture

Dave Cohen commented
Status: Active » Postponed (maintainer needs more info)

Hey Nigel,

I'm not sure I understand completely. Are you talking about a facebook specific throttle or drupal's? DFF modules include an action which is perform some other action once per user of an app. and that action has its own throttle. If that's not meeting your needs try to explain why not. If I grok the need for the improvement, and the code is good, I'll include it.

regarding apikey in table... there are a number of ways to refer to an app in Drupal for Facebook. Each app has an apikey from facebook, as well as a label and a node id. It's a safe bet to use the apikey, as that will remain the same while conceivably someone could implement a replacement for fb_app.module that does not use node id or label (as unlikely as that may be).

I'm not sure how you're using nid/fbu so I'm not sure what to say there.

NigelC’s picture

NigelC commented
Status: Postponed (maintainer needs more info) » Active

Sorry I was a bit unspecific. Regarding the apikey - thats kind of what I figured. I wasn't sure though if I really cared. But in the interest of doing things that work 100% rather than work for me, I will put it in.

The throttle I was referring to was in "Perform an action once per user" from your fb_action.module. If you create multiple actions with different throttles they kind of interfere with each other since the ordering of cron actions is based on a single time_cron field per user. Lets say you run cron once per hour and want to split all your users over 24 hours for action A (say 1000 users = 42 per cron). But you want another action B to be once per week and so you give it a throttle of 6 per cron. Problem is that by doing action A you therefore never get action B for that user since its now at the bottom of the list. Now if your numbers are just so, you will probably get roughly what you want, but for some cases the numbers may line up and you never get one of the actions for some users. (ie 4 users, action A throttle = 2, action B throttle = 2 : first 2 users always get action A, second 2 always get action B and nothing else).

Instead I will create a table just for this time_cron and it will be indexed by user and nid (where nid is related to something like a fb_feed template - or a fb_notify template in my case). I still have an issue in the future of what to do if a node is not involved, but right now I am not to worried about that.

The other thing I would like to look at while doing this is a switch to make the throttle unchange from the current usage or division based. ie if throttle = 10 and division = true, then it throttle (as currently used) = number of users / division throttle. That way I can roughly get a time based output to users - ie 1 action per day, 1 action per week etc.

Nigel

Dave Cohen’s picture

Dave Cohen commented

Sounds like the time_cron needs to be moved away from the fb_user_app table. Instead a new fb_action table with columns apikey, fbu (and/or uid), action id, and time_cron. I guess it was a shortcut for me to put that in fb_user_app.

Regarding the division throttle.. that idea worries me because the idea is to limit each cron execution to a reasonable time. To avoid a php timeout or spiked server load. So you should tune that to the most that can reliably be run during each cron execution. And that number should not increase when the user base increases. Instead I'd recomment increasing the frequency of the cron jobs when there are more users.

NigelC’s picture

NigelC commented

hmm thats an interesting thought. A script that runs the cron job, looks up the number of users and adjusts the next cron time based on user count. definetly better for load.

Letharion’s picture

Letharion commented
Status: Active » Closed (won't fix)

Since Drupal 5 is no longer supported, I'm taking the liberty to close all FB D5 issues. Issues that haven't moved since D5 are unlikely to still be valid.