List:Commits« Previous MessageNext Message »
From:Guilhem Bichot Date:April 27 2011 1:56pm
Subject:bzr commit into mysql-trunk branch (guilhem.bichot:3350) WL#5872
View as plain text  
#At file:///home/mysql_src/bzrrepos_new/wl5872/ based on revid:sven.sandberg@stripped

 3350 Guilhem Bichot	2011-04-27
      WL#5872 "avoid using global heap memory to remember autoincrement values for statement-based binlog".
      As long as WL3404 "new binlog event Insert_id_list_event to store multiple auto_increment intervals"
      is not implemented, Discrete_intervals_list always contains at most one interval,
      so it makes sense to store the interval as a member of Discrete_intervals_list,
      rather than allocating it on the global heap with new(). This is to save one new()
      per statement inserting into an auto-increment column, and is a sub-part of
      WL5774 "Decrease number of malloc's for normal DML queries".
     @ sql/structs.h
        1) ** Edits to the full class **
        The simplified class has to provide all the interface of the full class.
        So, shorten the public interface of the full class, to shorten the work
        needed to code the simplified class.
        1.1) remove get_head()/current()/tail(), because those were used only
        in private functions where head/current/tail are accessible anyway;
        head/current/tail are implementation details, users of the class
        don't need to see them.
        1.2) make copy_shallow() and append(Discrete_interval*) private,
        because they are used only in private functions (same arguments as
        above).
        
        2) ** implementation of the simplified class **
        Same public interface as full class, but simpler code, and no new().
        This class is a "POD", so it can use the default copy constructor,
        and swap() is simple to write.

    modified:
      mysql-test/extra/rpl_tests/rpl_auto_increment.test
      mysql-test/suite/rpl/r/rpl_auto_increment.result
      sql/structs.h
=== modified file 'mysql-test/extra/rpl_tests/rpl_auto_increment.test'
--- a/mysql-test/extra/rpl_tests/rpl_auto_increment.test	2010-12-21 10:39:20 +0000
+++ b/mysql-test/extra/rpl_tests/rpl_auto_increment.test	2011-04-27 13:56:11 +0000
@@ -256,4 +256,17 @@ connection master;
 DROP TABLE t1;
 sync_slave_with_master;
 
+#
+# WL#5872 "avoid using global heap memory to remember autoincrement
+# values for statement-based binlog".
+#
+connection master;
+eval create table t1(a int auto_increment primary key) engine=$engine_type;
+insert into t1 values (null),(null),(1025),(null);
+sync_slave_with_master;
+select * from t1;
+connection master;
+drop table t1;
+sync_slave_with_master;
+
 --source include/rpl_end.inc

=== modified file 'mysql-test/suite/rpl/r/rpl_auto_increment.result'
--- a/mysql-test/suite/rpl/r/rpl_auto_increment.result	2010-12-21 04:47:22 +0000
+++ b/mysql-test/suite/rpl/r/rpl_auto_increment.result	2011-04-27 13:56:11 +0000
@@ -320,4 +320,13 @@ SELECT * FROM t1;
 id	data
 0	2
 DROP TABLE t1;
+create table t1(a int auto_increment primary key) engine=innodb;
+insert into t1 values (null),(null),(1025),(null);
+select * from t1;
+a
+1
+2
+1025
+1026
+drop table t1;
 include/rpl_end.inc

=== modified file 'sql/structs.h'
--- a/sql/structs.h	2010-10-21 09:49:16 +0000
+++ b/sql/structs.h	2011-04-27 13:56:11 +0000
@@ -292,7 +292,119 @@ public:
   };
 };
 
-/* List of Discrete_interval objects */
+/**
+  As long as statement-based binary logging does not support multiple
+  autoincrement intervals, we can use the simplified version of
+  Discrete_intervals_list which supports only 0 or 1 elements.
+*/
+#define DISCRETE_INTERVAL_LIST_HAS_MAX_ONE_ELEMENT 1
+
+#ifdef DISCRETE_INTERVAL_LIST_HAS_MAX_ONE_ELEMENT
+
+/**
+  This is a simplified version of Discrete_intervals_list: it's a degenerate
+  list which supports only 0 or 1 element. Compared to the full version (which
+  is after #else), it avoids one allocation on the global heap. Any fix done
+  here must also be done in the full version.
+  Implemented as part of WL#.
+*/
+class Discrete_intervals_list {
+private:
+  uint elements;                             ///< number of elements (0 or 1)
+  /// Storage for our only element. Meaningless if elements == 0.
+  Discrete_interval single_interval;
+  /**
+    Says whether the next get_next() call should return our only element. This
+    is false if:
+    - either the list is empty
+    - or the list has one element but we already returned it in a previous
+    get_next().
+  */
+  bool current_is_positioned;
+
+public:
+  Discrete_intervals_list()  { empty(); }
+  /// Empties the list
+  void empty()
+  {
+    elements= 0;
+    current_is_positioned= false;
+  }
+  void swap (Discrete_intervals_list *dli)
+  {
+    Discrete_intervals_list tmp(*dli);
+    *dli= *this;
+    *this= tmp;
+  }
+  /**
+    Gets next interval from list.
+    @returns next interval, or NULL if no next interval.
+  */
+  const Discrete_interval* get_next()
+  {
+    if (current_is_positioned)
+    {
+      /*
+        Positioned on single interval. We return it, and after this there is
+        no 'next' so we won't be positioned.
+      */
+      current_is_positioned= false;
+      return &single_interval;
+    }
+    else
+      return NULL;
+  }
+  /**
+    Appends an interval to the list.
+    @param start  start of interval
+    @val   how    many values it contains
+    @param incr   what increment between each value
+    @retval true  error
+    @retval false success
+  */
+  bool append(ulonglong start, ulonglong val, ulonglong incr)
+  {
+    if (elements == 1)
+    {
+      if (unlikely(single_interval.merge_if_contiguous(start, val, incr)))
+      {
+        /*
+          The merge was impossible. But we cannot create yet another interval
+          as we already contain one. This situation can happen. Assume
+          innodb_autoinc_lock_mode>=1 and
+          CREATE TABLE T(A INT AUTO_INCREMENT PRIMARY KEY) ENGINE=INNODB;
+          INSERT INTO T VALUES (NULL),(NULL),(1025),(NULL);
+          Then InnoDB will reserve [1,4] (because of 4 rows) then
+          [1026,1026]. Only the first interval is important for
+          statement-based binary logging as it tells the starting point. So we
+          ignore the second interval:
+        */
+        return false;
+      }
+    }
+    else
+    {
+      DBUG_ASSERT(elements == 0);
+      // need to add a new (first) interval:
+      elements= 1;
+      single_interval.replace(start, val, incr);
+      // If get_next() is called it should return new interval:
+      current_is_positioned= true;
+    }
+    return false;
+  }
+  ulonglong minimum()     const { return (elements ? single_interval.minimum() : 0); };
+  ulonglong maximum()     const { return (elements ? single_interval.maximum() : 0); };
+  uint      nb_elements() const { return elements; }
+};
+
+#else
+
+/**
+  List of Discrete_interval objects. This is the full version which supports
+  any number of intervals.
+  Implemented as part of WL#3146.
+*/
 class Discrete_intervals_list {
 private:
   Discrete_interval        *head;
@@ -316,12 +428,34 @@ private:
   void operator=(Discrete_intervals_list &);  /* prevent use of these */
   Discrete_intervals_list(const Discrete_intervals_list &);
 
-public:
-  Discrete_intervals_list() : head(NULL), current(NULL), elements(0) {};
   void empty_no_free()
   {
     set_members(NULL, NULL, NULL, 0);
   }
+  void copy_shallow(const Discrete_intervals_list * dli)
+  {
+    head= dli->head;
+    tail= dli->tail;
+    current= dli->current;
+    elements= dli->elements;
+  }
+  bool append(Discrete_interval *new_interval)
+  {
+    DBUG_ENTER("Discrete_intervals_list::append");
+    if (unlikely(new_interval == NULL))
+      DBUG_RETURN(1);
+    DBUG_PRINT("info",("adding new auto_increment interval"));
+    if (head == NULL)
+      head= current= new_interval;
+    else
+      tail->next= new_interval;
+    tail= new_interval;
+    elements++;
+    DBUG_RETURN(0);
+  }
+
+public:
+  Discrete_intervals_list() : head(NULL), current(NULL), elements(0) {};
   void empty()
   {
     for (Discrete_interval *i= head; i;)
@@ -332,21 +466,14 @@ public:
     }
     empty_no_free();
   }
-  void copy_shallow(const Discrete_intervals_list * dli)
-  {
-    head= dli->get_head();
-    tail= dli->get_tail();
-    current= dli->get_current();
-    elements= dli->nb_elements();
-  }
   void swap (Discrete_intervals_list * dli)
   {
     Discrete_interval *h, *t, *c;
     uint el;
-    h= dli->get_head();
-    t= dli->get_tail();
-    c= dli->get_current();
-    el= dli->nb_elements();
+    h= dli->head;
+    t= dli->tail;
+    c= dli->current;
+    el= dli->elements;
     dli->copy_shallow(this);
     set_members(h, t, c, el);
   }
@@ -370,27 +497,11 @@ public:
     }
     DBUG_RETURN(0);
   }
-  
-  bool append(Discrete_interval *new_interval)
-  {
-    DBUG_ENTER("Discrete_intervals_list::append");
-    if (unlikely(new_interval == NULL))
-      DBUG_RETURN(1);
-    DBUG_PRINT("info",("adding new auto_increment interval"));
-    if (head == NULL)
-      head= current= new_interval;
-    else
-      tail->next= new_interval;
-    tail= new_interval;
-    elements++;
-    DBUG_RETURN(0);
-  }
+
   ulonglong minimum()     const { return (head ? head->minimum() : 0); };
   ulonglong maximum()     const { return (head ? tail->maximum() : 0); };
   uint      nb_elements() const { return elements; }
-  Discrete_interval* get_head() const { return head; };
-  Discrete_interval* get_tail() const { return tail; };
-  Discrete_interval* get_current() const { return current; };
 };
+#endif
 
 #endif /* STRUCTS_INCLUDED */


Attachment: [text/bzr-bundle] bzr/guilhem.bichot@oracle.com-20110427135611-otwgx7zl0atkijko.bundle
Thread
bzr commit into mysql-trunk branch (guilhem.bichot:3350) WL#5872Guilhem Bichot27 Apr
  • Re: bzr commit into mysql-trunk branch (guilhem.bichot:3350) WL#5872Sven Sandberg28 Apr
    • Re: bzr commit into mysql-trunk branch (guilhem.bichot:3350) WL#5872Guilhem Bichot28 Apr
      • Re: bzr commit into mysql-trunk branch (guilhem.bichot:3350) WL#5872Sven Sandberg28 Apr
        • Re: bzr commit into mysql-trunk branch (guilhem.bichot:3350) WL#5872Guilhem Bichot30 Apr
    • Re: bzr commit into mysql-trunk branch (guilhem.bichot:3350) WL#5872Guilhem Bichot2 May
Re: bzr commit into mysql-trunk branch (guilhem.bichot:3350) WL#5872Guilhem Bichot27 Apr
  • Re: bzr commit into mysql-trunk branch (guilhem.bichot:3350) WL#5872Guilhem Bichot27 Apr
Re: bzr commit into mysql-trunk branch (guilhem.bichot:3350) WL#5872Guilhem Bichot27 Apr
Re: bzr commit into mysql-trunk branch (guilhem.bichot:3350) WL#5872Guilhem Bichot27 Apr
Re: bzr commit into mysql-trunk branch (guilhem.bichot:3350) WL#5872Sven Sandberg28 Apr
Re: bzr commit into mysql-trunk branch (guilhem.bichot:3350) WL#5872Sven Sandberg28 Apr