Closed (fixed)
Project:
Webform
Version:
6.x-3.x-dev
Component:
Code
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
3 Mar 2009 at 04:12 UTC
Updated:
25 May 2011 at 05:21 UTC
Jump to comment: Most recent file
Comments
Comment #1
quicksketchIt's important to know that component IDs are unique per-node. So a typical auto-increment won't work with the way Webform currently operates. I certainly wouldn't mind fixing this problem if you find a way around it.
Comment #2
damienmckennaThe heart of this is actually an urge to remove the LOCK statements. Given that the statement would fail if there happened to be a collision (unlikely to happen), would it be sufficient to just have better error checking?
Comment #3
quicksketchIf a collision was unlikely to occur in your situation, you could think of this as saying "a request waiting for the lock is unlikely to happen" also. I know the urge to remove locks might be high, but is there any particular problem caused by these locks? If you're in a situation where hundreds of components are being created a minute, Webform will likely run into many other scalability problems than this single table lock. I don't personally see a problem with the approach that would cause any site noticeable penalties.
Comment #4
mark trappHaving just run into this problem, I'm looking for a solution as well. The main issue I see with this is that Drupal 6 core doesn't require Lock privileges to be granted to a database (http://drupal.org/getting-started/6/install/create-database), so no contributed modules should, either. That is, I shouldn't have to modify my database environment just to get a contributed module to work correctly. I don't want to step on any toes by modifying the issue, but this issue seems more like a bug report than a feature request.
Comment #5
damienmckennaAmorfati: I think at the heart of this is that:
I don't think quicksketch would complain if someone were to submit a patch which resolved this, and I'm more than happy to test one, but it's far from a reasonable priority for anyone right now.
Personally speaking, the only reason I created the issue is because I had a DBA complain "there shouldn't be any manual locks", but Quicksketch's explanation works fine for us, especially given the scenarios we're using it in.
Comment #6
glennpratt commentedIf this isn't going to change, then someone needs to make it clear in documentation and on the download page. You might also throw up an explanation as an error or to the Site Status report.
I'm hoping someone can patch this, because right now people who install Webform on a properly configured Drupal install will receive errors.
Comment #7
gregglesI think it's surprising more than a problem, and the error message is just annoying, not a real problem. Perhaps webform can detect whether the account has this permission and, if not, just skip the lock. That hides the error which eliminates the "problem."
Alternative solutions:
1. Create webform's own locking mechanism using a semaphore of some kind (blech)
2. Create an index on nid and component ID (if there isn't one) and leverage that to enforce the uniqueness
3. A hook_requirements that checks for the permission and, if not there, disables the module until the permission is granted which seems bad b/c for most sites the lack of this lock isn't actually a problem for the module to be able to function properly
Comment #8
quicksketchJust a note that Drupal 7 has a built-in semaphore system which would suit this purpose. Considering the table locking isn't a problem for most sites, we'll probably leave it as-is in Drupal 6 and implement it in the 3.x version for D7. In the mean time I've update the project page to reflect the requirement.
Comment #9
markDrupal commentedThe lock seems to prevent 2 users working on the same form from submitting a new component at the same time.
Views has some sort of locking similar to what you need here. I don't believe Views allows 2 sessions to work on the same view at the same time.
Comment #10
quicksketchThe lock is what makes 2 users working on the same form possible. There is just a slight delay before a user's request is processed. So if 2 users clicked "Save" at the exact same time, one user's request would be processed first, while the other has to wait 5ms more until the lock is released. The lock actually exists specifically to make it so that this situation does not cause a problem, otherwise users could end up overwriting each other's data.
Comment #11
crutch commentedAttempting to add a recipient to the form is causing this WSOC but shows error on screen, d6.15 >> Warning: Table 'watchdog' was not locked with LOCK TABLES query: INSERT INTO watchdog (uid, type, message, variables, severity, link, location, referer, hostname, timestamp) VALUES (1, 'php', '%message in %file on line %line.', 'a:4:{s:6:\"%error\";s:12:\"user warning\";s:8:\"%message\";s:334:\"Unknown column 'excluded_components' in 'field list'\nquery: INSERT INTO webform_emails (nid, eid, email, subject, from_name, from_address, template, excluded_components) VALUES (8, 1, 'me@mysite.com', 'default', 'default', 'default', 'default', '')\";s:5:\"%file\";s:99:\"C:\\TWAMPd_2010\\htdocs\\sites\\all\\modules\\webform\\includes\\webform.emails.inc\";s:5:\"%line\";i:465;}', 3, '', 'http://localhost/node/ in C:\TWAMPd_2010\htdocs\includes\database.mysql.inc on line 128
--------
Then to go back and visit the form this error is shown, but the theme comes back >> user warning: Unknown column 'excluded_components' in 'field list' query: INSERT INTO webform_emails (nid, eid, email, subject, from_name, from_address, template, excluded_components) VALUES (8, 1, 'me@mysite.com', 'default', 'default', 'default', 'default', '') in C:\TWAMPd_2010\htdocs\sites\all\modules\webform\includes\webform.emails.inc on line 465.
Comment #12
crutch commentedComment #13
quicksketchThe last warning is probably the one you need to be careful about. Did you run update.php since you last updated? It sounds like excluded_components is not a column in your webform_emails database table. I think that is the only issue here and it's not related to the table locking.
Comment #14
crutch commentedThanks QS. I was just starting to build a site and decided to use 3.
I did an initial install with all my standard modules I had locally including 2.9. I then updated a few one at a time doing update.php after each one. Then I saw webform 3 was available so I gave it a shot by downloading, and doing update.php. Everything seemed to work great and I like the new layout.
I was able to create the form and move around well but when going to add the recipient I begin encountering the error loop where I can never add the first recipient email address. I since started over with 2.9.
What I'll try to do is once I get the site build with Webform 2.9, I'll create a copy of the site and then try and upgrade to 3.
Comment #15
quicksketchMoving this to won't fix. Anyone interested in providing a patch may do so, but it doesn't seem like anyone thinks this is a high enough priority to fix since it's almost entirely a philosophical (rather than practical) issue about avoiding table locks.
Comment #16
dstolSubscribe
Comment #17
ben.bunk commentedsubscribe
Comment #18
dstol@quicksketch, it seems that the philosophical has become reality in my case.
Patches against 6.x-2.x and 6.x-3.x.
Comment #19
dstolReally more of a bug report as we're getting time outs while editing webform components.
Comment #20
quicksketchThanks dstol, this looks great! The lock_* functions only became available in some middle version of Drupal 6 (like 6.12 or something), so we'll need to proceed with documentation about this requirement. Great patch though, I'll take a look next time I'm reviewing patches.
Comment #21
quicksketchAh, feels nice to finally fix this issue. I've committed these two patches to the 3.x branches. No new releases of 2.x will be made.
There was a bit of a bug in the 3.x patches with this typo:
lock_acquire('webofmr_component_insert'), but while fixing it I decided to make these locks per-node anyway. So now users have to be editing the exact same node to run into this problem as opposed to having two completely separate nodes locking each other.In Drupal 6 I also updated the README.txt and added a hook_requirements() to make sure users have Drupal 6.16, in which the locking framework was added. For reference, see #251792: Implement a locking framework for long operations.
Comment #22
quicksketch