From 16a60253721330323ae201032f0b852697ce2a00 Mon Sep 17 00:00:00 2001 From: Andreas Fischer Date: Thu, 21 Mar 2013 23:04:17 +0100 Subject: [ticket/11469] Add SQL insert buffer allowing easier handling of multi inserts. 1. Tries to prevent going over max packet size by flushing to the database after a certain number of rows have been added. 2. Because of 1., it is less likely to reach a connection timeout when inserting a huge number of rows. 3. By flushing the buffer when a certain size is reached, memory usage should be lower compared to building the whole insert row set first. PHPBB3-11469 --- phpBB/includes/db/sql_insert_buffer.php | 111 ++++++++++++++++++++++++++++++++ 1 file changed, 111 insertions(+) create mode 100644 phpBB/includes/db/sql_insert_buffer.php (limited to 'phpBB/includes/db/sql_insert_buffer.php') diff --git a/phpBB/includes/db/sql_insert_buffer.php b/phpBB/includes/db/sql_insert_buffer.php new file mode 100644 index 0000000000..8d4b03ef53 --- /dev/null +++ b/phpBB/includes/db/sql_insert_buffer.php @@ -0,0 +1,111 @@ +db = $db; + $this->db_supports_multi_insert = $db->multi_insert; + $this->table_name = $table_name; + $this->max_buffered_rows = $max_buffered_rows; + } + + /** + * Inserts a single row into the buffer if multi insert is supported by the + * database (otherwise an insert query is sent immediately). Then flushes + * the buffer if the number of rows in the buffer is now greater than or + * equal to $max_buffered_rows. + * + * @param array $row + * + * @return null + */ + public function insert(array $row) + { + if (!$this->db_supports_multi_insert) + { + $this->db->sql_multi_insert($this->table_name, array($row)); + } + + $this->buffer[] = $row; + + if (sizeof($this->buffer) >= $this->max_buffered_rows) + { + $this->flush(); + } + } + + /** + * Inserts a row set, i.e. an array of rows, by calling insert(). + * + * Please note that it is in most cases better to use insert() instead of + * first building a huge rowset. Or at least sizeof($rows) should be kept + * small. + * + * @param array $rows + * + * @return null + */ + public function insert_all(array $rows) + { + foreach ($rows as $row) + { + $this->insert($row); + } + } + + /** + * Flushes the buffer content to the DB and clears the buffer. + * + * @return null + */ + public function flush() + { + if (!empty($this->buffer)) + { + $this->db->sql_multi_insert($this->table_name, $this->buffer); + $this->buffer = array(); + } + } +} -- cgit v1.2.1 From fc8bf3f3c767067a03d240403598d62fb22ce889 Mon Sep 17 00:00:00 2001 From: Andreas Fischer Date: Mon, 25 Mar 2013 01:41:09 +0100 Subject: [ticket/11469] Add comment about using sql_multi_insert when not buffering. PHPBB3-11469 --- phpBB/includes/db/sql_insert_buffer.php | 3 +++ 1 file changed, 3 insertions(+) (limited to 'phpBB/includes/db/sql_insert_buffer.php') diff --git a/phpBB/includes/db/sql_insert_buffer.php b/phpBB/includes/db/sql_insert_buffer.php index 8d4b03ef53..fe45206893 100644 --- a/phpBB/includes/db/sql_insert_buffer.php +++ b/phpBB/includes/db/sql_insert_buffer.php @@ -65,6 +65,9 @@ class phpbb_db_sql_insert_buffer { if (!$this->db_supports_multi_insert) { + // The database does not support multi inserts. + // Pass data on to sql_multi_insert right away which will + // immediately send an INSERT INTO query to the database. $this->db->sql_multi_insert($this->table_name, array($row)); } -- cgit v1.2.1 From bf6f2c5875f2476366c1bd660506e70c0f006c9d Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 27 Mar 2013 11:47:40 +0100 Subject: [ticket/11469] Return after sql_multi_insert when multi_insert is false PHPBB3-11469 --- phpBB/includes/db/sql_insert_buffer.php | 1 + 1 file changed, 1 insertion(+) (limited to 'phpBB/includes/db/sql_insert_buffer.php') diff --git a/phpBB/includes/db/sql_insert_buffer.php b/phpBB/includes/db/sql_insert_buffer.php index fe45206893..49cf5b8ef6 100644 --- a/phpBB/includes/db/sql_insert_buffer.php +++ b/phpBB/includes/db/sql_insert_buffer.php @@ -69,6 +69,7 @@ class phpbb_db_sql_insert_buffer // Pass data on to sql_multi_insert right away which will // immediately send an INSERT INTO query to the database. $this->db->sql_multi_insert($this->table_name, array($row)); + return; } $this->buffer[] = $row; -- cgit v1.2.1 From 94a15f85a64db283a9c9402c40b2d35cb484fb37 Mon Sep 17 00:00:00 2001 From: Andreas Fischer Date: Wed, 27 Mar 2013 18:06:48 +0100 Subject: [ticket/11469] Add example code to class documentation. PHPBB3-11469 --- phpBB/includes/db/sql_insert_buffer.php | 15 +++++++++++++++ 1 file changed, 15 insertions(+) (limited to 'phpBB/includes/db/sql_insert_buffer.php') diff --git a/phpBB/includes/db/sql_insert_buffer.php b/phpBB/includes/db/sql_insert_buffer.php index fe45206893..772f368987 100644 --- a/phpBB/includes/db/sql_insert_buffer.php +++ b/phpBB/includes/db/sql_insert_buffer.php @@ -19,6 +19,21 @@ if (!defined('IN_PHPBB')) * Collects rows for insert into a database until the buffer size is reached. * Then flushes the buffer to the database and starts over again. * +* Usage: +* +* $buffer = new phpbb_db_sql_insert_buffer($db, 'test_table', 1234); +* +* while (do_stuff()) +* { +* $buffer->insert(array( +* 'column1' => 'value1', +* 'column2' => 'value2', +* )); +* } +* +* $buffer->flush(); +* +* * @package dbal */ class phpbb_db_sql_insert_buffer -- cgit v1.2.1 From 8c5fcac2325356bacb72517f6fbd95f9bfdaf16d Mon Sep 17 00:00:00 2001 From: Andreas Fischer Date: Wed, 27 Mar 2013 18:22:59 +0100 Subject: [ticket/11469] Add benefits over collecting huge insert arrays to class doc. PHPBB3-11469 --- phpBB/includes/db/sql_insert_buffer.php | 12 ++++++++++++ 1 file changed, 12 insertions(+) (limited to 'phpBB/includes/db/sql_insert_buffer.php') diff --git a/phpBB/includes/db/sql_insert_buffer.php b/phpBB/includes/db/sql_insert_buffer.php index 6b884dd412..4bf0608227 100644 --- a/phpBB/includes/db/sql_insert_buffer.php +++ b/phpBB/includes/db/sql_insert_buffer.php @@ -19,6 +19,18 @@ if (!defined('IN_PHPBB')) * Collects rows for insert into a database until the buffer size is reached. * Then flushes the buffer to the database and starts over again. * +* Benefits over collecting a (possibly huge) insert array and then using +* $db->sql_multi_insert() include: +* +* - Going over max packet size of the database connection is usually prevented +* because the data is submitted in batches. +* +* - Reaching database connection timeout is usually prevented because +* submission of batches talks to the database every now and then. +* +* - Usage of less PHP memory because data no longer needed is discarded on +* buffer flush. +* * Usage: * * $buffer = new phpbb_db_sql_insert_buffer($db, 'test_table', 1234); -- cgit v1.2.1 From dc766f29b4381e3cecfacbfeb848b4e13c3e48f9 Mon Sep 17 00:00:00 2001 From: Andreas Fischer Date: Wed, 27 Mar 2013 19:19:26 +0100 Subject: [ticket/11469] Have all methods of phpbb_db_sql_insert_buffer provide feedback. PHPBB3-11469 --- phpBB/includes/db/sql_insert_buffer.php | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) (limited to 'phpBB/includes/db/sql_insert_buffer.php') diff --git a/phpBB/includes/db/sql_insert_buffer.php b/phpBB/includes/db/sql_insert_buffer.php index 4bf0608227..dd4a62a948 100644 --- a/phpBB/includes/db/sql_insert_buffer.php +++ b/phpBB/includes/db/sql_insert_buffer.php @@ -86,7 +86,8 @@ class phpbb_db_sql_insert_buffer * * @param array $row * - * @return null + * @return bool True when some data was flushed to the database. + * False otherwise. */ public function insert(array $row) { @@ -96,15 +97,18 @@ class phpbb_db_sql_insert_buffer // Pass data on to sql_multi_insert right away which will // immediately send an INSERT INTO query to the database. $this->db->sql_multi_insert($this->table_name, array($row)); - return; + + return true; } $this->buffer[] = $row; if (sizeof($this->buffer) >= $this->max_buffered_rows) { - $this->flush(); + return $this->flush(); } + + return false; } /** @@ -116,20 +120,26 @@ class phpbb_db_sql_insert_buffer * * @param array $rows * - * @return null + * @return bool True when some data was flushed to the database. + * False otherwise. */ public function insert_all(array $rows) { + $result = false; + foreach ($rows as $row) { - $this->insert($row); + $result |= $this->insert($row); } + + return $result; } /** * Flushes the buffer content to the DB and clears the buffer. * - * @return null + * @return bool True when some data was flushed to the database. + * False otherwise. */ public function flush() { @@ -137,6 +147,10 @@ class phpbb_db_sql_insert_buffer { $this->db->sql_multi_insert($this->table_name, $this->buffer); $this->buffer = array(); + + return true; } + + return false; } } -- cgit v1.2.1 From 53f9e2131c9b87d35207ea585981fc9b084d0a11 Mon Sep 17 00:00:00 2001 From: Andreas Fischer Date: Wed, 27 Mar 2013 19:27:27 +0100 Subject: [ticket/11469] Add note about calling flush() after batch insert is done. PHPBB3-11469 --- phpBB/includes/db/sql_insert_buffer.php | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'phpBB/includes/db/sql_insert_buffer.php') diff --git a/phpBB/includes/db/sql_insert_buffer.php b/phpBB/includes/db/sql_insert_buffer.php index dd4a62a948..3bd96616b3 100644 --- a/phpBB/includes/db/sql_insert_buffer.php +++ b/phpBB/includes/db/sql_insert_buffer.php @@ -31,6 +31,11 @@ if (!defined('IN_PHPBB')) * - Usage of less PHP memory because data no longer needed is discarded on * buffer flush. * +* Attention: +* Please note that users of this class have to call flush() to flush the +* remaining rows to the database after their batch insert operation is +* finished. +* * Usage: * * $buffer = new phpbb_db_sql_insert_buffer($db, 'test_table', 1234); -- cgit v1.2.1 From c9f059c4f2793e9f98c3e0fbaad06708dd557d31 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 27 Mar 2013 20:55:48 +0100 Subject: [ticket/11469] Cast $result to boolean in insert_all() |= returns integer values PHPBB3-11469 --- phpBB/includes/db/sql_insert_buffer.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'phpBB/includes/db/sql_insert_buffer.php') diff --git a/phpBB/includes/db/sql_insert_buffer.php b/phpBB/includes/db/sql_insert_buffer.php index 3bd96616b3..dd7932c7bd 100644 --- a/phpBB/includes/db/sql_insert_buffer.php +++ b/phpBB/includes/db/sql_insert_buffer.php @@ -137,7 +137,7 @@ class phpbb_db_sql_insert_buffer $result |= $this->insert($row); } - return $result; + return (bool) $result; } /** -- cgit v1.2.1 From 4132573088c376fcb44cc588d9341c8d38b6d694 Mon Sep 17 00:00:00 2001 From: Andreas Fischer Date: Wed, 27 Mar 2013 23:35:36 +0100 Subject: [ticket/11469] Use buffer with a single element instead of extra code path. PHPBB3-11469 --- phpBB/includes/db/sql_insert_buffer.php | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) (limited to 'phpBB/includes/db/sql_insert_buffer.php') diff --git a/phpBB/includes/db/sql_insert_buffer.php b/phpBB/includes/db/sql_insert_buffer.php index dd7932c7bd..03c8a875b9 100644 --- a/phpBB/includes/db/sql_insert_buffer.php +++ b/phpBB/includes/db/sql_insert_buffer.php @@ -96,19 +96,11 @@ class phpbb_db_sql_insert_buffer */ public function insert(array $row) { - if (!$this->db_supports_multi_insert) - { - // The database does not support multi inserts. - // Pass data on to sql_multi_insert right away which will - // immediately send an INSERT INTO query to the database. - $this->db->sql_multi_insert($this->table_name, array($row)); - - return true; - } - $this->buffer[] = $row; - if (sizeof($this->buffer) >= $this->max_buffered_rows) + // Flush buffer if it is full or when DB does not support multi inserts. + // In the later case, the buffer will always only contain one row. + if (!$this->db_supports_multi_insert || sizeof($this->buffer) >= $this->max_buffered_rows) { return $this->flush(); } -- cgit v1.2.1 From 1bd13acb753553ed5b9ab54144d0ca6507b031a3 Mon Sep 17 00:00:00 2001 From: Andreas Fischer Date: Wed, 27 Mar 2013 23:37:08 +0100 Subject: [ticket/11469] Use multi insert property from DB. Do not copy value to buffer. PHPBB3-11469 --- phpBB/includes/db/sql_insert_buffer.php | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) (limited to 'phpBB/includes/db/sql_insert_buffer.php') diff --git a/phpBB/includes/db/sql_insert_buffer.php b/phpBB/includes/db/sql_insert_buffer.php index 03c8a875b9..46d397e7b4 100644 --- a/phpBB/includes/db/sql_insert_buffer.php +++ b/phpBB/includes/db/sql_insert_buffer.php @@ -58,9 +58,6 @@ class phpbb_db_sql_insert_buffer /** @var phpbb_db_driver */ protected $db; - /** @var bool */ - protected $db_supports_multi_insert; - /** @var string */ protected $table_name; @@ -78,7 +75,6 @@ class phpbb_db_sql_insert_buffer public function __construct(phpbb_db_driver $db, $table_name, $max_buffered_rows = 500) { $this->db = $db; - $this->db_supports_multi_insert = $db->multi_insert; $this->table_name = $table_name; $this->max_buffered_rows = $max_buffered_rows; } @@ -100,7 +96,7 @@ class phpbb_db_sql_insert_buffer // Flush buffer if it is full or when DB does not support multi inserts. // In the later case, the buffer will always only contain one row. - if (!$this->db_supports_multi_insert || sizeof($this->buffer) >= $this->max_buffered_rows) + if (!$this->db->multi_insert || sizeof($this->buffer) >= $this->max_buffered_rows) { return $this->flush(); } -- cgit v1.2.1 From 4bd5f279dc98f036021c04172ecbb30c092de59f Mon Sep 17 00:00:00 2001 From: Andreas Fischer Date: Thu, 28 Mar 2013 00:27:02 +0100 Subject: [ticket/11469] Add comment about using bitwise operator. PHPBB3-11469 --- phpBB/includes/db/sql_insert_buffer.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'phpBB/includes/db/sql_insert_buffer.php') diff --git a/phpBB/includes/db/sql_insert_buffer.php b/phpBB/includes/db/sql_insert_buffer.php index 46d397e7b4..c18f908429 100644 --- a/phpBB/includes/db/sql_insert_buffer.php +++ b/phpBB/includes/db/sql_insert_buffer.php @@ -118,11 +118,12 @@ class phpbb_db_sql_insert_buffer */ public function insert_all(array $rows) { - $result = false; + // Using bitwise |= because PHP does not have logical ||= + $result = 0; foreach ($rows as $row) { - $result |= $this->insert($row); + $result |= (int) $this->insert($row); } return (bool) $result; -- cgit v1.2.1