Cron fails

Hi,

Thanks for the fast reply.

It's fixed itself. I'd installed supercron and was invoking visitorinfo cron via that to test it. I was getting 500 errors and trying to find anything in the main (shared) error log. Then it worked. I reckon the problem is that visitorinfo_cron() is open ended. It will look up as many blank IP as there are and each time it waits 0.5s. Cron and functions have a timeout limit so I think I was just hitting that. The final time when it completed it had take 155s so the timeout must be longer than that. The visitorinfo cron function probably should be limited to do all blank IPs OR some maximum number of lookups so it doesn't take too long.

I've also now set the refresh time to 0 so that it never refreshes. That seems like it also might be part of the cause as at some time doesn't this mean it will try to do all the IPs again? Or at least 2 days worth.

Part of the problem might also be that my cron had stopped. Maybe because cron was interrupted (host had moved not long ago) and that broke it until I cleared the semaphore flag. By then I had 4 weeks backlog on visitorinfo.

Malc

The patch seems to be malformed if I'm reading it right. At line 5 it has

@@ -233,7 +233,8 @@ function visitorinfo_cron() {

so line 233 should be "function visitorinfo_cron() {" which is it is (well 232 in .dev version). But then the next block in the patch file says

@@ -241,19 +242,22 @@ function visitorinfo_cron() {

And you can't have the same text at both line 233 and 241. At 240 the module has " if ($interval > 0) {"

I ended patching by hand as that wasn't the only problem (well I could get patch to run so quicker to do it by hand). This was on the latest dev version which I would have thought would have included this.

Thanks for the quick reply.

Must be have been the -p1 that was the problem them. The error from patch was VERY cryptic and when I looked at the patch file this looked like an error.

I do think line 30 is wrong though

$result = db_query_range("SELECT csid, ip FROM {visitorinfo} v WHERE v.city = '%s'", $nothing, 0, $limit);

since you removed $nothing so '%s' should be ''.

I think that line should be

    $sql = "SELECT    csid, ip
            FROM      {visitorinfo} v
            WHERE     v.city = ''
            ORDER BY  csid DESC";
    $result = db_query_range($sql, 0, $limit);

That way you fill in the latest results first (but maybe the natural result is csid DESC). The latest results are the more important as the map is filled with csid DESC.

The other thing I can't find is anything which trims the table or re-arranges it. The logic seems to be:

Get visitor's IP
Check if IP already in database, if not insert it.
At cron fill in new IP locations.
For map grab the latest, new, 200 IPs

The interesting thing is that if the IP is from a repeat visitor then nothing changes in the database. Since the map takes the latest 200 (or whatever figure you set of course) then only new visitors appear on the map. I would have thought that latest visitors (new or repeat) is more logically. Or most frequent visitors (which would mean recording a count of course).

If the aim is to show the latest visitors then after find the visitor is in the database the city etc. details could be grabbed and the visitor inserted again. Then cron could delete entries after 200 with city etc. details (or whatever is the map setting).

Alternately the table could have a visit_count which is inc'ed each repeat visit and a visit_date which is also updated. You could then opt for latest 200 or most frequent 200. Table trimming might be left to manual, maybe.

Good point. I'll start another specific thread.