3746 Tor Didriksen 2012-01-18
Bug#13580775 ASSERTION FAILED: RECORD_LENGTH == M_RECORD_LENGTH
filesort tries to re-use the sort buffer between invocations in order to save
malloc/free overhead.
The fix for Bug 11748783 - 37359: FILESORT CAN BE MORE EFFICIENT.
added an assert that buffer properties (num_records, record_length) are
consistent between invocations. Indeed, they are not necessarily consistent.
Fix: re-allocate the sort buffer if properties change.
@ mysql-test/r/partition.result
New test case.
@ mysql-test/t/partition.test
New test case.
@ sql/filesort.cc
If we already have allocated a sort buffer, then verify that the same properties
(keys, rec_length) that we need for this invocation.
@ sql/filesort_utils.h
Add sort_buffer_properties
@ sql/table.h
Add sort_buffer_properties
@ unittest/gunit/filesort_buffer-t.cc
Test new member funciton.
modified:
mysql-test/r/partition.result
mysql-test/t/partition.test
sql/filesort.cc
sql/filesort_utils.h
sql/table.h
unittest/gunit/filesort_buffer-t.cc
3745 Praveenkumar Hulakund 2012-01-18
BUG#11764724 - 57586: UNKNOWN TABLE WHEN TRYING TO DROP A TABLE AND NO AVAILABLE UNDO
SLOTS LEFT
Modified changes made as part of fix for this bug to fix Perf regression.
FIX for perf regression:
-------------------------------
Current fix had a if condition to check whether return value of function "ha_delete_table"
is "HA_ERR_TOO_MANY_CONCURRENT_TRXS". If yes then I used to "goto err" and in "err:" check
whether error is "HA_ERR_TOO_MANY_CONCURRENT_TRXS" and print error message. So there were
TWO if conditions added to check whether error is "HA_ERR_TOO_MANY_CONCURRENT_TRXS" or not.
These conditions were getting evaluated every time, this caused perf regression.
As a fix, removed both the if conditions and added only one if condition in "if (error)" block.
This condition is added to check whether error is "HA_ERR_TOO_MANY_CONCURRENT_TRXS". If yes
then print error and then "goto err". Now, this condition is checked only if there is any error.
Otherwise it will not be evaluated and so performance should not be affected.
Actual Issue Description:
-------------------------------
If you attempt to drop an existing InnoDB table, but you do not have any
available undo slots open, then you will receive an "unknown table" error.
mysql> DROP TABLE test.innodb_table_monitor;
ERROR 1051 (42S02): Unknown table 'innodb_table_monitor'
Actual issue Analysis:
-------------------------------
Here, max number of concurrent transactions/connections are started and then the
next transaction to drop a table is initiated. Since, system has already max number
of transactions running, next transaction to "drop table" was not started and innodb
returned error "DB_TOO_MANY_CONCURRENT_TRXS". But this error was not handled properly
in sql layer because of which "drop table" operation was throwing wrong error message.
Fix:
-------------------------------
As a fix, I have added check in sql (in function "mysql_rm_table_part2") to handle error
code "HA_ERR_TOO_MANY_CONCURRENT_TRXS" and throw proper error message.
Ouput of drop command after fix (with max number of concurrent transaction running)
mysql> drop table test;
ERROR 177 (HY000): Too many active concurrent transactions
modified:
sql/sql_table.cc
=== modified file 'mysql-test/r/partition.result'
--- a/mysql-test/r/partition.result 2011-10-06 09:50:40 +0000
+++ b/mysql-test/r/partition.result 2012-01-18 13:03:10 +0000
@@ -2429,3 +2429,25 @@ SELECT * FROM vtmp;
1
DROP VIEW vtmp;
DROP TABLE t1;
+#
+# Bug#13580775 ASSERTION FAILED: RECORD_LENGTH == M_RECORD_LENGTH,
+# FILE FILESORT_UTILS.CC
+#
+CREATE TABLE t1 (
+a INT PRIMARY KEY,
+b INT,
+c CHAR(1),
+d INT,
+KEY (c,d)
+) PARTITION BY KEY () PARTITIONS 1;
+INSERT INTO t1 VALUES (1,1,'a',1), (2,2,'a',1);
+SELECT 1 FROM t1 WHERE 1 IN
+(SELECT group_concat(b)
+FROM t1
+WHERE c > geomfromtext('point(1 1)')
+GROUP BY b
+);
+1
+1
+1
+DROP TABLE t1;
=== modified file 'mysql-test/t/partition.test'
--- a/mysql-test/t/partition.test 2011-10-06 09:50:40 +0000
+++ b/mysql-test/t/partition.test 2012-01-18 13:03:10 +0000
@@ -2433,3 +2433,27 @@ SELECT 1 FROM t1 AS t1_0 JOIN t1 ON t1_0
SELECT * FROM vtmp;
DROP VIEW vtmp;
DROP TABLE t1;
+
+--echo #
+--echo # Bug#13580775 ASSERTION FAILED: RECORD_LENGTH == M_RECORD_LENGTH,
+--echo # FILE FILESORT_UTILS.CC
+--echo #
+
+CREATE TABLE t1 (
+ a INT PRIMARY KEY,
+ b INT,
+ c CHAR(1),
+ d INT,
+ KEY (c,d)
+) PARTITION BY KEY () PARTITIONS 1;
+
+INSERT INTO t1 VALUES (1,1,'a',1), (2,2,'a',1);
+
+SELECT 1 FROM t1 WHERE 1 IN
+(SELECT group_concat(b)
+ FROM t1
+ WHERE c > geomfromtext('point(1 1)')
+ GROUP BY b
+);
+
+DROP TABLE t1;
=== modified file 'sql/filesort.cc'
--- a/sql/filesort.cc 2012-01-12 14:53:51 +0000
+++ b/sql/filesort.cc 2012-01-18 13:03:10 +0000
@@ -38,6 +38,7 @@
#include "opt_trace.h"
#include <algorithm>
+#include <utility>
using std::max;
using std::min;
@@ -284,6 +285,19 @@ ha_rows filesort(THD *thd, TABLE *table,
{
ha_rows keys= memory_available / (param.rec_length + sizeof(char*));
param.max_keys_per_buffer= (uint) min(num_rows, keys);
+ if (table_sort.get_sort_keys())
+ {
+ // If we have already allocated a buffer, it better have same size!
+ if (std::make_pair(param.max_keys_per_buffer, param.rec_length) !=
+ table_sort.sort_buffer_properties())
+ {
+ /*
+ table->sort will still have a pointer to the same buffer,
+ but that will be overwritten by the assignment below.
+ */
+ table_sort.free_sort_buffer();
+ }
+ }
table_sort.alloc_sort_buffer(param.max_keys_per_buffer, param.rec_length);
if (table_sort.get_sort_keys())
break;
@@ -444,7 +458,10 @@ ha_rows filesort(THD *thd, TABLE *table,
#ifdef SKIP_DBUG_IN_FILESORT
DBUG_POP(); /* Ok to DBUG */
#endif
+
+ // Assign the copy back!
table->sort= table_sort;
+
DBUG_PRINT("exit",
("num_rows: %ld examined_rows: %ld found_rows: %ld",
(long) num_rows, (long) *examined_rows, (long) *found_rows));
=== modified file 'sql/filesort_utils.h'
--- a/sql/filesort_utils.h 2011-11-07 15:32:36 +0000
+++ b/sql/filesort_utils.h 2012-01-18 13:03:10 +0000
@@ -20,6 +20,8 @@
#include "my_base.h"
#include "sql_array.h"
+#include <utility>
+
/*
Calculate cost of merge sort
@@ -86,6 +88,10 @@ public:
/// Allocates the buffer, but does *not* initialize pointers.
uchar **alloc_sort_buffer(uint num_records, uint record_length);
+ /// What is the <num_records, record_length> for the buffer?
+ std::pair<uint, uint> sort_buffer_properties()
+ { return std::make_pair(m_idx_array.size(), m_record_length); }
+
/// Frees the buffer.
void free_sort_buffer();
=== modified file 'sql/table.h'
--- a/sql/table.h 2012-01-13 09:33:13 +0000
+++ b/sql/table.h 2012-01-18 13:03:10 +0000
@@ -337,6 +337,9 @@ public:
uchar **alloc_sort_buffer(uint num_records, uint record_length)
{ return filesort_buffer.alloc_sort_buffer(num_records, record_length); }
+ std::pair<uint, uint> sort_buffer_properties()
+ { return filesort_buffer.sort_buffer_properties(); }
+
void free_sort_buffer()
{ filesort_buffer.free_sort_buffer(); }
=== modified file 'unittest/gunit/filesort_buffer-t.cc'
--- a/unittest/gunit/filesort_buffer-t.cc 2011-12-20 09:51:05 +0000
+++ b/unittest/gunit/filesort_buffer-t.cc 2012-01-18 13:03:10 +0000
@@ -16,11 +16,11 @@
// First include (the generated) my_config.h, to get correct platform defines.
#include "my_config.h"
#include <gtest/gtest.h>
+#include <utility>
#include "filesort_utils.h"
#include "table.h"
-
namespace {
class FileSortBufferTest : public ::testing::Test
@@ -38,7 +38,15 @@ protected:
TEST_F(FileSortBufferTest, FileSortBuffer)
{
const char letters[10]= "abcdefghi";
+ std::pair<uint, uint> buffer_properties= fs_info.sort_buffer_properties();
+ EXPECT_EQ(0U, buffer_properties.first);
+ EXPECT_EQ(0U, buffer_properties.second);
+
uchar **sort_keys= fs_info.alloc_sort_buffer(10, sizeof(char));
+ buffer_properties= fs_info.sort_buffer_properties();
+ EXPECT_EQ(10U, buffer_properties.first);
+ EXPECT_EQ(sizeof(char), buffer_properties.second);
+
uchar **null_sort_keys= NULL;
EXPECT_NE(null_sort_keys, sort_keys);
for (uint ix= 0; ix < 10; ++ix)
No bundle (reason: useless for push emails).
| Thread |
|---|
| • bzr push into mysql-trunk branch (tor.didriksen:3745 to 3746) Bug#13580775 | Tor Didriksen | 20 Jan |
