April 2005 - Development - lists.drupal.org

[drupal-devel] [bug] race condition in variable_set in bootstrap.inc
by jhriggs 08 Apr '05

08 Apr '05

Issue status update for http://drupal.org/node/20114 Project: Drupal Version: cvs Component: base system Category: bug reports Priority: critical Assigned to: Ricetons Reported by: Ricetons Updated by: jhriggs Status: patch 1. Assuming that all we are talking about is variable_set() calls, DELETE is not an issue, because the patch removes deletes. It only does updates and inserts. Now if you throw in variable_del() calls, that's another story, but it's also outside the scope of this issue. It would likely be much more rare than the delete/insert problems we see now. 2. Your second example is the only lingering downfall. As I mentioned in my post: "...they may both see that the row does not exist and both attempt to do an INSERT. This will only happen the first time a variable is created, though, which may be more acceptable than what we are seeing now." 3. This doesn't add any queries. Right now it does DELETE and INSERT. With the patch it does SELECT and UPDATE/INSERT. Two queries either way. As I alluded, this may not be a true fix -- I don't think there is one for race conditions like this, even with table-locking -- but it addresses the problem in a way that makes sense and eliminates an error in the majority of cases. jhriggs Previous comments: ------------------------------------------------------------------------ April 6, 2005 - 21:53 : Ricetons This bug was exposed by the poormancron module. We can see that the module itself contains no bug. But in the function variable_get(), the function just execute SQL like this: DELETE bar WHERE key="foo" INSERT INTO bar VALUES(foo, bar2ooo) but suppose this: thread A and thread B has executed the DELETE sentence. And they do INSERT at the same time. then an SQL error like this occured: duplicated key 'foo' in table "variables" It's a critical bug. It should be fixed before the time that drupal 4.6.0 released. ------------------------------------------------------------------------ April 8, 2005 - 02:01 : robertDouglass I also experience this on a site that has many different cron cycles ranging from 2 minutes to 6 hours. The query errors which arise (as noted above) are impossible to reproduce through normal testing yet occur fairly (surprisingly) often in the logs. This doesn't just apply to cron jobs but rather to all code that calls variable_set. Setting to CVS since this hasn't been addressed since the issue was opened. ------------------------------------------------------------------------ April 8, 2005 - 07:10 : jhriggs Attachment: http://drupal.org/files/issues/variable_set_update.patch (922 bytes) I'm not sure that there is a "right" way to fix this. In mysql, we could use a REPLACE rather than DELETE/INSERT, but that wouldn't work for pgsql. And, if we are really seeing this type of race, there is no telling which REPLACE would actually make it last and be saved. Instead, we could do a check to see if the row exists and do an UPDATE if it does and an INSERT otherwise; however, this would be prone to the same problem if we are seeing such a race. That is, they may both see that the row does not exist and both attempt to do an INSERT. This will only happen the first time a variable is created, though, which may be more acceptable than what we are seeing now. If the row does already exist, they would both do UPDATES and would "succeed" in the sense that there is no duplicate key error. There is still no telling which one actually sticks, though. The attached patch takes the second approach. ------------------------------------------------------------------------ April 8, 2005 - 07:37 : Jeremy(a)kerneltrap.org What keeps another session from doing a DELETE after your session does its SELECT COUNT but before it does its UPDATE? Or, what keeps another session from doing an INSERT after your session does its SELECT COUNT but before it does its INSERT? All you've done is add another query... I think you'll need to use locking to fix this right.

1 0

[drupal-devel] [feature] settings.php: better instructions for $db_url and $db_prefix
by danielc 08 Apr '05

08 Apr '05

Issue status update for http://drupal.org/node/19474 Project: Drupal Version: cvs Component: base system Category: feature requests Priority: normal Assigned to: Anonymous Reported by: danielc Updated by: danielc Status: patch Attachment: http://drupal.org/files/issues/settings.diff (2.49 KB) Improves instructions in /drupal/sites/default/settings.php: * How to escape URL characters in $db_url. * How to use the array format of $db_prefix. danielc

5 7

[drupal-devel] 4.6 + PostgreSQL
by Marc Poris 08 Apr '05

08 Apr '05

Hi, For the last couple days I have been using 4.6 with PostgreSQL on windows with Apache 2 and PHP5. I've gotten a bunch of pg_query() errors and am wondering if this stuff is supposed to be working yet with PostgreSQL. Some of the errors: 1. Attempting to add a new role: INSERT INTO role (name) VALUES ('superduper') in C:\...\includes\database.pgsql.inc on line 62. pg_query() [function.pg-query <http://www.footballfantasywizard.com/function.pg-query>]: Query failed: ERROR: duplicate key violates unique constraint "role_pkey" in C:\...\includes\database.pgsql.inc on line 45. I fixed this by incrementing the role_rid_seq Current Value, but looking at the code in user.module, it seems that we would want to change the following (add a call to db_next_id): db_query("INSERT INTO {role} (name) VALUES ('%s')", $edit['name']); 2. Under logs, top users (and top pages) this error happens: SELECT COUNT(a.uid) AS hits, a.uid, u.name FROM accesslog a LEFT JOIN users u ON a.uid = u.uid GROUP BY a.uid ORDER BY hits DESC LIMIT 30 OFFSET 0 in C:\...\includes\database.pgsql.inc on line 63. pg_query() [function.pg-query <http://www.footballfantasywizard.com/function.pg-query>]: Query failed: ERROR: column "u.name" must appear in the GROUP BY clause... 3. First time I went to the forums (can't reproduce this one, but will do so on a clean install): INSERT INTO vocabulary (name, description, help, multiple, required, hierarchy, relations, weight, module, vid) VALUES ('Forums', '', '', 0, 1, 1, 0, '', 'forum', '1') pg_query() [function.pg-query <http://www.footballfantasywizard.com/function.pg-query>]: Query failed: ERROR: invalid input syntax for integer: "" --- I guess my questions are: 1. Is there a main contact for Drupal/PostgreSQL issues (I emailed James Arthur whose name is at the top of database.pgsql and he gave me some great starting points but said he's been out of the loop for a number of years)? 2. When 4.6 is released, what does that mean as far as PostgreSQL stability? And I'm volunteering to help out -- I've got some time on my hands, but I am not a PostgreSQL expert. Doesn't look like one is really needed, but there appears to be a need for someone to be banging on it. Please feel free to flame away. Hope I haven't put this out in the wrong venue. Marc

5 6

[drupal-devel] Taxonomy Similar: Very Early First Draft
by Morbus Iff 08 Apr '05

08 Apr '05

Hey all. Anyone interested in my tagging patch has probably seen: http://disobey.com/detergent/2005/similar_keywords.jpg which is/was a homebrew UI to select similar tags to the ones entered during content creation. I've just finished an early draft for Drupal: http://disobey.com/detergent/2005/drupal_taxonomy_similar.jpg cvs.drupal.org/viewcvs/drupal/contributions/modules/taxonomy_similar/ Some notes: * the low match count is because I've only imported 8000 terms, and not all the node content that exists at NHPR.org. I'll be populating term_node soon so I can get an idea of the hit on tag->node counting. * the module works best when you already have a large tagging vocabulary. It'll probably be pretty useless if you have less than, say, 500 terms. * there's no help, README, blah - hell, there's not much of anything in the way of documentation besides that screenshot. Still, the code DOES work (and the next version will redirect you to node/$nid instead of itself after submission). * it will ONLY kick in on free-tagging vocabularies. there is no configuration necessary. install it, enable it, insert/update a node that uses free tagging. I got into some small discussion about the UI today, and I wanted to rationalize why I've done it the way I've done it. The current UI supports ALL these possibilities, and I've yet to find a compact or intuitive alternative based on other folks suggestions. * don't change a thing (close the window, or "submit"). * re-assign tags (uncheck your original, check new one). * add tags (keep your original checked, check new one). * delete tags (uncheck your original, don't check new one). I have plans on the following explorations: * Explore "merging" of tags, such that two similar tags can be merged into one combined tag. this would be an 'administer taxonomy' access. * Explore playing with term_relation so that similar tags are related to one another when a user makes that explicit choice. * Explore a 'view nodes' sorta screen or block, which would just link off to similar keywords. this seems evilly dangerous, since the similarity code is prohibitive to run on a large vocab (8000+ tested). * Currently, there's a bit of confusion when it comes to "your tags" that are also "similar tags" for another one. So, say you choose "Death Penalty". A similar tag is "Juvenile Death Penalty", so you also choose that one. The NEXT time you go back to that screen, should "Juvenile Death Penalty" (now one of "your tags") still be shown as a similar tag under "Death Penalty"? From a clutter standpoint, no. From a comprehension/expectation standpoint, I'm not sure. Anyways. -- Morbus Iff ( you are nothing without your robot car, NOTHING! ) Culture: http://www.disobey.com/ and http://www.gamegrene.com/ Spidering Hacks: http://amazon.com/exec/obidos/ASIN/0596005776/disobeycom icq: 2927491 / aim: akaMorbus / yahoo: morbus_iff / jabber.org: morbus

3 9

[drupal-devel] [bug] race condition in variable_set in bootstrap.inc
by Jeremy 08 Apr '05

08 Apr '05

Issue status update for http://drupal.org/node/20114 Project: Drupal Version: cvs Component: base system Category: bug reports Priority: critical Assigned to: Ricetons Reported by: Ricetons Updated by: Jeremy(a)kerneltrap.org Status: patch What keeps another session from doing a DELETE after your session does its SELECT COUNT but before it does its UPDATE? Or, what keeps another session from doing an INSERT after your session does its SELECT COUNT but before it does its INSERT? All you've done is add another query... I think you'll need to use locking to fix this right. Jeremy(a)kerneltrap.org Previous comments: ------------------------------------------------------------------------ April 6, 2005 - 22:53 : Ricetons This bug was exposed by the poormancron module. We can see that the module itself contains no bug. But in the function variable_get(), the function just execute SQL like this: DELETE bar WHERE key="foo" INSERT INTO bar VALUES(foo, bar2ooo) but suppose this: thread A and thread B has executed the DELETE sentence. And they do INSERT at the same time. then an SQL error like this occured: duplicated key 'foo' in table "variables" It's a critical bug. It should be fixed before the time that drupal 4.6.0 released. ------------------------------------------------------------------------ April 8, 2005 - 03:01 : robertDouglass I also experience this on a site that has many different cron cycles ranging from 2 minutes to 6 hours. The query errors which arise (as noted above) are impossible to reproduce through normal testing yet occur fairly (surprisingly) often in the logs. This doesn't just apply to cron jobs but rather to all code that calls variable_set. Setting to CVS since this hasn't been addressed since the issue was opened. ------------------------------------------------------------------------ April 8, 2005 - 08:10 : jhriggs Attachment: http://drupal.org/files/issues/variable_set_update.patch (922 bytes) I'm not sure that there is a "right" way to fix this. In mysql, we could use a REPLACE rather than DELETE/INSERT, but that wouldn't work for pgsql. And, if we are really seeing this type of race, there is no telling which REPLACE would actually make it last and be saved. Instead, we could do a check to see if the row exists and do an UPDATE if it does and an INSERT otherwise; however, this would be prone to the same problem if we are seeing such a race. That is, they may both see that the row does not exist and both attempt to do an INSERT. This will only happen the first time a variable is created, though, which may be more acceptable than what we are seeing now. If the row does already exist, they would both do UPDATES and would "succeed" in the sense that there is no duplicate key error. There is still no telling which one actually sticks, though. The attached patch takes the second approach.

1 0

[drupal-devel] [bug] comment stats don't update after approval in moderation queue
by ankur 08 Apr '05

08 Apr '05

Issue status update for http://drupal.org/node/18215 Project: Drupal Version: 4.5.2 Component: comment.module Category: bug reports Priority: normal Assigned to: Anonymous Reported by: ankur Updated by: ankur Status: patch Attachment: http://drupal.org/files/issues/comment.module_5.diff (861 bytes) I don't know if this is by design, but I experimenting with forcing anonymous comments through the comments approval queue. After I logged in as someone with appropriate permissions and marked the comment "published" in the comments approval queue, the link attached to the appropriate node teaser still said "add new comment" rather than "1 comment" or "1 new comment". Here's a patch for it. I believe the diff for the patch is taken against comment.module from 4.5.2. It makes some minor additions to function comment_admin_edit(). -Ankur ankur

6 13

[drupal-devel] [bug] have db_prefix_tables() handle 'default' key not being set
by danielc 08 Apr '05

08 Apr '05

Issue status update for http://drupal.org/node/19609 Project: Drupal Version: cvs Component: base system Category: bug reports Priority: normal Assigned to: Anonymous Reported by: danielc Updated by: danielc Status: patch And the patch to issue 19474 (http://drupal.org/node/19474) contains a patch to provide such information to settings.php. danielc Previous comments: ------------------------------------------------------------------------ March 28, 2005 - 16:24 : danielc Attachment: http://drupal.org/files/issues/database.diff (720 bytes) The db_prefix_tables() function in includes/database.inc generates notices if $db_prefix['default'] is not set. This patch fixes that problem. This has a side benefit of making $db_prefix easier to use. ------------------------------------------------------------------------ March 28, 2005 - 17:06 : moshe weitzman nice ... this has been a bit a pimple for a while. ------------------------------------------------------------------------ March 29, 2005 - 10:40 : danielc Attachment: http://drupal.org/files/issues/database2.diff (1.17 KB) I've come up with a new version of the patch that optimizes the speed of the whole function. ------------------------------------------------------------------------ March 29, 2005 - 10:56 : danielc Change Version from "4.5.2" to "cvs". ------------------------------------------------------------------------ March 29, 2005 - 11:01 : danielc Pardon me, I'm testing if this goes the drupal-dev list now that the version is "cvs." ------------------------------------------------------------------------ April 3, 2005 - 07:45 : Dries Any idea of how much faster the second patch is? I don't think it makes a real difference in which case I'd favor the first patch. ------------------------------------------------------------------------ April 3, 2005 - 13:40 : danielc The speed increase is 5%. ------------------------------------------------------------------------ April 3, 2005 - 13:52 : Dries 5% compared to what? What did you measure? ------------------------------------------------------------------------ April 3, 2005 - 23:56 : danielc I tested the db_prefix_tables() function in two configurations. With my first patch applied and with my second patch applied. The second patch is 5% faster than the first. This improvement is the same for both the string and array formats of $db_url. ------------------------------------------------------------------------ April 7, 2005 - 19:43 : danielc Can this please be applied to 4-6 and HEAD? ------------------------------------------------------------------------ April 8, 2005 - 06:34 : Dries I was checking settings.php to read up on how to use $db_prefix but noticed that only the 'simple' case was documented. The fact that $db_prefix can be an array isn't explained anywhere (not in the phpdoc of db_prefix_tables() and not in settings.php). Where did that go? I wanted to check what the documentation had to say on the availability of a 'default' entry ... ------------------------------------------------------------------------ April 8, 2005 - 07:11 : moshe weitzman docs in the administrators guide - http://drupal.org/node/2622

1 0

[drupal-devel] [feature] Link to node on comment edit page
by Dries 08 Apr '05

08 Apr '05

Issue status update for http://drupal.org/node/11782 Project: Drupal Version: cvs Component: comment.module Category: feature requests Priority: normal Assigned to: Anonymous Reported by: kps Updated by: Dries Status: patch I agree with Steven. This doesn't look like the proper fix. Maybe this is automagically fixed by Moshe's comment patch ... ? Dries Previous comments: ------------------------------------------------------------------------ October 20, 2004 - 17:31 : kps Attachment: http://drupal.org/files/issues/comment.module.patch (579 bytes) We have had users manage to post their comments on the wrong nodes, and when reviewing a newly submitted comment via admin/comment/list/approval, there seems to be no practical way to tell which node the comment belongs to. The attached patch adds a link to the node on the comment edit page. ------------------------------------------------------------------------ March 13, 2005 - 21:06 : killes(a)www.drop.org The patch is in html. ------------------------------------------------------------------------ March 26, 2005 - 20:15 : kps Attachment: http://drupal.org/files/issues/comment-admin-show-node.patch (579 bytes) Interesting! The patch wasn't in HTML when I uploaded it, and the stated size (579 bytes) matches the size of the plain text file, not the size of the HTML. I'll try again.... ------------------------------------------------------------------------ March 27, 2005 - 23:40 : Steven Perhaps this should be fixed through a breadcrumb instead?

1 0

[drupal-devel] [bug] race condition in variable_set in bootstrap.inc
by jhriggs 08 Apr '05

08 Apr '05

Issue status update for http://drupal.org/node/20114 Project: Drupal Version: cvs Component: base system Category: bug reports Priority: critical Assigned to: Ricetons Reported by: Ricetons Updated by: jhriggs -Status: active +Status: patch Attachment: http://drupal.org/files/issues/variable_set_update.patch (922 bytes) I'm not sure that there is a "right" way to fix this. In mysql, we could use a REPLACE rather than DELETE/INSERT, but that wouldn't work for pgsql. And, if we are really seeing this type of race, there is no telling which REPLACE would actually make it last and be saved. Instead, we could do a check to see if the row exists and do an UPDATE if it does and an INSERT otherwise; however, this would be prone to the same problem if we are seeing such a race. That is, they may both see that the row does not exist and both attempt to do an INSERT. This will only happen the first time a variable is created, though, which may be more acceptable than what we are seeing now. If the row does already exist, they would both do UPDATES and would "succeed" in the sense that there is no duplicate key error. There is still no telling which one actually sticks, though. The attached patch takes the second approach. jhriggs Previous comments: ------------------------------------------------------------------------ April 6, 2005 - 21:53 : Ricetons This bug was exposed by the poormancron module. We can see that the module itself contains no bug. But in the function variable_get(), the function just execute SQL like this: DELETE bar WHERE key="foo" INSERT INTO bar VALUES(foo, bar2ooo) but suppose this: thread A and thread B has executed the DELETE sentence. And they do INSERT at the same time. then an SQL error like this occured: duplicated key 'foo' in table "variables" It's a critical bug. It should be fixed before the time that drupal 4.6.0 released. ------------------------------------------------------------------------ April 8, 2005 - 02:01 : robertDouglass I also experience this on a site that has many different cron cycles ranging from 2 minutes to 6 hours. The query errors which arise (as noted above) are impossible to reproduce through normal testing yet occur fairly (surprisingly) often in the logs. This doesn't just apply to cron jobs but rather to all code that calls variable_set. Setting to CVS since this hasn't been addressed since the issue was opened.

1 0

[drupal-devel] [feature] Separate prefetch request log
by drumm 08 Apr '05

08 Apr '05

Issue status update for http://drupal.org/node/19781 Project: Drupal Version: cvs Component: statistics.module Category: feature requests Priority: normal Assigned to: drumm Reported by: drumm Updated by: drumm Status: patch Attachment: http://drupal.org/files/issues/statistics.module.diff (6.15 KB) So Google went and put prefetch in search results [1]. I predict that the semantic HTML with CSS community will start using this more and more. And we might find places to use it in Drupal. Since these prefetch requests aren't exactly the same as other requests we should probably have another log for those hits. [1] http://www.google.com/googleblog/2005/03/enhanced-searching-with-firefox.ht… drumm

2 1