nodeblock_node_delete() always calls _block_rehash(), which causes a lot of database traffic and seems to be the cause of occasional deadlock messages when deleting nodes.

I assume the _block_rehash() function only needs to be called for nodes which have nodeblock properties, please review the attached patch.

CommentFileSizeAuthor
#2 1531408-2-menublock-block_rehash.patch1.54 KBpfrenssen
nodeblock_node_delete_conditional_rehash.patch798 bytesjoericapens

Comments

pfrenssen’s picture

pfrenssen
Location Sofia
commented
Priority: Normal » Major

This is quite a bit worse. The _block_rehash() is called on every hook_node_insert(), hook_node_update() and hook_node_delete(). Not just for nodeblocks, but for all nodes. On a highly trafficked site with 180 blocks I'm getting integrity constraint violations because of this.

If this condition occurs on node save this will probably cause the node not to be saved successfully, so this might cause data loss. This warrants a major or even critical priority for this issue.

I am not so fond of the patch though. There's no need to check both conditions to see whether _block_rehash() needs to be run, and you forgot to address the one in _nodeblock_insert_update().

pfrenssen’s picture

pfrenssen
Location Sofia
commented
Status: Active » Needs review
StatusFileSize
new 1531408-2-menublock-block_rehash.patch1.54 KB

Here's my take on it.

Johnny vd Laar’s picture

Johnny vd Laar commented
Status: Needs review » Fixed

Committed a fix but modified it a little bit because of issue #1432726: Remove blocks from the block tables when deleting a node we don't need a full block rehash on a node delete. We just need to remove the entries that were already created.

http://drupalcode.org/project/nodeblock.git/commit/94b131e

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.