Local node id's not atomic!
MartysMind - March 11, 2009 - 06:47
| Project: | Type-local nids |
| Version: | 6.x-1.2 |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | jbrown |
| Status: | postponed |
Jump to:
Description
I just took a look at the code for the 5.x branch and noticed that the allocation of local node id's is not atomic. On a server with heavy load you are likely to get multiple nodes with the same local node id.
One solution would be to mimic the Drupal db_next_id() functionality:
function db_next_id($name) {
$name = db_prefix_tables($name);
db_query('LOCK TABLES {sequences} WRITE');
$id = db_result(db_query("SELECT id FROM {sequences} WHERE name = '%s'", $name)) + 1;
db_query("REPLACE INTO {sequences} VALUES ('%s', %d)", $name, $id);
db_query('UNLOCK TABLES');
return $id;
}And lock the table during allocation, or perhaps find a way to add local node ids to Drupals sequence table.
-Marty
#1
You are quite correct. There is a race condition.
A better solution would be to use a database transaction.
This is not a critical issue.
#2
I guess whether it was critical or not would depend on your servers load and if you actually expect your server to work ;-)
#3
I'll set it to postponed until someone creates a patch.
This module should really be implemented as a CCK field.
#4
I have a theory...
If a bug causes a module to be unstable or intermittently fail.. then it is a critical bug.
Why? Simply put if I am selecting modules to implement a production web site I need to know if there are problems that may cause that website to fail unexpectedly.
Of course that may make me a bit of a heretic, since in my view MANY Drupal modules are in the same shape and should really be labeled "beta" instead of "production".
#5
Just make it give the MySQL table an auto-increment. I'm not sure how to do this from within Drupal, so I won't bother. However, I thought that was how this worked, too. MySQL can have auto-incremented integers as value types. PHP just inserts a blank value and it auto-inserts the next number.
#6
I would recommend avoiding certain auto-increment unless you are willing to write support for all SQL databases drupal supports.
For whatever reason, auto-increment does not seem to be an SQL standard.
Therefore, using this means extra code and extra bugs.
For help implementing this in multiple SQL languages, see:
- http://www.w3schools.com/SQL/sql_autoincrement.asp
- http://pointbeing.net/weblog/2008/03/mysql-versus-postgresql-adding-an-a... and/or http://siphu.wordpress.com/2009/08/10/add-auto-increment-column-in-postg...
For whatever reason, postgresql seems to use SERIAL for an equivalent auto-increment.
I strongly suspect Drupal does this already (somewhere), so you may be able to recycle the Drupal core database code to produce a portable solution.
That is, if you are going to use the autoincrement concept from #5.