Links module: Site crashes -> loop detected

Regarding your time limit issue, are you saying that the upgrade script ran for 300 seconds and still wasn't done, or that you changed the time to 300 seconds but it still timed out in 30? I wouldn't think it would take 300 seconds to run 1600 links, but you may want to try setting the time to something like 600 just to be sure. It may be that your server isn't really allowing you to override the time limit. Also, check PHP's memory limit setting to see if you may be running out of RAM.

As for the friendliness of the URL, I'm not sure if I can do anything about that. The information in the URL (using the example you provide) includes:

links

gets you to the "links" module

goto

tells links.module that we're doing an outbound link rather than showing a node

2792

gives the link ID so we can look up the target URL from the database (the actual URL would have been longer)

1745

gives the node ID where the outbound "click count" should be recorded (links tracks outbound clicks by referring node, since the same link record may be multiply referenced)

links_weblink

gives the context of the link

The last field, context, is not strictly required. It gives links.module a little more information for filtering the outbound links based on which module (part of Links, or someone else's module that uses the Links API) generated the link. If you omit this parameter, the link will still work.

You can omit the last numerical parameter (in your example, 1745) as well, and again, the outbound link will still work. But the click will not be counted because links.module won't know which node using that URL is the one that generated the link.

I'm going to take a look at that context field in the outbound URLs and see if it's really needed there. Maybe I can get along without it. But the two numeric parameters are definitely needed.

Hope this helps.

Scott (Syscrusher)

I did some code review. I knew there was a reason why I added that context parameter to the redirection URLs, but I couldn't recall why.

It turns out that this parameter is needed for click counting because the same node can refer to the same link from multiple contexts. For example, suppose you have node #1234 that is a weblink type, and that you have configured your site so that weblink nodes are allowed to also have related links. It's possible -- silly, but possible -- to have the same link (and therefore the same link ID number) be attached to that node in the {links_node} table for both reasons, and they would be distinguished only by their context ($module) field.

Now, I'll be the first to grant that this is a silly example, but remember that the Links Package is meant to be an API used by other modules as well. For instance, suppose someone writes a contrib module that adds an "Author's Home Page" link to Flexinode or CCK content. Or suppose they add a field like that to Book Review or Recipe or any other node-type module. Now you might possibly have a link that appears as a "Related Link" and as an "Author's Home Page" link from the same node. Again, I think this might be confusing to the user, but I don't want to limit the API because I'm concerned about situations where someone might be importing legacy content that merges what used to be two different link repositories.

If people are really annoyed by the context field here, I'm willing to reconsider the above -- so please post comments to this thread if this is an issue for you. Maybe we *should* have a restriction that the same URL can only be linked from any given node in one way. If we add that stricture, though, the question becomes what to do when some module attempts to break that restriction. An error message isn't very user-friendly, because it would be hard to explain to the user why the database couldn't deal with this. Comments?

Scott

First, I do think that it is important to be able to have multiple instances of the same link associated with a single node. I think that for maximum flexibility, you need to be able to distinguish each instance from the others, including the click-counting.

I've been thinking about this in the context of my pet feature request [1], which I've been giving some more thought to recently, and I'm not sure the current API and underlying schema are able to handle this case. I think it may be more appropriate to continue this thought in that thread ... which I'll do momentarily.

[1] http://drupal.org/node/55208

Great feature set and general functionality for this module, congrats. However, I think this site crash issue may be related to something I observed while running the devel module with a test site.

On going to node that has one single link_related link, the function links_load_links_for_node gets called an amazing 25 times, consisting of 5 calls, each of which appears to loop through the function 5 times. On first read through of the code, problem seems to stem from misunderstanding about the nodeapi apparatus. Specifically:

in links_related_nodeapi ($op = 'load'): links_load_links_for_node is called, and returns an array called (links_related) which contains the link info(lid, url, md5, etc). That's all we really need, I believe! However...

in links_related_nodeapi ($op = 'view'): l_l_l_f_n is called again, and this time the info is added to GLOBALs (don't understand this at all);

and to make matters worse, within ($op = 'view') the function _links_related_prep_view is called, and this function ALSO calls links_load_links_for_node all over again.

My nodeapi knowledge is rusty, so correct me if I'm wrong: but it seems that calling links_load_links_for_node is only necessary ONCE, in links_related_nodeapi (op = load); this successfully adds the info to the node array, under the desired key ['links_related']. Subsequently, only manipulation and themeing functions are necessary - there's no further need to return to the db. So the calls to links_load_links in $op = view, and in the links_related_prep_view function, aren't needed.

It gets worse, because even on a teaser page generated by a call to tax/term/, links_load_links is called 25 times (5x5) for each individual node that has a related link. This could be why the module is leading to crashes on a large site...

So that's a possible diagnosis.

Ugh. That's a plausible scenario. I'll look into this as soon as I can. Until recently, I was rather out of the Drupal world due to business travel and other higher-priority things, but I'm back home now.

Syscrusher

Just wondering...

I guess this bug is present under 5.x. Has this issue been looked into? #5, #6, and #7

I ask because the voting up.down package at http://drupal.org/project/vote_up_down requires this.

However, if this issue has not yet been weeded out then I for one am reluctant to install due to the potentially crippling consequences. Therefore, the voting up/down package is off the cards as well :(

Sorry to bring this up again, but I guess many a reader may feel the same apprehension.

Keep up the good work

Sorry...I forgot this wasn't fixed yet (and it was unassigned...hmmm....I must have been really tired that day).

I'll look into this ASAP. It may take a couple of days to figure it out, because I've so far been unable to replicate the error in my test environment.

Syscrusher