Bug#36319083 [InnoDB] Merge sort buffer can be too small

Symptom: In some circumstances an index cannot be created;
this is configuration-dependent

Root cause: Merge sort file buffer size can in some cases be
calculated to be the size of IO_BLOCK_SIZE. The logic for the
buffer is that subsequent records of data are added to the
buffer, but when adding a row would overflow the buffer, thw
contents are written to disk in multiples of IO_BLOCK_SIZE and
space is freed up. If the buffer is only IO_BLOCK_SIZE, it is
likely that at that point the existing contents' length is
less than IO_BLOCK_SIZE, in which case nothing gets written,
no space can be freed up, and the new record cannot be added.

Fix: Since maximum allowed key length is 3072 and
IO_BLOCK_SIZE is 4096 bytes, and given other factors
contributing to buffer length, like page size, which also
affects allowed key size, 2 * IO_BLOCK_SIZE is sufficient
minimum length to ensure there is always IO_BLOCK_SIZE
bytes in the buffer when the write happens. If this ever
changes, the merge will fail gracefully.

Change-Id: I7aec373cfed9e364751372cce3746eb7ad75b3b9

Author: Andrzej Jarzabek

storage/innobase/ddl/ddl0ctx.cc

@@ -206,7 +206,10 @@ size_t Context::merge_io_buffer_size(size_t n_buffers) const noexcept {
   /* We aim to do IO_BLOCK_SIZE writes all the time. */
   ut_a(!(io_size % IO_BLOCK_SIZE));
 
-  return std::max(std::max((ulong)srv_page_size, (ulong)IO_BLOCK_SIZE),
+  /* The buffer must be at least large enough to fit one IO block plus one row.
+     2 * IO_BLOCK_SIZE meets this criterion given limits on key length -
+     see ha_innobase::max_supported_key_length() */
+  return std::max(std::max((ulong)srv_page_size, (ulong)IO_BLOCK_SIZE * 2LU),
                   (ulong)io_size);
 }
 

storage/innobase/ddl/ddl0merge.cc

@@ -321,20 +321,28 @@ dberr_t Merge_file_sort::Output_file::write(const mrec_t *mrec,
   if (unlikely(m_ptr + rec_size + need >= m_buffer.first + m_buffer.second)) {
     const size_t n_write = m_ptr - m_buffer.first;
     const auto len = ut_uint64_align_down(n_write, IO_BLOCK_SIZE);
-    auto err = ddl::pwrite(m_file.get(), m_buffer.first, len, m_offset);
+    if (len != 0) {
+      auto err = ddl::pwrite(m_file.get(), m_buffer.first, len, m_offset);
 
-    if (err != DB_SUCCESS) {
-      return err;
-    }
+      if (err != DB_SUCCESS) {
+        return err;
+      }
 
-    ut_a(n_write >= len);
-    const auto n_move = n_write - len;
+      ut_a(n_write >= len);
+      const auto n_move = n_write - len;
 
-    m_ptr = m_buffer.first;
-    memmove(m_ptr, m_ptr + len, n_move);
-    m_ptr += n_move;
+      m_ptr = m_buffer.first;
+      memmove(m_ptr, m_ptr + len, n_move);
+      m_ptr += n_move;
 
-    m_offset += len;
+      m_offset += len;
+    }
+
+    if (unlikely(m_ptr + rec_size + need >= m_buffer.first + m_buffer.second)) {
+      // Should be caught earlier
+      ut_d(ut_error);
+      ut_o(return DB_TOO_BIG_RECORD);
+    }
   }
 
   m_last_mrec = m_ptr;