Fixed assert in perfschema/pfs.cc::start_idle_wait_v1 when using performance schema and big packets in debug version.

The bug was that my_real_read() called net_before_header_psi() multiple times for long packets.
Fixed by adding a flag when we are reading a header.
Did also some cleanups to interface of my_net_read() to avoid unnecessary calls if performance schema is not used.

- Added my_net_read_packet() as a replacement for my_net_read(). my_net_read() is still in the client library for old clients.
- Removed THD->m_server_idle (not needed anymore as this is now given as argument to my_net_read_packet()
- Added my_net_read_packet(), which is a new version of my_net_read() with a new parameter if we are doing a read for a new command from the server.
- Added tests for compressed protocol and big packets





include/mysql.h.pp:
  Added my_net_read_packet() as a replacement for my_net_read()
include/mysql_com.h:
  Added my_net_read_packet() as a replacement for my_net_read()
mysql-test/r/mysql_client_test_comp.result:
  New test
mysql-test/t/mysql_client_test-master.opt:
  Added max_allowed_packet to be able to test big packets and packet size overflows.
mysql-test/t/mysql_client_test_comp-master.opt:
  New test
mysql-test/t/mysql_client_test_nonblock-master.opt:
  Added max_allowed_packet to be able to test big packets and packet size overflows.
sql-common/client.c:
  Use my_net_read_packet()
sql/mf_iocache.cc:
  Use my_net_read_packet()
sql/mysqld.cc:
  Removed THD->m_server_idle (not needed anymore as this is now given as argument to my_net_read_packet()
sql/net_serv.cc:
  Added argument to my_real_read() to indicte if we are reading the first block of the next statement and should call performance schema.
  Added 'compatibilty function' my_net_read().
  Added my_net_read_packet(), which is a new version of my_net_read() with a new parameter if we are doing a read for a new command from the server.
sql/sql_class.cc:
  Removed m_server_idle (not needed anymore)
sql/sql_class.h:
  Removed m_server_idle (not needed anymore)
sql/sql_parse.cc:
  Removed m_server_idle (not needed anymore)
tests/mysql_client_test.c:
  Added tests for compressed protocol and big packets

include/mysql.h.pp

@@ -49,9 +49,9 @@ enum enum_field_types { MYSQL_TYPE_DECIMAL, MYSQL_TYPE_TINY,
    MYSQL_TYPE_DATETIME, MYSQL_TYPE_YEAR,
    MYSQL_TYPE_NEWDATE, MYSQL_TYPE_VARCHAR,
    MYSQL_TYPE_BIT,
-   MYSQL_TYPE_TIMESTAMP2,
+                        MYSQL_TYPE_TIMESTAMP2,
-   MYSQL_TYPE_DATETIME2,
+                        MYSQL_TYPE_DATETIME2,
-   MYSQL_TYPE_TIME2,
+                        MYSQL_TYPE_TIME2,
                         MYSQL_TYPE_NEWDECIMAL=246,
    MYSQL_TYPE_ENUM=247,
    MYSQL_TYPE_SET=248,
@@ -94,7 +94,7 @@ my_bool net_write_command(NET *net,unsigned char command,
      const unsigned char *header, size_t head_len,
      const unsigned char *packet, size_t len);
 int net_real_write(NET *net,const unsigned char *packet, size_t len);
-unsigned long my_net_read(NET *net);
+unsigned long my_net_read_packet(NET *net, my_bool read_from_server);
 struct sockaddr;
 int my_connect(my_socket s, const struct sockaddr *name, unsigned int namelen,
         unsigned int timeout);
@@ -553,7 +553,7 @@ int mysql_list_processes_cont(MYSQL_RES **ret, MYSQL *mysql,
 int mysql_options(MYSQL *mysql,enum mysql_option option,
           const void *arg);
 int mysql_options4(MYSQL *mysql,enum mysql_option option,
-                   const void *arg1, const void *arg2);
+                                       const void *arg1, const void *arg2);
 void mysql_free_result(MYSQL_RES *result);
 int mysql_free_result_start(MYSQL_RES *result);
 int mysql_free_result_cont(MYSQL_RES *result, int status);

include/mysql_com.h

@@ -541,7 +541,8 @@ my_bool	net_write_command(NET *net,unsigned char command,
 			  const unsigned char *header, size_t head_len,
 			  const unsigned char *packet, size_t len);
 int	net_real_write(NET *net,const unsigned char *packet, size_t len);
-unsigned long my_net_read(NET *net);
+unsigned long my_net_read_packet(NET *net, my_bool read_from_server);
+#define my_net_read(A) my_net_read_packet((A), 0)
 
 #ifdef MY_GLOBAL_INCLUDED
 void my_net_set_write_timeout(NET *net, uint timeout);

mysql-test/r/mysql_client_test_comp.result

@@ -0,0 +1,4 @@
+SET @old_slow_query_log= @@global.slow_query_log;
+call mtr.add_suppression(" Error reading file './client_test_db/test_frm_bug.frm'");
+ok
+SET @@global.slow_query_log= @old_slow_query_log;

mysql-test/t/mysql_client_test-master.opt

@@ -1,3 +1,4 @@
 --general-log
 --general-log-file=$MYSQLTEST_VARDIR/log/master.log 
 --log-output=FILE,TABLE
+--max-allowed-packet=32000000

mysql-test/t/mysql_client_test_comp-master.opt

@@ -0,0 +1,2 @@
+--loose-enable-performance-schema
+--max-allowed-packet=32000000

mysql-test/t/mysql_client_test_comp.test

@@ -0,0 +1,20 @@
+# run mysql_client_test with performance schema
+
+# No need to run this with embedded server
+-- source include/not_embedded.inc
+
+# need to have the dynamic loading turned on for the client plugin tests
+--source include/have_plugin_auth.inc
+
+SET @old_slow_query_log= @@global.slow_query_log;
+
+call mtr.add_suppression(" Error reading file './client_test_db/test_frm_bug.frm'");
+
+--exec echo "$MYSQL_CLIENT_TEST" > $MYSQLTEST_VARDIR/log/mysql_client_test_comp.out.log 2>&1
+--exec $MYSQL_CLIENT_TEST --getopt-ll-test=25600M >> $MYSQLTEST_VARDIR/log/mysql_client_test_comp.out.log 2>&1
+
+# End of test
+echo ok;
+
+# Restore state changed by mysql_test_run
+SET @@global.slow_query_log= @old_slow_query_log;

mysql-test/t/mysql_client_test_nonblock-master.opt

@@ -1 +1,2 @@
 --general-log --general-log-file=$MYSQLTEST_VARDIR/log/master.log --log-output=FILE,TABLE
+--max-allowed-packet=32000000

sql-common/client.c

@@ -764,8 +764,8 @@ cli_safe_read(MYSQL *mysql)
 
 restart:
   if (net->vio != 0)
-    len=my_net_read(net);
+    len= my_net_read_packet(net, 0);
-
+  
   if (len == packet_error || len == 0)
   {
     DBUG_PRINT("error",("Wrong connection or packet. fd: %s  len: %lu",

sql/mf_iocache.cc

@@ -57,7 +57,7 @@ int _my_b_net_read(register IO_CACHE *info, uchar *Buffer,
 
   if (!info->end_of_file)
     DBUG_RETURN(1);	/* because my_b_get (no _) takes 1 byte at a time */
-  read_length=my_net_read(net);
+  read_length= my_net_read_packet(net, 0);
   if (read_length == packet_error)
   {
     info->error= -1;

sql/mysqld.cc

@@ -1115,65 +1115,60 @@ void net_before_header_psi(struct st_net *net, void *user_data, size_t /* unused
   thd= static_cast<THD*> (user_data);
   DBUG_ASSERT(thd != NULL);
 
-  if (thd->m_server_idle)
+  /*
-  {
+    We only come where when the server is IDLE, waiting for the next command.
-    /*
+    Technically, it is a wait on a socket, which may take a long time,
-      The server is IDLE, waiting for the next command.
+    because the call is blocking.
-      Technically, it is a wait on a socket, which may take a long time,
+    Disable the socket instrumentation, to avoid recording a SOCKET event.
-      because the call is blocking.
+    Instead, start explicitly an IDLE event.
-      Disable the socket instrumentation, to avoid recording a SOCKET event.
+  */
-      Instead, start explicitly an IDLE event.
+  MYSQL_SOCKET_SET_STATE(net->vio->mysql_socket, PSI_SOCKET_STATE_IDLE);
-    */
+  MYSQL_START_IDLE_WAIT(thd->m_idle_psi, &thd->m_idle_state);
-    MYSQL_SOCKET_SET_STATE(net->vio->mysql_socket, PSI_SOCKET_STATE_IDLE);
-    MYSQL_START_IDLE_WAIT(thd->m_idle_psi, &thd->m_idle_state);
-  }
 }
 
-void net_after_header_psi(struct st_net *net, void *user_data, size_t /* unused: count */, my_bool rc)
+void net_after_header_psi(struct st_net *net, void *user_data,
+                          size_t /* unused: count */, my_bool rc)
 {
   THD *thd;
   thd= static_cast<THD*> (user_data);
   DBUG_ASSERT(thd != NULL);
 
-  if (thd->m_server_idle)
+  /*
+    The server just got data for a network packet header,
+    from the network layer.
+    The IDLE event is now complete, since we now have a message to process.
+    We need to:
+    - start a new STATEMENT event
+    - start a new STAGE event, within this statement,
+    - start recording SOCKET WAITS events, within this stage.
+    The proper order is critical to get events numbered correctly,
+    and nested in the proper parent.
+  */
+  MYSQL_END_IDLE_WAIT(thd->m_idle_psi);
+
+  if (! rc)
   {
-    /*
+    thd->m_statement_psi= MYSQL_START_STATEMENT(&thd->m_statement_state,
-      The server just got data for a network packet header,
+                                                stmt_info_new_packet.m_key,
-      from the network layer.
+                                                thd->db, thd->db_length,
-      The IDLE event is now complete, since we now have a message to process.
+                                                thd->charset());
-      We need to:
-      - start a new STATEMENT event
-      - start a new STAGE event, within this statement,
-      - start recording SOCKET WAITS events, within this stage.
-      The proper order is critical to get events numbered correctly,
-      and nested in the proper parent.
-    */
-    MYSQL_END_IDLE_WAIT(thd->m_idle_psi);
 
-    if (! rc)
+    THD_STAGE_INFO(thd, stage_init);
-    {
-      thd->m_statement_psi= MYSQL_START_STATEMENT(&thd->m_statement_state,
-                                                  stmt_info_new_packet.m_key,
-                                                  thd->db, thd->db_length,
-                                                  thd->charset());
-
-      THD_STAGE_INFO(thd, stage_init);
-    }
-
-    /*
-      TODO: consider recording a SOCKET event for the bytes just read,
-      by also passing count here.
-    */
-    MYSQL_SOCKET_SET_STATE(net->vio->mysql_socket, PSI_SOCKET_STATE_ACTIVE);
   }
+
+  /*
+    TODO: consider recording a SOCKET event for the bytes just read,
+    by also passing count here.
+  */
+  MYSQL_SOCKET_SET_STATE(net->vio->mysql_socket, PSI_SOCKET_STATE_ACTIVE);
 }
 
+
 void init_net_server_extension(THD *thd)
 {
   /* Start with a clean state for connection events. */
   thd->m_idle_psi= NULL;
   thd->m_statement_psi= NULL;
-  thd->m_server_idle= false;
   /* Hook up the NET_SERVER callback in the net layer. */
   thd->m_net_server_extension.m_user_data= thd;
   thd->m_net_server_extension.m_before_header= net_before_header_psi;

sql/net_serv.cc

@@ -824,7 +824,8 @@ static my_bool my_net_skip_rest(NET *net, uint32 remain, thr_alarm_t *alarmed,
 */
 
 static ulong
-my_real_read(NET *net, size_t *complen)
+my_real_read(NET *net, size_t *complen,
+             my_bool header __attribute__((unused)))
 {
   uchar *pos;
   size_t length;
@@ -839,14 +840,16 @@ my_real_read(NET *net, size_t *complen)
 		  NET_HEADER_SIZE);
 #ifdef MYSQL_SERVER
   size_t count= remain;
-  struct st_net_server *server_extension;
+  struct st_net_server *server_extension= 0;
-  server_extension= static_cast<st_net_server*> (net->extension);
+
-  if (server_extension != NULL)
+  if (header)
   {
-    void *user_data= server_extension->m_user_data;
+    server_extension= static_cast<st_net_server*> (net->extension);
-    DBUG_ASSERT(server_extension->m_before_header != NULL);
+    if (server_extension != NULL)
-    DBUG_ASSERT(server_extension->m_after_header != NULL);
+    {
-    server_extension->m_before_header(net, user_data, count);
+      void *user_data= server_extension->m_user_data;
+      server_extension->m_before_header(net, user_data, count);
+    }
   }
 #endif
 
@@ -1042,6 +1045,16 @@ end:
 }
 
 
+/* Old interface. See my_net_read_packet() for function description */
+
+#undef my_net_read
+
+ulong my_net_read(NET *net)
+{
+  return my_net_read_packet(net, 0);
+}
+
+
 /**
   Read a packet from the client/server and return it without the internal
   package header.
@@ -1053,13 +1066,17 @@ end:
   If the packet was compressed, its uncompressed and the length of the
   uncompressed packet is returned.
 
+  read_from_server is set when the server is reading a new command
+  from the client.
+
   @return
   The function returns the length of the found packet or packet_error.
   net->read_pos points to the read data.
 */
 
+
 ulong
-my_net_read(NET *net)
+my_net_read_packet(NET *net, my_bool read_from_server)
 {
   size_t len, complen;
 
@@ -1069,7 +1086,7 @@ my_net_read(NET *net)
   if (!net->compress)
   {
 #endif
-    len = my_real_read(net,&complen);
+    len = my_real_read(net,&complen, read_from_server);
     if (len == MAX_PACKET_LENGTH)
     {
       /* First packet of a multi-packet.  Concatenate the packets */
@@ -1079,7 +1096,7 @@ my_net_read(NET *net)
       {
 	net->where_b += len;
 	total_length += len;
-	len = my_real_read(net,&complen);
+	len = my_real_read(net,&complen, 0);
       } while (len == MAX_PACKET_LENGTH);
       if (len != packet_error)
 	len+= total_length;
@@ -1171,11 +1188,13 @@ my_net_read(NET *net)
       }
 
       net->where_b=buf_length;
-      if ((packet_len = my_real_read(net,&complen)) == packet_error)
+      if ((packet_len = my_real_read(net,&complen, read_from_server))
+          == packet_error)
       {
         MYSQL_NET_READ_DONE(1, 0);
 	return packet_error;
       }
+      read_from_server= 0;
       if (my_uncompress(net->buff + net->where_b, packet_len,
 			&complen))
       {

sql/sql_class.cc

@@ -876,7 +876,6 @@ THD::THD()
    accessed_rows_and_keys(0),
    m_statement_psi(NULL),
    m_idle_psi(NULL),
-   m_server_idle(false),
    thread_id(0),
    global_disable_checkpoint(0),
    failed_com_change_user(0),

sql/sql_class.h

@@ -2501,8 +2501,6 @@ public:
   /** Idle instrumentation state. */
   PSI_idle_locker_state m_idle_state;
 #endif /* HAVE_PSI_IDLE_INTERFACE */
-  /** True if the server code is IDLE for this connection. */
-  bool m_server_idle;
 
   /*
     Id of current query. Statement can be reused to execute several queries

sql/sql_parse.cc

@@ -934,9 +934,7 @@ bool do_command(THD *thd)
   */
   DEBUG_SYNC(thd, "before_do_command_net_read");
 
-  thd->m_server_idle= TRUE;
+  packet_length= my_net_read_packet(net, 1);
-  packet_length= my_net_read(net);
-  thd->m_server_idle= FALSE;
 
   if (packet_length == packet_error)
   {

tests/mysql_client_test.c

@@ -18616,7 +18616,7 @@ static void test_bug56976()
   const char*   query = "SELECT LENGTH(?)";
   char *long_buffer;
   unsigned long i, packet_len = 256 * 1024L;
-  unsigned long dos_len    = 2 * 1024 * 1024L;
+  unsigned long dos_len    = 35000000;
 
   DBUG_ENTER("test_bug56976");
   myheader("test_bug56976");
@@ -19255,6 +19255,106 @@ static void test_mdev4326()
   myquery(rc);
 }
 
+
+/*
+  Check compressed protocol
+*/
+
+static void test_compressed_protocol()
+{
+  MYSQL *mysql_local;
+  char query[4096], *end;
+  int i;
+  myheader("test_compressed_protocol");
+
+  if (!(mysql_local= mysql_client_init(NULL)))
+  {
+    fprintf(stderr, "\n mysql_client_init() failed");
+    exit(1);
+  }
+
+  if (!(mysql_real_connect(mysql_local, opt_host, opt_user,
+                           opt_password, current_db, opt_port,
+                           opt_unix_socket, CLIENT_COMPRESS)))
+  {
+    fprintf(stderr, "\n connection failed(%s)", mysql_error(mysql_local));
+    exit(1);
+  }
+  mysql_options(mysql_local,MYSQL_OPT_COMPRESS,NullS);
+
+  end= strmov(strfill(strmov(query, "select length(\""),1000,'a'),"\")");
+
+  for (i=0 ; i < 2 ; i++)
+  {
+    MYSQL_RES *res;
+
+    int rc= mysql_real_query(mysql, query, (int) (end-query));
+    myquery(rc);
+    res= mysql_store_result(mysql);
+    DBUG_ASSERT(res != 0);
+    mysql_free_result(res);
+  }
+
+  mysql_close(mysql_local);
+}
+
+/*
+  Check big packets
+*/
+
+static void test_big_packet()
+{
+  MYSQL *mysql_local;
+  char *query, *end;
+  /* We run the tests with a server with max packet size of 3200000 */
+  size_t big_packet= 31000000L;
+  int i;
+  MYSQL_PARAMETERS *mysql_params= mysql_get_parameters();
+  long org_max_allowed_packet= *mysql_params->p_max_allowed_packet;
+  long opt_net_buffer_length= *mysql_params->p_net_buffer_length;
+
+  myheader("test_big_packet");
+
+  query= (char*) my_malloc(big_packet+1024, MYF(MY_WME));
+  DIE_UNLESS(query);
+  
+  if (!(mysql_local= mysql_client_init(NULL)))
+  {
+    fprintf(stderr, "\n mysql_client_init() failed");
+    exit(1);
+  }
+
+  if (!(mysql_real_connect(mysql_local, opt_host, opt_user,
+                           opt_password, current_db, opt_port,
+                           opt_unix_socket, 0)))
+  {
+    fprintf(stderr, "\n connection failed(%s)", mysql_error(mysql_local));
+    exit(1);
+  }
+
+  *mysql_params->p_max_allowed_packet= big_packet+1000;
+  *mysql_params->p_net_buffer_length=  8L*256L*256L;
+
+  end= strmov(strfill(strmov(query, "select length(\""), big_packet,'a'),"\")");
+
+  for (i=0 ; i < 2 ; i++)
+  {
+    MYSQL_RES *res;
+    int rc= mysql_real_query(mysql, query, (int) (end-query));
+    myquery(rc);
+    res= mysql_store_result(mysql);
+    DBUG_ASSERT(res != 0);
+    mysql_free_result(res);
+  }
+
+  mysql_close(mysql_local);
+  my_free(query);
+  
+  *mysql_params->p_max_allowed_packet= org_max_allowed_packet;
+  *mysql_params->p_net_buffer_length = opt_net_buffer_length;
+}
+
+
 static struct my_tests_st my_tests[]= {
   { "disable_query_logs", disable_query_logs },
   { "test_view_sp_list_fields", test_view_sp_list_fields },
@@ -19526,6 +19626,8 @@ static struct my_tests_st my_tests[]= {
   { "test_bug13001491", test_bug13001491 },
   { "test_mdev4326", test_mdev4326 },
   { "test_ps_sp_out_params", test_ps_sp_out_params },
+  { "test_compressed_protocol", test_compressed_protocol },
+  { "test_big_packet", test_big_packet },
   { 0, 0 }
 };