List:Commits« Previous MessageNext Message »
From:Guilhem Bichot Date:March 1 2011 10:31am
Subject:bzr commit into mysql-trunk branch (guilhem.bichot:3270)
View as plain text  
#At file:///home/mysql_src/bzrrepos_new/mysql-next-mr-opt-backporting-wl4800/ based on revid:guilhem.bichot@stripped

 3270 Guilhem Bichot	2011-03-01
      More fixes for OOM handling and testing: see comment of opt_trace.cc
     @ dbug/dbug.c
        bug: EXCLUDE & SUBDIR is 0. So with this code:
        DBUG_SET("+d,xx"); // adds to list of dbug keywords (cs->stack->keywords), with "INCLUDE" state
        DBUG_SET("-d,xx"); // dels from list, now list empty
        DBUG_SET("-d,xx"); // adds to list, with "EXCLUDE" state
        DBUG_SET("+d,xx"); // modifies list element, gives it both "EXCLUDE" _and_ "INCLUDE" state!!
        The sequence above can happen in repetitions of this pattern:
        1) DBUG_SET("+d,simulate_out_of_memory");
        2) Possible but not guaranteed call to my_realloc() or my_malloc(),
        which if called do DBUG_SET("-d,simulate_out_of_memory");
        3) DBUG_SET("-d,simulate_out_of_memory"); (needed if my_realloc()/malloc
        was not called in (2)
     @ mysql-test/t/optimizer_trace_oom.test
        this test cannot be run because "new" OOM failure cannot be caught.
     @ mysys/my_malloc.c
        Implement "simulate_out_of_memory" in my_realloc(),
        it already exists in my_malloc().
     @ sql/opt_trace.cc
        Move from
          if (Opt_trace_context::simulate_oom_xxx)
            xxx; // same code as when malloc() fails below
          if (malloc(...) == NULL)
            xxx;
        to
          if (Opt_trace_context::simulate_oom_xxx)
            DBUG_SET("+d,simulate_out_of_memory");
          if (malloc(...) == NULL) // actually Sql_array::append or String::append call
            xxx;
        In other words, the first if() just sets a mark to force my_malloc()
        and my_realloc() to fail. This tests more closely what will happen
        in case of OOM, for example it tests the implied my_error() calls
        of my_malloc/my_realloc().
        The changes to the special-char-escaping code are just indentation.
        Notes about "new" OOM-handling problems.
     @ sql/opt_trace.h
        how the API user will be told about OOM
     @ sql/opt_trace2server.cc
        "new" notes
     @ unittest/gunit/opt_trace-t.cc
        Changed tests of OOM according to changes in opt_trace.cc:
        because we now want my_malloc/my_realloc to fail, we need to make
        them be called; for example, we need to use at least 16 cells in the
        Sql_array-s to provoke my_realloc(). This makes the tests more
        complex:
        - OOMinBookKeeping needs many nested objects so reuses
        open_object().
        - OOMinPurge needs more traces (and purge_stmts() needs to let
        the all_stmts_to_del list grow).
        OOMinBookKeeping2 is deleted, was too hard to convert.
        Setting up error_handler_hook to detect OOM.

    removed:
      mysql-test/r/optimizer_trace_oom.result
      mysql-test/t/optimizer_trace_oom.test
    modified:
      dbug/dbug.c
      mysys/my_malloc.c
      sql/opt_trace.cc
      sql/opt_trace.h
      sql/opt_trace2server.cc
      unittest/gunit/opt_trace-t.cc
=== modified file 'dbug/dbug.c'
--- a/dbug/dbug.c	2011-01-11 09:09:21 +0000
+++ b/dbug/dbug.c	2011-03-01 10:31:38 +0000
@@ -1412,7 +1412,7 @@ next:
         }
         else
         {
-          (*cur)->flags&=~(EXCLUDE & SUBDIR);
+          (*cur)->flags&=~(EXCLUDE | SUBDIR);
           (*cur)->flags|=INCLUDE | subdir;
         }
         goto next;

=== removed file 'mysql-test/r/optimizer_trace_oom.result'
--- a/mysql-test/r/optimizer_trace_oom.result	2011-02-28 15:51:31 +0000
+++ b/mysql-test/r/optimizer_trace_oom.result	1970-01-01 00:00:00 +0000
@@ -1,110 +0,0 @@
-set @@session.optimizer_trace="enabled=on";
-select @@debug;
-@@debug
-
-OOM when creating a new "statement trace"
-select 1;
-1
-1
-select * from information_schema.OPTIMIZER_TRACE;
-QUERY	TRACE	MISSING_BYTES_BEYOND_MAX_MEM_SIZE	OS_MALLOC_ERROR
-select 1	{
-  "steps": [
-    {
-      "join_preparation": {
-        "select#": 1,
-        "steps": [
-          {
-            "expanded_query": "/* select#1 */ select 1 AS `1`"
-          }
-        ]
-      }
-    },
-    {
-      "join_optimization": {
-        "select#": 1,
-        "steps": [
-        ]
-      }
-    },
-    {
-      "join_execution": {
-        "select#": 1,
-        "steps": [
-        ]
-      }
-    }
-  ]
-}	0	0
-set session debug="+d,opt_trace_oom1";
-select @@debug;
-ERROR HY001: Out of memory; restart server and try again (needed 192 bytes)
-select 2;
-ERROR HY001: Out of memory; restart server and try again (needed 192 bytes)
-select * from information_schema.OPTIMIZER_TRACE;
-QUERY	TRACE	MISSING_BYTES_BEYOND_MAX_MEM_SIZE	OS_MALLOC_ERROR
-select 1	{
-  "steps": [
-    {
-      "join_preparation": {
-        "select#": 1,
-        "steps": [
-          {
-            "expanded_query": "/* select#1 */ select 1 AS `1`"
-          }
-        ]
-      }
-    },
-    {
-      "join_optimization": {
-        "select#": 1,
-        "steps": [
-        ]
-      }
-    },
-    {
-      "join_execution": {
-        "select#": 1,
-        "steps": [
-        ]
-      }
-    }
-  ]
-}	0	0
-set session debug=default;
-select @@debug;
-@@debug
-
-select 3;
-3
-3
-select * from information_schema.OPTIMIZER_TRACE;
-QUERY	TRACE	MISSING_BYTES_BEYOND_MAX_MEM_SIZE	OS_MALLOC_ERROR
-select 3	{
-  "steps": [
-    {
-      "join_preparation": {
-        "select#": 1,
-        "steps": [
-          {
-            "expanded_query": "/* select#1 */ select 3 AS `3`"
-          }
-        ]
-      }
-    },
-    {
-      "join_optimization": {
-        "select#": 1,
-        "steps": [
-        ]
-      }
-    },
-    {
-      "join_execution": {
-        "select#": 1,
-        "steps": [
-        ]
-      }
-    }
-  ]
-}	0	0

=== removed file 'mysql-test/t/optimizer_trace_oom.test'
--- a/mysql-test/t/optimizer_trace_oom.test	2011-02-28 15:51:31 +0000
+++ b/mysql-test/t/optimizer_trace_oom.test	1970-01-01 00:00:00 +0000
@@ -1,33 +0,0 @@
-# Test how opt trace reacts to out-of-memory, when used in the
-# server.
-# Other OOM tests are in opt_trace-t.
-
---source include/have_optimizer_trace.inc
---source include/have_debug.inc
-
-set @@session.optimizer_trace="enabled=on";
-
-select @@debug;
-
---echo OOM when creating a new "statement trace"
-
-select 1;
-select * from information_schema.OPTIMIZER_TRACE;
-
-set session debug="+d,opt_trace_oom1";
---error ER_OUTOFMEMORY
-select @@debug;
-
-# this shouldn't be traced
---error ER_OUTOFMEMORY
-select 2;
-
-# so we should still see "select 1":
-select * from information_schema.OPTIMIZER_TRACE;
-
-set session debug=default;
-select @@debug;
-
-# this should be traced:
-select 3;
-select * from information_schema.OPTIMIZER_TRACE;

=== modified file 'mysys/my_malloc.c'
--- a/mysys/my_malloc.c	2010-07-08 21:20:08 +0000
+++ b/mysys/my_malloc.c	2011-03-01 10:31:38 +0000
@@ -79,35 +79,35 @@ void *my_realloc(void *oldpoint, size_t 
                    (ulong) size, my_flags));
 
   DBUG_ASSERT(size > 0);
+  DBUG_EXECUTE_IF("simulate_out_of_memory",
+                  point= NULL;
+                  goto end;);
   if (!oldpoint && (my_flags & MY_ALLOW_ZERO_PTR))
     DBUG_RETURN(my_malloc(size, my_flags));
 #ifdef USE_HALLOC
-  if (!(point = malloc(size)))
+  point= malloc(size);
+#else
+  point= realloc(oldpoint, size);
+#endif
+end:
+  if (point == NULL)
   {
     if (my_flags & MY_FREE_ON_ERROR)
       my_free(oldpoint);
     if (my_flags & MY_HOLD_ON_ERROR)
       DBUG_RETURN(oldpoint);
     my_errno=errno;
-    if (my_flags & MY_FAE+MY_WME)
-      my_error(EE_OUTOFMEMORY, MYF(ME_BELL+ME_WAITTANG),size);
+    if (my_flags & (MY_FAE+MY_WME))
+      my_error(EE_OUTOFMEMORY, MYF(ME_BELL+ME_WAITTANG), size);
+    DBUG_EXECUTE_IF("simulate_out_of_memory",
+                    DBUG_SET("-d,simulate_out_of_memory"););
   }
+#ifdef USE_HALLOC
   else
   {
     memcpy(point,oldpoint,size);
     free(oldpoint);
   }
-#else
-  if ((point= realloc(oldpoint, size)) == NULL)
-  {
-    if (my_flags & MY_FREE_ON_ERROR)
-      my_free(oldpoint);
-    if (my_flags & MY_HOLD_ON_ERROR)
-      DBUG_RETURN(oldpoint);
-    my_errno=errno;
-    if (my_flags & (MY_FAE+MY_WME))
-      my_error(EE_OUTOFMEMORY, MYF(ME_BELL+ME_WAITTANG), size);
-  }
 #endif
   DBUG_PRINT("exit",("ptr: %p", point));
   DBUG_RETURN(point);

=== modified file 'sql/opt_trace.cc'
--- a/sql/opt_trace.cc	2011-02-28 15:51:31 +0000
+++ b/sql/opt_trace.cc	2011-03-01 10:31:38 +0000
@@ -593,10 +593,6 @@ bool Opt_trace_stmt::open_struct(const c
         else
           current_struct->add_alnum("...");
       }
-#ifndef DBUG_OFF
-      if (unlikely(Opt_trace_context::simulate_oom_in_open_struct))
-        goto err;
-#endif
       if (unlikely(stack_of_values_of_support_I_S.append(support_I_S)))
         goto err;
       support_I_S= NO_FOR_THIS_AND_CHILDREN;
@@ -622,12 +618,20 @@ bool Opt_trace_stmt::open_struct(const c
   d.current_struct=   current_struct;
   d.has_disabled_I_S= has_disabled_I_S;
   d.current_struct_empty= current_struct_empty;
-#ifndef DBUG_OFF
-  if (unlikely(Opt_trace_context::simulate_oom_in_open_struct))
-    goto err;
-#endif
-  if (unlikely(stack_of_current_structs.append(d)))
-    goto err;
+  {
+    if (Opt_trace_context::simulate_oom_in_open_struct)
+      DBUG_SET("+d,simulate_out_of_memory");
+    const bool rc= stack_of_current_structs.append(d);
+    /*
+      If the append() above didn't trigger reallocation, we need to turn the
+      symbol off by ourselves, or it could make an unrelated allocation
+      fail.
+    */
+    if (Opt_trace_context::simulate_oom_in_open_struct)
+      DBUG_SET("-d,simulate_out_of_memory");
+    if (unlikely(rc))
+      goto err;
+  }
   current_struct= struc;
   current_struct_empty= true;
   return false;
@@ -831,94 +835,87 @@ bool Opt_trace_stmt::Buffer::append_esca
   }
   else
   {
-#ifndef DBUG_OFF // no mostly-useless if() in release binary
-    if (unlikely(Opt_trace_context::simulate_oom_in_buffers))
-      rc= true;
-    else
-#endif
+    rc= false;
+    const char *pstr, *pstr_end;
+    char buf[128];                     // Temporary output buffer.
+    char *pbuf= buf;
+    for (pstr= str, pstr_end= (str + length) ; pstr < pstr_end ; pstr++)
     {
-      rc= false;
-      const char *pstr, *pstr_end;
-      char buf[128];                     // Temporary output buffer.
-      char *pbuf= buf;
-      for (pstr= str, pstr_end= (str + length) ; pstr < pstr_end ; pstr++)
+      char esc;
+      const char c= *pstr;
+      /*
+        JSON syntax says that control characters must be escaped. Experience
+        confirms that this means ASCII 0->31 and " and \ . A few of
+        them are accepted with a short escaping syntax (using \ : like \n)
+        but for most of them, only \uXXXX works, where XXXX is a
+        hexadecimal value for the code point.
+        Rules also mention escaping / , but Python's and Perl's json modules
+        do not require it, and somewhere on Internet someone said JSON
+        allows escaping of / but does not require it.
+      */
+      switch (c)
       {
-        char esc;
-        const char c= *pstr;
-        /*
-          JSON syntax says that control characters must be escaped. Experience
-          confirms that this means ASCII 0->31 and " and \ . A few of
-          them are accepted with a short escaping syntax (using \ : like \n)
-          but for most of them, only \uXXXX works, where XXXX is a
-          hexadecimal value for the code point.
-          Rules also mention escaping / , but Python's and Perl's json modules
-          do not require it, and somewhere on Internet someone said JSON
-          allows escaping of / but does not require it.
-        */
-        switch (c)
-        {
-          // Don't use \u when possible for common chars, \ is easier to read:
-        case '\\':
-          esc= '\\';
-          break;
-        case '"':
-          esc= '\"';
-          break;
-        case '\n':
-          esc= 'n';
-          break;
-        case '\r':
-          esc= 'r';
-          break;
-        case '\t':
-          esc= 't';
-          break;
-        default:
-          esc= 0;
-          break;
-        }
-        if (esc != 0)                           // Escaping with backslash.
+        // Don't use \u when possible for common chars, \ is easier to read:
+      case '\\':
+        esc= '\\';
+        break;
+      case '"':
+        esc= '\"';
+        break;
+      case '\n':
+        esc= 'n';
+        break;
+      case '\r':
+        esc= 'r';
+        break;
+      case '\t':
+        esc= 't';
+        break;
+      default:
+        esc= 0;
+        break;
+      }
+      if (esc != 0)                           // Escaping with backslash.
+      {
+        *pbuf++= '\\';
+        *pbuf++= esc;
+      }
+      else
+      {
+        uint ascii_code= (uint)c;
+        if (ascii_code < 32)                  // Escaping with \u
         {
           *pbuf++= '\\';
-          *pbuf++= esc;
-        }
-        else
-        {
-          uint ascii_code= (uint)c;
-          if (ascii_code < 32)                  // Escaping with \u
+          *pbuf++= 'u';
+          *pbuf++= '0';
+          *pbuf++= '0';
+          if (ascii_code < 16)
           {
-            *pbuf++= '\\';
-            *pbuf++= 'u';
             *pbuf++= '0';
-            *pbuf++= '0';
-            if (ascii_code < 16)
-            {
-              *pbuf++= '0';
-            }
-            else
-            {
-              *pbuf++= '1';
-              ascii_code-= 16;
-            }
-            *pbuf++= _dig_vec_lower[ascii_code];
           }
           else
-            *pbuf++= c; // Normal character, no escaping needed.
-        }
-        /*
-          To fit a next character, we need at most 6 bytes (happens when using
-          \uXXXX syntax) before the buffer's end:
-        */
-        if (pbuf > buf + (sizeof(buf) - 6))
-        {
-          // Possibly no room in 'buf' for next char, so flush buf.
-          rc|= string_buf.append(buf, pbuf - buf);
-          pbuf= buf; // back to buf's start
+          {
+            *pbuf++= '1';
+            ascii_code-= 16;
+          }
+          *pbuf++= _dig_vec_lower[ascii_code];
         }
+        else
+          *pbuf++= c; // Normal character, no escaping needed.
+      }
+      /*
+        To fit a next character, we need at most 6 bytes (happens when using
+        \uXXXX syntax) before the buffer's end:
+      */
+      if (pbuf > buf + (sizeof(buf) - 6))
+      {
+        // Possibly no room in 'buf' for next char, so flush buf.
+        rc|= string_buf.append(buf, pbuf - buf);
+        pbuf= buf; // back to buf's start
       }
-      // Flush any chars left in 'buf'.
-      rc|= string_buf.append(buf, pbuf - buf);
     }
+    // Flush any chars left in 'buf'.
+    rc|= string_buf.append(buf, pbuf - buf);
     malloc_error|= rc;
   }
   return rc;
@@ -935,12 +932,11 @@ bool Opt_trace_stmt::Buffer::append(cons
   }
   else
   {
-#ifndef DBUG_OFF // no mostly-useless if() in release binary
     if (unlikely(Opt_trace_context::simulate_oom_in_buffers))
-      rc= true;
-    else
-#endif
-      rc= string_buf.append(str, length);
+      DBUG_SET("+d,simulate_out_of_memory");
+    rc= string_buf.append(str, length);
+    if (unlikely(Opt_trace_context::simulate_oom_in_buffers))
+      DBUG_SET("-d,simulate_out_of_memory");
     malloc_error|= rc;
   }
   return rc;
@@ -1118,14 +1114,21 @@ bool Opt_trace_context::start(enum enum_
     since_offset_0++;
   }
 
-  Opt_trace_stmt *stmt= new Opt_trace_stmt(this, new_stmt_support_I_S);
-  DBUG_PRINT("opt",("new stmt %p support_I_S %d", stmt,
-                    new_stmt_support_I_S));
-  DBUG_EXECUTE_IF("opt_trace_oom1", {
-      delete stmt;
-      stmt= NULL;
-    });
-
+#ifdef WHEN_WE_HAVE_NEW_NOTHROW
+  /*
+    If we could even overload new(nothrow) so that it supports the DBUG below,
+    we could test it below:
+  */
+  DBUG_EXECUTE_IF("opt_trace_oom1", DBUG_SET("+d,simulate_out_of_memory"););
+  Opt_trace_stmt *stmt= new(nothrow)
+    Opt_trace_stmt(this, new_stmt_support_I_S);
+  DBUG_EXECUTE_IF("opt_trace_oom1",
+                  DBUG_SET("-d,simulate_out_of_memory");
+                  /*
+                    turn this flag off or we'll never be able to issue any
+                    statement!
+                  */
+                  DBUG_SET("-d,opt_trace_oom1"););
   if (unlikely(stmt == NULL))
   {
     /*
@@ -1133,9 +1136,15 @@ bool Opt_trace_context::start(enum enum_
       Opt_trace_stmt::trace_buffer::malloc_error but here there is not even an
       Opt_trace_stmt.
     */
-    my_error(ER_OUTOFMEMORY, MYF(0), sizeof(Opt_trace_stmt));
     goto err;
   }
+#else
+  // OOM-unsafe "new".
+  Opt_trace_stmt *stmt= new Opt_trace_stmt(this, new_stmt_support_I_S);
+#endif
+
+  DBUG_PRINT("opt",("new stmt %p support_I_S %d", stmt,
+                    new_stmt_support_I_S));
 
   if (unlikely(stack_of_current_stmts.append(current_stmt_in_gen)))
   {
@@ -1241,28 +1250,27 @@ void Opt_trace_context::purge_stmts(bool
         Remember to free it (as in @c free()) when possible. For now, make it
         invisible in OPTIMIZER_TRACE table.
       */
-#ifndef DBUG_OFF
-      if (!Opt_trace_context::simulate_oom_in_purge)
-#endif
+      if (Opt_trace_context::simulate_oom_in_purge)
+        DBUG_SET("+d,simulate_out_of_memory");
+      if (likely(!all_stmts_to_del.append(all_stmts_for_I_S.at(idx))))
+        all_stmts_for_I_S.del(idx);
+      else
       {
-        if (likely(!all_stmts_to_del.append(all_stmts_for_I_S.at(idx))))
-          all_stmts_for_I_S.del(idx);
-        else
-        {
-          /*
-            OOM. Cannot purge. Which at worse should only break the
-            offset/limit feature (the trace will accidentally still show up in
-            the OPTIMIZER_TRACE table). append() above has called my_error().
-          */
-        }
+        /*
+          OOM. Cannot purge. Which at worse should only break the
+          offset/limit feature (the trace will accidentally still show up in
+          the OPTIMIZER_TRACE table). append() above has called my_error().
+        */
       }
+      if (Opt_trace_context::simulate_oom_in_purge)
+        DBUG_SET("-d,simulate_out_of_memory");
     }
   }
   /* Examine list of "to be freed" traces and free what can be */
   for (idx= (all_stmts_to_del.elements() - 1) ; idx >= 0 ; idx--)
   {
     Opt_trace_stmt *stmt= all_stmts_to_del.at(idx);
-    if (!stmt->has_ended())
+    if (!stmt->has_ended() || Opt_trace_context::simulate_oom_in_purge)
     {
       /*
         This trace is not finished, freeing it now would lead to use of
@@ -1282,6 +1290,10 @@ void Opt_trace_context::purge_stmts(bool
         So if a trace is not finished, we will wait until it is and
         re-consider it then (which is why this function is called in @c
         Opt_trace_stmt::end() too).
+
+        In unit testing (simulate_oom_in_purge==true) we let the list grow so
+        that it consumes its pre-allocated cells and finally requires a
+        (failing) allocation.
       */
     }
     else

=== modified file 'sql/opt_trace.h'
--- a/sql/opt_trace.h	2011-02-28 15:51:31 +0000
+++ b/sql/opt_trace.h	2011-03-01 10:31:38 +0000
@@ -318,6 +318,27 @@ struct TABLE;
   done in @c JOIN::optimize()). Keep in mind that in an object keys should
   be unique, an application may lose duplicate keys.
 
+  @section OOM_HANDLING Handling of "out-of-memory" errors
+
+  All memory allocations (with exceptions: see below) in the Optimizer trace
+  use @c my_error() to report errors, which itself calls @c
+  error_handler_hook. It is the responsibility of the API user to set up a
+  proper @c error_handler_hook which will alter her/him of the OOM
+  problem. When in the server, this is already the case (@c error_handler_hook
+  is @c my_message_sql() which makes the statement fail).
+  Note that the debug binary may crash if OOM (OOM can cause syntax
+  errors...).
+  @todo @c new error handling. In released and pushbuild2 builds, @c
+  my_new.cc:new is used, which is broken (if allocation fails the constructor
+  is still called, and segfault). In builds with g++, the standard @c new
+  doesn't work either (it throws an exception but as we use -fno-exceptions we
+  cannot catch it and it kills the program).
+  As we don't support exceptions, we need new(nothrow) in order to be able to
+  handle OOM.
+  But "nothrow" is in the standard C++ library, which we don't link with.
+  So we have two calls to "new" (one to create Opt_trace_context, one to
+  create Opt_trace_stmt), which may crash. When we have nothrow we should
+  change them new(nothrow). Grep for WHEN_WE_HAVE_NEW_NOTHROW .
 */
 
 class Opt_trace_struct;

=== modified file 'sql/opt_trace2server.cc'
--- a/sql/opt_trace2server.cc	2011-02-28 15:51:31 +0000
+++ b/sql/opt_trace2server.cc	2011-03-01 10:31:38 +0000
@@ -180,12 +180,12 @@ bool opt_trace_start(THD *thd, const TAB
   */
   if (thd->opt_trace == NULL)
   {
-    // As we use gcc and not g++, this new() behaves as new(std::nothrow).
-    if ((thd->opt_trace= new Opt_trace_context) == NULL)
-    {
-      my_error(ER_OUTOFMEMORY, MYF(0), sizeof(Opt_trace_context));
+#ifdef WHEN_WE_HAVE_NEW_NOTHROW
+    if ((thd->opt_trace= new(nothrow) Opt_trace_context) == NULL)
       DBUG_RETURN(false);
-    }
+#else
+    thd->opt_trace= new Opt_trace_context;      // OOM-unsafe "new".
+#endif
     allocated_here= true;
   }
 

=== modified file 'unittest/gunit/opt_trace-t.cc'
--- a/unittest/gunit/opt_trace-t.cc	2011-02-28 15:51:31 +0000
+++ b/unittest/gunit/opt_trace-t.cc	2011-03-01 10:31:38 +0000
@@ -22,6 +22,7 @@
 #include <gtest/gtest.h>
 
 #include <opt_trace.h>
+#include <mysys_err.h>                          // for testing of OOM
 
 #ifdef OPTIMIZER_TRACE
 
@@ -79,9 +80,25 @@ class TraceContentTest : public ::testin
 {
 public:
   Opt_trace_context trace;
+protected:
+  virtual void SetUp()
+  {
+    error_handler_hook= my_error_handler;
+    oom= false;
+    // Setting debug flags triggers enter/exit trace, so redirect to /dev/null.
+    DBUG_SET("o," IF_WIN("NUL", "/dev/null"));
+  }
+  static bool oom; ///< whether we got an OOM error from opt trace
+  static void my_error_handler(uint error, const char *str, myf MyFlags)
+  {
+    if (error == EE_OUTOFMEMORY)
+      oom= true;
+  }
 };
 
 
+bool TraceContentTest::oom;
+
 TEST_F(TraceContentTest, ConstructAndDestruct)
 {
 }
@@ -622,7 +639,37 @@ TEST(TraceSettingsTest, MaxMemSize2)
 }
 
 
+void open_object(uint count, Opt_trace_context *trace, bool simulate_oom)
+{
+  if (count == 0)
+    return;
+  count--;
+  char key[4];
+  /*
+    Add 100 to always have a key of length 3, this matters to
+    TraceContentTest.Indent.
+    We could use just a fixed string, but it would cause an assertion failure
+    (due to invalid JSON, which itself is conceivable in case of OOM).
+  */
+  llstr(100 + count, key);
+  if (simulate_oom)
+  {
+    if (count == 90)
+      Opt_trace_context::simulate_oom_in_open_struct= true;
+    /*
+      Now we let 80 objects be created, so that one of them surely hits
+      re-allocation and OOM failure.
+    */
+    if (count == 10)
+      Opt_trace_context::simulate_oom_in_open_struct= false;
+  }
+  Opt_trace_object oto(trace, key);
+  open_object(count, trace, simulate_oom);
+}
+
+
 #ifndef DBUG_OFF
+
 /// Test reaction to out-of-memory condition in trace buffer
 TEST_F(TraceContentTest, OOMinBuffer)
 {
@@ -633,7 +680,8 @@ TEST_F(TraceContentTest, OOMinBuffer)
     {
       Opt_trace_array ota(&trace, "one array");
       Opt_trace_context::simulate_oom_in_buffers= true;
-      ota.add(200.4);
+      for (int i= 0; i < 30; i++)
+        ota.add_alnum("_______________________________________________");
       Opt_trace_context::simulate_oom_in_buffers= false;
     }
   }
@@ -642,96 +690,33 @@ TEST_F(TraceContentTest, OOMinBuffer)
   ASSERT_FALSE(it.at_end());
   Opt_trace_info info;
   it.get_value(&info);
-  const char expected[]=
-    "{\n"
-    "  \"one array\": [\n"
-    "\n"                    // 200.4 missing
-    "  ]\n"
-    "}";
-  EXPECT_STREQ(expected, info.trace_ptr);
-  EXPECT_EQ(sizeof(expected) - 1, info.trace_length);
   EXPECT_EQ(0U, info.missing_bytes);
   EXPECT_EQ(true, info.malloc_error);   // malloc error reported
   it.next();
   ASSERT_TRUE(it.at_end());
+  EXPECT_EQ(true, oom);
 }
 
 
 /// Test reaction to out-of-memory condition in book-keeping data structures
-TEST_F(TraceContentTest, OOMinBookKeeping1)
+TEST_F(TraceContentTest, OOMinBookKeeping)
 {
   ASSERT_EQ(false, trace.start(YES_FOR_THIS, false, false, -1, 1, ULONG_MAX,
                                all_features));
   {
     Opt_trace_object oto(&trace);
-    {
-      Opt_trace_array ota(&trace, "one array");
-      Opt_trace_context::simulate_oom_in_open_struct= true;
-      Opt_trace_object oto1(&trace);
-      Opt_trace_context::simulate_oom_in_open_struct= false;
-    }
+    open_object(100, &trace, true);
   }
   trace.end();
   Opt_trace_iterator it(&trace);
   ASSERT_FALSE(it.at_end());
   Opt_trace_info info;
   it.get_value(&info);
-  const char expected[]=
-    "{\n"
-    "  \"one array\": [\n"
-    "    {\n" // there is only '{' for this phantom object
-    "  ]\n"
-    "}";
-  EXPECT_STREQ(expected, info.trace_ptr);
-  EXPECT_EQ(sizeof(expected) - 1, info.trace_length);
-  EXPECT_EQ(0U, info.missing_bytes);
-  EXPECT_EQ(true, info.malloc_error);   // malloc error reported
-  it.next();
-  ASSERT_TRUE(it.at_end());
-}
-
-
-/// Same as OOMinBookKeeping1 but when creating object for disabled feature
-TEST_F(TraceContentTest, OOMinBookKeeping2)
-{
-  ASSERT_EQ(false, trace.start(YES_FOR_THIS, false, false, -1, 1, ULONG_MAX,
-                               Opt_trace_context::MISC));
-  {
-    Opt_trace_object oto(&trace);
-    {
-      Opt_trace_array ota(&trace, "one array");
-      Opt_trace_context::simulate_oom_in_open_struct= true;
-      Opt_trace_object oto1(&trace, Opt_trace_context::GREEDY_SEARCH);
-      Opt_trace_context::simulate_oom_in_open_struct= false;
-      /*
-        Assume that now there is free memory. Because oto1 was changed to be
-        dummy, oto2 will be in the wrong context (as element of 'ota'),
-        causing a syntax error.
-      */
-      Opt_trace_context::dbug_assert_on_syntax_error= false;
-      Opt_trace_object oto2(&trace, "memory_is_back");
-      Opt_trace_context::dbug_assert_on_syntax_error= true;
-    }
-  }
-  trace.end();
-  Opt_trace_iterator it(&trace);
-  ASSERT_FALSE(it.at_end());
-  Opt_trace_info info;
-  it.get_value(&info);
-  const char expected[]=
-    "{\n"
-    "  \"one array\": [\n"
-    "    \"...\"** invalid JSON (unexpected key \"memory_is_back\") ** ,\n"
-    "    {\n" // there is only '{' for this phantom object
-    "    }\n"
-    "  ]\n"
-    "}";
-  EXPECT_STREQ(expected, info.trace_ptr);
-  EXPECT_EQ(sizeof(expected) - 1, info.trace_length);
   EXPECT_EQ(0U, info.missing_bytes);
   EXPECT_EQ(true, info.malloc_error);   // malloc error reported
   it.next();
   ASSERT_TRUE(it.at_end());
+  EXPECT_EQ(true, oom);
 }
 
 
@@ -747,10 +732,32 @@ TEST_F(TraceContentTest, OOMinPurge)
   make_one_trace(&trace, "107", -3, 2);
   make_one_trace(&trace, "108", -3, 2);
   make_one_trace(&trace, "109", -3, 2);
+  make_one_trace(&trace, "110", -3, 2);
+  make_one_trace(&trace, "111", -3, 2);
+  make_one_trace(&trace, "112", -3, 2);
+  make_one_trace(&trace, "113", -3, 2);
+  make_one_trace(&trace, "114", -3, 2);
+  make_one_trace(&trace, "115", -3, 2);
+  make_one_trace(&trace, "116", -3, 2);
+  make_one_trace(&trace, "117", -3, 2);
+  make_one_trace(&trace, "118", -3, 2);
+  make_one_trace(&trace, "119", -3, 2);
+  make_one_trace(&trace, "120", -3, 2);
+  make_one_trace(&trace, "121", -3, 2);
+  make_one_trace(&trace, "122", -3, 2); // purge first fails here
+
   Opt_trace_context::simulate_oom_in_purge= false;
-  // The two first could not be purged so are still shown:
-  const char *expected_traces3[]= {"103", "104", NULL};
+  // 122 could not purge 119, so we should see 119 and 120
+  const char *expected_traces3[]= {"119", "120", NULL};
   check(trace, expected_traces3);
+  EXPECT_EQ(true, oom);
+
+  // Back to normal:
+  oom= false;
+  make_one_trace(&trace, "123", -3, 2); // purge succeeds
+  const char *expected_traces4[]= {"121", "122", NULL};
+  check(trace, expected_traces4);
+  EXPECT_EQ(false, oom);
 }
 
 #endif // !DBUG_OFF
@@ -922,16 +929,6 @@ TEST_F(TraceContentTest, NonUtf8)
 }
 
 
-void open_object(Opt_trace_context *trace)
-{
-  static int count= 100;
-  if (count == 0)
-    return;
-  count--;
-  Opt_trace_object oto(trace, "abc");
-  open_object(trace);
-}
-
 /**
    Test indentation by many blanks.
    By creating a 100-level deep structure, we force an indentation which
@@ -943,7 +940,7 @@ TEST_F(TraceContentTest, Indent)
                                all_features));
   {
     Opt_trace_object oto(&trace);
-    open_object(&trace);
+    open_object(100, &trace, false);
   }
   trace.end();
   Opt_trace_iterator it(&trace);
@@ -956,8 +953,8 @@ TEST_F(TraceContentTest, Indent)
     empty object is noted I(N); so the relationship between the size before
     Nth call and the size after Nth call is:
     S(N+1) = S(N)
-             + I(N)   (indentation before added '"abc": {\n' )
-             + 9      (length of added '"abc": {\n' )
+             + I(N)   (indentation before added '"xxx": {\n' )
+             + 9      (length of added '"xxx": {\n' )
              + I(N)   (indentation before added '}\n' )
              + 2      (length of added '}\n' )
     and the indentation is increased by two as we are one level deeper:


Attachment: [text/bzr-bundle] bzr/guilhem.bichot@oracle.com-20110301103138-05rz8w5rz7jamf1f.bundle
Thread
bzr commit into mysql-trunk branch (guilhem.bichot:3270) Guilhem Bichot1 Mar