Merge pull request #2879 from annando/1610-performance-too

Avoiding deadlocks and small sql improvements

include/dbstructure.php

@@ -918,7 +918,7 @@ function db_definition($charset) {
 					"ownerid_created" => array("owner-id","created"),
 					"wall_body" => array("wall","body(6)"),
 					"uid_visible_moderated_created" => array("uid","visible","moderated","created"),
-					"uid_uri" => array("uid","uri"),
+					"uid_uri" => array("uid", "uri"),
 					"uid_wall_created" => array("uid","wall","created"),
 					"resource-id" => array("resource-id"),
 					"uid_type" => array("uid","type"),

include/items.php

@@ -730,6 +730,7 @@ function item_store($arr,$force_parent = false, $notify = false, $dontcache = fa
 	if ($arr["uid"] == 0) {
 		$arr["global"] = true;
 
+		// Set the global flag on all items if this was a global item entry
 		q("UPDATE `item` SET `global` = 1 WHERE `uri` = '%s'", dbesc($arr["uri"]));
 	} else {
 		$isglobal = q("SELECT `global` FROM `item` WHERE `uid` = 0 AND `uri` = '%s'", dbesc($arr["uri"]));
@@ -763,6 +764,17 @@ function item_store($arr,$force_parent = false, $notify = false, $dontcache = fa
 		return 0;
 	}
 
+	// Check for already added items.
+	// There is a timing issue here that sometimes creates double postings.
+	// An unique index would help - but the limitations of MySQL (maximum size of index values) prevent this.
+	if ($arr["uid"] == 0) {
+		$r = qu("SELECT `id` FROM `item` WHERE `uri` = '%s' AND `uid` = 0 LIMIT 1", dbesc(trim($arr['uri'])));
+		if (dbm::is_result($r)) {
+			logger('Global item already stored. URI: '.$arr['uri'].' on network '.$arr['network'], LOGGER_DEBUG);
+			return 0;
+		}
+	}
+
 	// Store the unescaped version
 	$unescaped = $arr;
 
@@ -777,41 +789,67 @@ function item_store($arr,$force_parent = false, $notify = false, $dontcache = fa
 			. implode("`, `", array_keys($arr))
 			. "`) VALUES ('"
 			. implode("', '", array_values($arr))
-			. "')" );
+			. "')");
 
 	// And restore it
 	$arr = $unescaped;
 
-	// find the item that we just created
+	// When the item was successfully stored we fetch the ID of the item.
-	$r = q("SELECT `id` FROM `item` WHERE `uri` = '%s' AND `uid` = %d AND `network` = '%s' ORDER BY `id` ASC",
+	if (dbm::is_result($r)) {
+		$r = q("SELECT LAST_INSERT_ID() AS `item-id`");
+		if (dbm::is_result($r)) {
+			$current_post = $r[0]['item-id'];
+		} else {
+			// This shouldn't happen
+			$current_post = 0;
+		}
+	} else {
+		// This can happen - for example - if there are locking timeouts.
+		logger("Item wasn't stored - we quit here.");
+		q("COMMIT");
+		return 0;
+	}
+
+	if ($current_post == 0) {
+		// This is one of these error messages that never should occur.
+		logger("couldn't find created item - we better quit now.");
+		q("COMMIT");
+		return 0;
+	}
+
+	// How much entries have we created?
+	// We wouldn't need this query when we could use an unique index - but MySQL has length problems with them.
+	$r = q("SELECT COUNT(*) AS `entries` FROM `item` WHERE `uri` = '%s' AND `uid` = %d AND `network` = '%s'",
 		dbesc($arr['uri']),
 		intval($arr['uid']),
 		dbesc($arr['network'])
 	);
 
-	if (count($r) > 1) {
+	if (!dbm::is_result($r)) {
-		// There are duplicates. Keep the oldest one, delete the others
+		// This shouldn't happen, since COUNT always works when the database connection is there.
-		logger('item_store: duplicated post occurred. Removing newer duplicates. uri = '.$arr['uri'].' uid = '.$arr['uid']);
+		logger("We couldn't count the stored entries. Very strange ...");
-		q("DELETE FROM `item` WHERE `uri` = '%s' AND `uid` = %d AND `network` = '%s' AND `id` > %d",
-			dbesc($arr['uri']),
-			intval($arr['uid']),
-			dbesc($arr['network']),
-			intval($r[0]["id"])
-		);
-		q("COMMIT");
-		return 0;
-	} elseif (count($r)) {
-
-		$current_post = $r[0]['id'];
-		logger('item_store: created item ' . $current_post);
-
-		item_set_last_item($arr);
-	} else {
-		logger('item_store: could not locate created item');
 		q("COMMIT");
 		return 0;
 	}
 
+	if ($r[0]["entries"] > 1) {
+		// There are duplicates. We delete our just created entry.
+		logger('Duplicated post occurred. uri = '.$arr['uri'].' uid = '.$arr['uid']);
+
+		// Yes, we could do a rollback here - but we are having many users with MyISAM.
+		q("DELETE FROM `item` WHERE `id` = %d", intval($current_post));
+		q("COMMIT");
+		return 0;
+	} elseif ($r[0]["entries"] == 0) {
+		// This really should never happen since we quit earlier if there were problems.
+		logger("Something is terribly wrong. We haven't found our created entry.");
+		q("COMMIT");
+		return 0;
+	}
+
+	logger('item_store: created item '.$current_post);
+	item_set_last_item($arr);
+
 	if (!$parent_id || ($arr['parent-uri'] === $arr['uri']))
 		$parent_id = $current_post;
 
@@ -855,19 +893,6 @@ function item_store($arr,$force_parent = false, $notify = false, $dontcache = fa
 		);
 	}
 
-
-	/**
-	 * If this is now the last-child, force all _other_ children of this parent to *not* be last-child
-	 */
-
-	if ($arr['last-child']) {
-		$r = q("UPDATE `item` SET `last-child` = 0 WHERE `parent-uri` = '%s' AND `uid` = %d AND `id` != %d",
-			dbesc($arr['uri']),
-			intval($arr['uid']),
-			intval($current_post)
-		);
-	}
-
 	$deleted = tag_deliver($arr['uid'],$current_post);
 
 	// current post can be deleted if is for a community page and no mention are
@@ -884,20 +909,33 @@ function item_store($arr,$force_parent = false, $notify = false, $dontcache = fa
 			logger('item_store: new item not found in DB, id ' . $current_post);
 	}
 
+	if ($arr['parent-uri'] === $arr['uri']) {
+		add_thread($current_post);
+	} else {
+		update_thread($parent_id);
+	}
+
+	q("COMMIT");
+
+	// Due to deadlock issues with the "term" table we are doing these steps after the commit.
+	// This is not perfect - but a workable solution until we found the reason for the problem.
 	create_tags_from_item($current_post);
 	create_files_from_item($current_post);
 
-	// Only check for notifications on start posts
+	// If this is now the last-child, force all _other_ children of this parent to *not* be last-child
-	if ($arr['parent-uri'] === $arr['uri']) {
+	// It is done after the transaction to avoid dead locks.
-		add_thread($current_post);
+	if ($arr['last-child']) {
-		q("COMMIT");
+		$r = q("UPDATE `item` SET `last-child` = 0 WHERE `parent-uri` = '%s' AND `uid` = %d AND `id` != %d",
+			dbesc($arr['uri']),
+			intval($arr['uid']),
+			intval($current_post)
+		);
+	}
 
+	if ($arr['parent-uri'] === $arr['uri']) {
 		add_shadow_thread($current_post);
 	} else {
-		update_thread($parent_id);
+		add_shadow_entry($current_post);
-		q("COMMIT");
-
-		add_shadow_entry($arr);
 	}
 
 	check_item_notification($current_post, $uid);

include/notifier.php

@@ -599,10 +599,10 @@ function notifier_run(&$argv, &$argc){
 
 			foreach($r as $rr) {
 				if((! $mail) && (! $fsuggest) && (! $followup)) {
-					q("insert into deliverq ( `cmd`,`item`,`contact` ) values ('%s', %d, %d )",
+					q("INSERT INTO `deliverq` (`cmd`,`item`,`contact`) VALUES ('%s', %d, %d)
-						dbesc($cmd),
+						ON DUPLICATE KEY UPDATE `cmd` = '%s', `item` = %d, `contact` = %d",
-						intval($item_id),
+						dbesc($cmd), intval($item_id), intval($rr['id']),
-						intval($rr['id'])
+						dbesc($cmd), intval($item_id), intval($rr['id'])
 					);
 				}
 			}

include/threads.php

@@ -23,7 +23,7 @@ function add_thread($itemid, $onlyshadow = false) {
 }
 
 /**
- * @brief Add a shadow entry for a given item id
+ * @brief Add a shadow entry for a given item id that is a thread starter
  *
  * We store every public item entry additionally with the user id "0".
  * This is used for the community page and for the search.
@@ -97,7 +97,15 @@ function add_shadow_thread($itemid) {
 			unset($item[0]['id']);
 			$item[0]['uid'] = 0;
 			$item[0]['origin'] = 0;
+			$item[0]['wall'] = 0;
 			$item[0]['contact-id'] = get_contact($item[0]['author-link'], 0);
+
+			if (in_array($item[0]['type'], array("net-comment", "wall-comment"))) {
+				$item[0]['type'] = 'remote-comment';
+			} elseif ($item[0]['type'] == 'wall') {
+				$item[0]['type'] = 'remote';
+			}
+
 			$public_shadow = item_store($item[0], false, false, true);
 
 			logger("Stored public shadow for thread ".$itemid." under id ".$public_shadow, LOGGER_DEBUG);
@@ -105,7 +113,17 @@ function add_shadow_thread($itemid) {
 	}
 }
 
-function add_shadow_entry($item) {
+/**
+ * @brief Add a shadow entry for a given item id that is a comment
+ *
+ * This function does the same like the function above - but for comments
+ *
+ * @param integer $itemid Item ID that should be added
+ */
+function add_shadow_entry($itemid) {
+
+	$items = q("SELECT * FROM `item` WHERE `id` = %d", intval($itemid));
+	$item = $items[0];
 
 	// Is this a shadow entry?
 	if ($item['uid'] == 0)
@@ -127,7 +145,16 @@ function add_shadow_entry($item) {
 
 	unset($item['id']);
 	$item['uid'] = 0;
+	$item['origin'] = 0;
+	$item['wall'] = 0;
 	$item['contact-id'] = get_contact($item['author-link'], 0);
+
+	if (in_array($item['type'], array("net-comment", "wall-comment"))) {
+		$item['type'] = 'remote-comment';
+	} elseif ($item['type'] == 'wall') {
+		$item['type'] = 'remote';
+	}
+
 	$public_shadow = item_store($item, false, false, true);
 
 	logger("Stored public shadow for comment ".$item['uri']." under id ".$public_shadow, LOGGER_DEBUG);

mod/item.php

@@ -115,7 +115,7 @@ function item_post(&$a) {
 		if(($r === false) || (! count($r))) {
 			notice( t('Unable to locate original post.') . EOL);
 			if(x($_REQUEST,'return'))
-				goaway($a->get_baseurl() . "/" . $return_path );
+				goaway($return_path);
 			killme();
 		}
 		$parent_item = $r[0];
@@ -197,7 +197,7 @@ function item_post(&$a) {
 	if((x($_REQUEST,'commenter')) && ((! $parent) || (! $parent_item['wall']))) {
 		notice( t('Permission denied.') . EOL) ;
 		if(x($_REQUEST,'return'))
-			goaway($a->get_baseurl() . "/" . $return_path );
+			goaway($return_path);
 		killme();
 	}
 
@@ -209,7 +209,7 @@ function item_post(&$a) {
 	if((! can_write_wall($a,$profile_uid)) && (! $allow_moderated)) {
 		notice( t('Permission denied.') . EOL) ;
 		if(x($_REQUEST,'return'))
-			goaway($a->get_baseurl() . "/" . $return_path );
+			goaway($return_path);
 		killme();
 	}
 
@@ -339,7 +339,7 @@ function item_post(&$a) {
 				killme();
 			info( t('Empty post discarded.') . EOL );
 			if(x($_REQUEST,'return'))
-				goaway($a->get_baseurl() . "/" . $return_path );
+				goaway($return_path);
 			killme();
 		}
 	}
@@ -756,7 +756,7 @@ function item_post(&$a) {
 	if(x($datarray,'cancel')) {
 		logger('mod_item: post cancelled by plugin.');
 		if($return_path) {
-			goaway($a->get_baseurl() . "/" . $return_path);
+			goaway($return_path);
 		}
 
 		$json = array('cancel' => 1);
@@ -795,7 +795,7 @@ function item_post(&$a) {
 		proc_run(PRIORITY_HIGH, "include/notifier.php", 'edit_post', $post_id);
 		if((x($_REQUEST,'return')) && strlen($return_path)) {
 			logger('return: ' . $return_path);
-			goaway($a->get_baseurl() . "/" . $return_path );
+			goaway($return_path);
 		}
 		killme();
 	} else
@@ -877,17 +877,26 @@ function item_post(&$a) {
 		intval($datarray['visible'])
 	       );
 
-	$r = q("SELECT `id` FROM `item` WHERE `uri` = '%s' LIMIT 1",
+	if (dbm::is_result($r)) {
-		dbesc($datarray['uri']));
+		$r = q("SELECT LAST_INSERT_ID() AS `item-id`");
-	if(!count($r)) {
+		if (dbm::is_result($r)) {
+			$post_id = $r[0]['item-id'];
+		} else {
+			$post_id = 0;
+		}
+	} else {
+		logger('mod_item: unable to create post.');
+		$post_id = 0;
+	}
+
+	if ($post_id == 0) {
 		q("COMMIT");
 		logger('mod_item: unable to retrieve post that was just stored.');
-		notice( t('System error. Post not saved.') . EOL);
+		notice(t('System error. Post not saved.') . EOL);
-		goaway($a->get_baseurl() . "/" . $return_path );
+		goaway($return_path);
 		// NOTREACHED
 	}
 
-	$post_id = $r[0]['id'];
 	logger('mod_item: saved item ' . $post_id);
 
 	$datarray["id"] = $post_id;
@@ -1000,50 +1009,22 @@ function item_post(&$a) {
 		}
 	}
 
+	if ($post_id == $parent) {
+		add_thread($post_id);
+	} else {
+		update_thread($parent, true);
+	}
+
+	q("COMMIT");
+
 	create_tags_from_item($post_id);
 	create_files_from_item($post_id);
 
-	if ($post_id == $parent) {
+	// Insert an item entry for UID=0 for global entries
-		add_thread($post_id);
+	if ($post_id != $parent) {
-		q("COMMIT");
-
 		add_shadow_thread($post_id);
 	} else {
-		update_thread($parent, true);
+		add_shadow_entry($post_id);
-		q("COMMIT");
-
-		// Insert an item entry for UID=0 for global entries
-		// We have to remove or change some data before that,
-		// so that the post appear like a regular received post.
-		// Additionally there is some data that isn't a database field.
-		$arr = $datarray;
-
-		$arr['app'] = $arr['source'];
-		unset($arr['source']);
-
-		unset($arr['self']);
-		unset($arr['wall']);
-		unset($arr['origin']);
-		unset($arr['api_source']);
-		unset($arr['message_id']);
-		unset($arr['profile_uid']);
-		unset($arr['post_id']);
-		unset($arr['dropitems']);
-		unset($arr['commenter']);
-		unset($arr['return']);
-		unset($arr['preview']);
-		unset($arr['post_id_random']);
-		unset($arr['emailcc']);
-		unset($arr['pubmail_enable']);
-		unset($arr['category']);
-		unset($arr['jsreload']);
-
-		if (in_array($arr['type'], array("net-comment", "wall-comment"))) {
-			$arr['type'] = 'remote-comment';
-		} elseif ($arr['type'] == 'wall') {
-			$arr['type'] = 'remote';
-		}
-		add_shadow_entry($arr);
 	}
 
 	// This is a real juggling act on shared hosting services which kill your processes
@@ -1070,7 +1051,7 @@ function item_post_return($baseurl, $api_source, $return_path) {
 		return;
 
 	if($return_path) {
-		goaway($baseurl . "/" . $return_path);
+		goaway($return_path);
 	}
 
 	$json = array('success' => 1);