← Back to team overview

maria-developers team mailing list archive

  1. maria-developers team
  2. Mailing list archive
  3. Message #08273

Re: [Commits] 83659a6: MDEV-7006 MDEV-7007: SET STATEMENT and slow log

  Thread Previous • Date Previous • • Thread Next 
On 25.02.15 23:15, Sergei Golubchik wrote:

Hi, Sanja!

On Feb 17, sanja@xxxxxxxxxxx wrote:

revision-id: 83659a68642da2eb6e9327f17a0ef730f512aa96
parent(s): 3e2849d2a01b06a61407b00989c3f981e62dd183
committer: Oleksandr Byelkin
branch nick: server
timestamp: 2015-02-17 12:54:51 +0100
message:

MDEV-7006 MDEV-7007: SET STATEMENT and slow log
fixed embedded server tests
MDEV-7009: SET STATEMENT min_examined_row_limit has no effect
MDEV-6948:SET STATEMENT gtid_domain_id = ... FOR has no effect (same for gtid_seq_no and server_id)

old values of SET STATENENT variables now saved in its own Query_arena and restored later

Looks good to me.
I didn't check, though, whether you've put every restore correctly.

See minor comments below:


diff --git a/mysql-test/t/set_statement_notembedded_binlog.test b/mysql-test/t/set_statement_notembedded_binlog.test
new file mode 100644
index 0000000..c9c2334
--- /dev/null
+++ b/mysql-test/t/set_statement_notembedded_binlog.test
@@ -0,0 +1,22 @@
+--source include/have_log_bin.inc
+--source include/not_embedded.inc
+
+--disable_warnings
+drop table if exists t1;
+drop view if exists t1;
+--enable_warnings

This was useful in our early MySQL days, but it's not anymore.
I've stopped adding these drop...if exists at the beginning of test
files a couple of years ago. mtr has a after-test check to ensure
that no test leaves its tables behind. So one can rely on every test
starting clean.

OK

+
+--echo #
+--echo # MDEV-6948: SET STATEMENT gtid_domain_id = ... FOR has no effect
+--echo # (same for gtid_seq_no and server_id)
+--echo #
+reset master;
+create table t1 (i int);
+set gtid_domain_id = 10;
+insert into t1 values (1),(2);
+set statement gtid_domain_id = 20 for insert into t1 values (3),(4);
+
+--replace_column 1 x 2 x 3 x 4 x 5 x
+show binlog events limit 5,5;
+
+drop table t1;
diff --git a/sql/sql_lex.cc b/sql/sql_lex.cc
index 0c1c3b3..ee956ed 100644
--- a/sql/sql_lex.cc
+++ b/sql/sql_lex.cc
@@ -4223,6 +4224,56 @@ int LEX::print_explain(select_result_sink *output, uint8 explain_flags,
    return res;
  }
+void LEX::set_arena_for_set_stmt(Query_arena *backup) 
+{
+  DBUG_ENTER("LEX::set_arena_for_set_stmt");
+  DBUG_ASSERT(arena_for_set_stmt== 0);
+  if (!mem_root_for_set_stmt)
+  {
+    mem_root_for_set_stmt= new MEM_ROOT();
+    if (!(mem_root_for_set_stmt))
+      DBUG_VOID_RETURN;

What happens in OOM case? You return early, but does the caller know
that? Or it'll continue, thinking that everything's ok?
Yes, it will continue till first thd->error then abort. but I put aditional check to speedup the process. 




+    init_sql_alloc(mem_root_for_set_stmt, ALLOC_ROOT_SET, ALLOC_ROOT_SET,
+                   MYF(MY_THREAD_SPECIFIC));
+  }
+  if (!(arena_for_set_stmt= new Query_arena(mem_root_for_set_stmt,

Why are you allocating memory for Query_arena, instead of creating it
on mem_root_for_set_stmt?
Arena is needed because we create Items and so they should go to the item free list of the arena. 




+                                            Query_arena::STMT_INITIALIZED)))
+    DBUG_VOID_RETURN;
+  DBUG_PRINT("info", ("mem_root: 0x%lx  arena: 0x%lx",
+                      (ulong) mem_root_for_set_stmt,
+                      (ulong) arena_for_set_stmt));
+  thd->set_n_backup_active_arena(arena_for_set_stmt, backup);
+  DBUG_VOID_RETURN;
+}
+
+
+void LEX::reset_arena_for_set_stmt(Query_arena *backup)
+{
+  DBUG_ENTER("LEX::reset_arena_for_set_stmt");
+  DBUG_ASSERT(arena_for_set_stmt);
+  thd->restore_active_arena(arena_for_set_stmt, backup);
+  DBUG_PRINT("info", ("mem_root: 0x%lx  arena: 0x%lx",
+                      (ulong) arena_for_set_stmt->mem_root,
+                      (ulong) arena_for_set_stmt));
+  DBUG_VOID_RETURN;
+}
+
+
+void LEX::free_arena_for_set_stmt()
+{
+  DBUG_ENTER("LEX::free_arena_for_set_stmt");
+  if (!arena_for_set_stmt)
+    return;
+  DBUG_PRINT("info", ("mem_root: 0x%lx  arena: 0x%lx",
+                      (ulong) arena_for_set_stmt->mem_root,
+                      (ulong) arena_for_set_stmt));
+  arena_for_set_stmt->free_items();
+  delete(arena_for_set_stmt);
+  free_root(mem_root_for_set_stmt, MYF(MY_KEEP_PREALLOC));
+  arena_for_set_stmt= 0;
+  DBUG_VOID_RETURN;
+}
+
  void LEX::restore_set_statement_var()
  {
    DBUG_ENTER("LEX::restore_set_statement_var");
diff --git a/sql/sql_lex.h b/sql/sql_lex.h
index 46d667c..ed3c4a0 100644
--- a/sql/sql_lex.h
+++ b/sql/sql_lex.h
@@ -2677,10 +2688,22 @@ struct LEX: public Query_tables_list
        limit_rows_examined_cnt= ULONGLONG_MAX;
    }
+ 
+  inline void free_set_stmt_mem_root()
+  {

Please, add

  DBUG_ASSERT(!is_arena_for_set_stmt());

OK



+    if (mem_root_for_set_stmt)
+    {
+      free_root(mem_root_for_set_stmt, MYF(0));
+      delete mem_root_for_set_stmt;
+      mem_root_for_set_stmt= 0;
+    }
+  }
+
    LEX();
virtual ~LEX() 
    {
+    free_set_stmt_mem_root();
      destroy_query_tables_list();
      plugin_unlock_list(NULL, (plugin_ref *)plugins.buffer, plugins.elements);
      delete_dynamic(&plugins);

Regards,
Sergei

References

  Thread Previous • Date Previous • • Thread Next