PXB-3862 : Combine scattered changed-page reads in page-tracking backups

https://perconadev.atlassian.net/browse/PXB-3862

Problem
-------
A page-tracking incremental reads only the pages the server marked as
changed. Consecutive changed pages are already read together in one
request; the problem is scattered changes. If the changed pages are

    1, 3, 5, 7, 9

xtrabackup issues five single-page reads. Every read request costs a
full I/O round trip, so with scattered changes the copy phase is bound
by the number of requests instead of the amount of data, and an
incremental can take several times longer than a full scan of the
same file.

Fix
---
Read the whole range 1-9 in one request. The unchanged pages 2, 4, 6,
8 are read as filler and discarded by the existing incremental write
filter, which drops every page whose FIL_PAGE_LSN is older than
incremental_lsn. The backup's content and size are byte-for-byte
unchanged; only the read pattern changes.

A gap is worth combining across exactly when its bytes cost less than
one read request. That cost, in bytes of sequential transfer, is
round_trip * bandwidth - and no fixed number fits both a local NVMe
and a network volume, so it is measured at backup start
(probe_storage, xb_io_probe.h): twelve scattered single-page reads
give the round trip, 16MB of sequential reads the bandwidth, on the
largest changed data file (at least 64MB). Then

    read_request_cost = round_trip * bandwidth / 1.5,
                        clamped to [64KB, 1MB], 512KB if unmeasurable
    max_gap           = read_request_cost / physical_page_size

The /1.5 margin absorbs copy-pipeline overhead and measurement noise,
erring toward reading less; it was calibrated on two instrumented
machines whose break-evens bound it from both sides. The
per-tablespace conversion lets compressed tablespaces combine across
the same byte cost. Filler bytes are always the actual gap sizes
present in the data, never the limit, so a generous limit reads
nothing extra.

--page-tracking-max-gap exposes the behaviour: "auto" (default) as
described; a page count pins one value for all tables and skips the
probe; 0 keeps the previous strict-consecutive reads.

Log messages
------------
Once per backup, the measurement:

  pagetracking: calibrated storage (./test/t1.ibd): request round
  trip 124 us, sequential read 984 MB/s -> one read request costs
  ~83KB of sequential transfer; gaps cheaper than this are combined

Per table with at least 1000 changed pages, when its copy finishes,
accumulated from what was actually read:

  pagetracking: test/t1.ibd: 3196 changed pages in 3196 ranges (avg
  gap 2.0 pages); max-gap=4 (auto) combined them into 2 reads:
  request reduction 1598.0x, read amplification 2.98x; issued 16
  read batches

"ranges" is the requests max-gap=0 would issue; "request reduction"
is the benefit and "read amplification" its price (bytes read divided
by changed bytes - read volume only, backup size is unaffected);
"issued" exceeds the group count only when a group is larger than
--read-buffer-size and is read in buffer-sized pieces. When the
typical gap costs more than one read request, an extra line names
both numbers and the pinned value to try, so a boundary case is
diagnosable from the log alone:

  pagetracking: test/t1.ibd: typical gap 8.9 pages (143KB) costs
  more than one read request (83KB); reads stay individual - if
  sequential read throughput is high, --page-tracking-max-gap=9 may
  be faster

Testing: unit tests (xb_page_group-t) cover the read request cost
model across device classes and the storage probe's failure modes; a
framework testcase sweeps change densities and asserts only
hardware-independent invariants - ranges vs combined reads vs issued
requests, auto vs strict, the cost floor and ceiling, and restore
correctness - and was verified to fail against a build with the
combining silently disabled.

Author: satya-bodapati

storage/innobase/xtrabackup/CMakeLists.txt

@@ -44,3 +44,25 @@ ENDIF()
 
 ADD_CUSTOM_TARGET(link_test_dir ALL
   COMMAND ${CMAKE_COMMAND} -E copy_directory "${CMAKE_CURRENT_SOURCE_DIR}/test" "${CMAKE_CURRENT_BINARY_DIR}/test")
+
+# PXB-3862: the server's unittest tree is disabled in XtraBackup builds
+# (top-level gates on "WITH_UNIT_TESTS AND NOT WITH_XTRABACKUP"), so build
+# the page-grouping / storage-probe unit tests here, self-contained
+# against the bundled googletest. The tested code is header-only
+# (xb_page_group.h, xb_io_probe.h), so no server libraries are needed.
+IF(WITH_UNIT_TESTS)
+  FILE(GLOB XB_GTEST_ROOT
+    ${CMAKE_SOURCE_DIR}/extra/googletest/googletest-release-*/googletest)
+  ADD_EXECUTABLE(xb_page_group-t
+    ${CMAKE_SOURCE_DIR}/unittest/gunit/innodb/xb_page_group-t.cc
+    ${XB_GTEST_ROOT}/src/gtest-all.cc
+    ${XB_GTEST_ROOT}/src/gtest_main.cc)
+  TARGET_INCLUDE_DIRECTORIES(xb_page_group-t PRIVATE
+    ${CMAKE_SOURCE_DIR} ${XB_GTEST_ROOT}/include ${XB_GTEST_ROOT})
+  SET_TARGET_PROPERTIES(xb_page_group-t PROPERTIES
+    RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/runtime_output_directory)
+  TARGET_COMPILE_OPTIONS(xb_page_group-t PRIVATE -Wno-undef)
+  FIND_PACKAGE(Threads REQUIRED)
+  TARGET_LINK_LIBRARIES(xb_page_group-t Threads::Threads)
+  ADD_TEST(NAME xb_page_group COMMAND xb_page_group-t)
+ENDIF()

storage/innobase/xtrabackup/src/changed_page_tracking.cc

@@ -268,10 +268,40 @@ bool is_component_installed(MYSQL *connection) {
   return mysql_component == 0 ? (false) : (true);
 }
 
-void range_get_next_page(xb_page_set *page_set) {
+/* Move current_page_it from the first changed page of the current read
+range to its last: the caller then issues one pread covering the whole
+range (sliced into --read-buffer-size pieces when larger). A gap is
+the count of unchanged pages strictly BETWEEN two changed pages, not
+their id difference: between changed pages 1 and 4 the gap is 2
+(pages 2 and 3). Gaps <= max_gap are combined across, so nearby
+changed pages form one larger sequential read instead of one read
+request each; the gap pages become filler - read, then dropped by the
+incremental write filter's LSN check: extra read volume, never backup
+content. A gap > max_gap ends the range; runs of consecutive changed
+pages (gap 0) always stay whole.
+
+  changed pages [1,3,6,9]  (gaps 1,2,2):
+    max_gap=0:  4 reads: [1] [3] [6] [9]
+    max_gap=2:  1 read:  [1-9]   (filler 2,4,5,7,8 read, dropped)
+
+Counting: the loop inspects one neighbouring pair per iteration and
+refuses (breaks) BEFORE the counting code, so reaching that code means
+the pair's gap was just combined: its pages are added to filler_pages
+right there. A refused gap becomes the space between two ranges and is
+counted as skipped pages by the caller (rf_page_tracking_get_next_batch)
+instead - each gap lands in exactly one of the two. Worked trace on the
+shared example in read_filt.h (changed pages 1,2,3,4,7,20,21,
+max_gap=4): (4,7) gap 2 -> filler_pages += 2, combined_gaps = 1;
+(7,20) gap 12 -> park on 7, range [1-7] ends; the caller then counts
+20 - 8 = 12 skipped when the next call builds [20-21].
+
+max_gap comes from --page-tracking-max-gap: by default ("auto") the
+storage's measured read request cost converted to pages of this
+tablespace's physical page size (see xb_io_probe.h). */
+void range_get_next_page(xb_page_set *page_set, ulint max_gap,
+                         ulint *filler_pages, ulint *combined_gaps) {
   ut_ad(page_set->current_page_it != page_set->pages.end());
 
-  /* loop to find the non continuous page id or end of block */
   while (true) {
     auto current_page = *page_set->current_page_it;
     page_set->current_page_it++;
@@ -280,18 +310,19 @@ void range_get_next_page(xb_page_set *page_set) {
       break;
     }
     auto next_page = *page_set->current_page_it;
-    if (next_page != current_page + 1) {
-      /* The next changed page is not adjacent to the current block. Step
-      back so that the iterator points to the last page of the current
-      contiguous block, as documented in the function comment. Leaving the
-      iterator on the next (non-adjacent) changed page makes the caller
-      extend the read batch up to that page, silently reading all the
-      unmodified pages in between. For large tablespaces with sparsely
-      distributed changed pages this degenerates into reading almost the
-      entire data file, defeating the purpose of page tracking. (PXB-3853) */
+    ut_ad(next_page > current_page);
+    if (next_page > current_page + 1 + max_gap) {
+      /* gap too large to combine across: park the iterator on the last
+      page of the current range so the read batch ends there */
       --page_set->current_page_it;
       break;
     }
+    if (next_page > current_page + 1) {
+      /* past the refusal check, so this gap of >= 1 unchanged pages was
+      just combined into the range: its pages are filler */
+      *filler_pages += next_page - current_page - 1;
+      ++*combined_gaps;
+    }
   }
 }
 

storage/innobase/xtrabackup/src/changed_page_tracking.h

@@ -63,9 +63,20 @@ void deinit(xb_space_map *space_map);
 return true if installed */
 bool is_component_installed(MYSQL *connection);
 
-/** Move the current_page_it iterator to poin the last page id in current block
-@param[in/out] page_set       page_set */
-void range_get_next_page(xb_page_set *page_set);
+/** Move the current_page_it iterator to point to the last page id of the
+current block. Changed pages separated by gaps of at most max_gap
+unchanged pages belong to the same block, so that they are read with one
+sequential read.
+@param[in/out] page_set       page_set
+@param[in]     max_gap        largest gap that may be combined across,
+                              in pages of the tablespace's physical
+                              page size
+@param[in/out] filler_pages   incremented by the unchanged pages inside
+                              every gap combined into this block
+@param[in/out] combined_gaps  incremented by the number of gaps combined
+                              into this block */
+void range_get_next_page(xb_page_set *page_set, ulint max_gap,
+                         ulint *filler_pages, ulint *combined_gaps);
 
 /** Set the backupid
 @param[in] connection  MySQL connection handler

storage/innobase/xtrabackup/src/fil_cur.cc

@@ -430,7 +430,7 @@ void xb_fil_cur_close(
     xb_fil_cur_t *cursor) /*!< in/out: source file cursor */
 {
   if (cursor->read_filter) {
-    cursor->read_filter->deinit(&cursor->read_filter_ctxt);
+    cursor->read_filter->deinit(cursor);
   }
 
   ut::free(cursor->scratch);

storage/innobase/xtrabackup/src/read_filt.cc

@@ -23,10 +23,14 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA
 /* Data file read filter implementation */
 
 #include "read_filt.h"
+
+#include <algorithm>
+#include <iomanip>
 #include "common.h"
 #include "dict0dict.h"
 #include "fil_cur.h"
 #include "xb0xb.h"
+#include "xb_io_probe.h"
 #include "xtrabackup.h"
 
 /****************************************************************/ /**
@@ -42,6 +46,14 @@ static void common_init(
   ctxt->buffer_capacity = cursor->buf_size;
   ctxt->page_size = cursor->page_size;
   ctxt->space_id = cursor->space_id;
+  ctxt->max_gap = 0;
+  ctxt->stat_batches = 0;
+  ctxt->stat_total_changed_pages = 0;
+  ctxt->stat_groups = 0;
+  ctxt->stat_combined_gaps = 0;
+  ctxt->stat_filler_pages = 0;
+  ctxt->stat_skipped_pages = 0;
+  ctxt->log_stats = false;
 }
 
 /****************************************************************/ /**
@@ -99,8 +111,8 @@ static void rf_pass_through_get_next_batch(
  Deinitialize the pass-through read filter.  */
 static void rf_pass_through_deinit(
     /*===================*/
-    xb_read_filt_ctxt_t *ctxt __attribute__((unused)))
-/*!<in: read filter context */
+    xb_fil_cur_t *cursor __attribute__((unused)))
+/*!<in: file cursor being closed */
 {}
 
 /** Initialize the page tracking based read filter.  Assumes that
@@ -112,6 +124,37 @@ static void rf_page_tracking_init(xb_read_filt_ctxt_t *ctxt,
                                   const xb_fil_cur_t *cursor, ulint space_id) {
   common_init(ctxt, cursor);
   ctxt->filter_batch_end = 0;
+
+  /* Full-scan spaces never consult max_gap; spaces without changed pages
+  are not read at all. */
+  if (space_id == dict_sys_t::s_dict_space_id ||
+      full_scan_tables.find(space_id) != full_scan_tables.end() ||
+      changed_page_tracking == nullptr ||
+      changed_page_tracking->count(space_id) == 0) {
+    return;
+  }
+
+  /* A pinned --page-tracking-max-gap is expressed in innodb_page_size
+  pages; scale it by the physical page size so compressed tablespaces
+  (zip size 1K-8K) combine across the same byte limit instead of a
+  proportionally smaller one. In auto mode (the default) the limit is
+  the read request cost - measured single-threaded at backup start, or
+  the conservative fallback - converted to pages of this tablespace's
+  physical page size: combine every gap cheaper than one saved read. */
+  ut_ad(UNIV_PAGE_SIZE % ctxt->page_size == 0);
+  ctxt->max_gap = static_cast<ulint>(
+      opt_page_tracking_max_gap_auto ? xb_read_request_cost / ctxt->page_size
+                                     : uint64_t{opt_page_tracking_max_gap} *
+                                           (UNIV_PAGE_SIZE / ctxt->page_size));
+
+  /* grouping statistics accumulate while the file is read and are
+  logged when it closes; below ~16MB of changed pages even fully
+  scattered reads cost a fraction of a second, so such tables only
+  add noise */
+  constexpr ulint LOG_MIN_CHANGED_PAGES = 1000;
+  ctxt->stat_total_changed_pages =
+      changed_page_tracking->at(space_id).pages.size();
+  ctxt->log_stats = (ctxt->stat_total_changed_pages >= LOG_MIN_CHANGED_PAGES);
 }
 
 /** Get the next batch of pages for the page tracking based filter.
@@ -210,9 +253,26 @@ static void rf_page_tracking_get_next_batch(xb_fil_cur_t *cursor,
       verify_skipped_pages();
 #endif
 
+      /* stats: one more read range. The gap between the previous
+      range's end and this one (if any) is exactly a gap the walker
+      refused (that refusal is what ended the previous range), so its
+      pages were seeked past, never read: skipped. Gaps the walker
+      combines lie inside a range and are counted as filler by the
+      walker itself - each gap lands in exactly one of the two. */
+      ctxt->stat_groups++;
+      if (ctxt->filter_batch_end != 0) {
+        ctxt->stat_skipped_pages += next_page_id - ctxt->filter_batch_end;
+      }
+
       ctxt->offset = next_page_id * ctxt->page_size;
       /* Find the end of the current page tracking block */
-      pagetracking::range_get_next_page(space);
+      {
+        ulint filler = 0, combined = 0;
+        pagetracking::range_get_next_page(space, ctxt->max_gap, &filler,
+                                          &combined);
+        ctxt->stat_filler_pages += filler;
+        ctxt->stat_combined_gaps += combined;
+      }
       ut_ad(space->current_page_it != space->pages.end());
 
       ctxt->filter_batch_end = (*space->current_page_it) + 1;
@@ -236,15 +296,72 @@ static void rf_page_tracking_get_next_batch(xb_fil_cur_t *cursor,
     *read_batch_len = ctxt->buffer_capacity;
   }
 
+  if (*read_batch_len > 0) {
+    ctxt->stat_batches++;
+  }
+
   ut_ad(ctxt->offset % ctxt->page_size == 0);
   ut_ad(*read_batch_start % ctxt->page_size == 0);
   ut_ad(*read_batch_len % ctxt->page_size == 0);
 }
 
-/** Deinitialize the page tracking based read filter.
-@param[in] ctxt   read filtr context */
-static void rf_page_tracking_deinit(xb_read_filt_ctxt_t *ctxt
-                                    __attribute__((unused))) {}
+/** Deinitialize the page tracking based read filter: log the grouping
+statistics accumulated while the file was read. Everything reported
+here describes what actually happened - no prediction. */
+static void rf_page_tracking_deinit(xb_fil_cur_t *cursor) {
+  const xb_read_filt_ctxt_t *ctxt = &cursor->read_filter_ctxt;
+  if (!ctxt->log_stats || ctxt->stat_groups == 0) {
+    return;
+  }
+
+  /* ranges = runs of consecutive changed pages: every combined gap
+  joined two of them into one read */
+  const ulint ranges = ctxt->stat_groups + ctxt->stat_combined_gaps;
+  const ulint boundaries = ranges - 1;
+  const double avg_gap = boundaries == 0
+                             ? 0.0
+                             : static_cast<double>(ctxt->stat_filler_pages +
+                                                   ctxt->stat_skipped_pages) /
+                                   static_cast<double>(boundaries);
+  /* the benefit and its price, as parallel ratios: how many times fewer
+  read requests, bought at how many times the changed read volume */
+  const double reduction =
+      static_cast<double>(ranges) / static_cast<double>(ctxt->stat_groups);
+  const double amplification =
+      static_cast<double>(ctxt->stat_total_changed_pages +
+                          ctxt->stat_filler_pages) /
+      static_cast<double>(ctxt->stat_total_changed_pages);
+  xb::info() << std::fixed << "pagetracking: " << cursor->rel_path << ": "
+             << ctxt->stat_total_changed_pages << " changed pages in " << ranges
+             << " ranges (avg gap " << std::setprecision(1) << avg_gap
+             << " pages); max-gap=" << ctxt->max_gap
+             << (opt_page_tracking_max_gap_auto ? " (auto)" : "")
+             << " combined them into " << ctxt->stat_groups
+             << " reads: request reduction " << reduction
+             << "x, read amplification " << std::setprecision(2)
+             << amplification << "x; issued " << ctxt->stat_batches
+             << " read batches";
+
+  /* Make an ineffective auto decision self-explanatory: when the
+  typical gap costs more than one read request, combining achieves
+  little and the reads stay individual. Only when the gap is within
+  reach of a plausible cost: past the clamp ceiling no calibration
+  could ever combine across it - that is genuinely sparse data. */
+  const uint64_t avg_gap_bytes =
+      static_cast<uint64_t>(avg_gap * static_cast<double>(ctxt->page_size));
+  if (opt_page_tracking_max_gap_auto && reduction < 1.5 &&
+      avg_gap_bytes > xb_read_request_cost &&
+      avg_gap_bytes <= pagetracking::READ_REQUEST_COST_MAX_BYTES) {
+    xb::info() << std::fixed << std::setprecision(1)
+               << "pagetracking: " << cursor->rel_path << ": typical gap "
+               << avg_gap << " pages (" << avg_gap_bytes / 1024
+               << "KB) costs more than one read request ("
+               << xb_read_request_cost / 1024
+               << "KB); reads stay individual - if sequential read "
+                  "throughput is high, --page-tracking-max-gap="
+               << static_cast<uint64_t>(avg_gap + 1.0) << " may be faster";
+  }
+}
 
 /* The pass-through read filter */
 xb_read_filt_t rf_pass_through = {&rf_pass_through_init,

storage/innobase/xtrabackup/src/read_filt.h

@@ -39,6 +39,65 @@ struct xb_read_filt_ctxt_t {
   ulint filter_batch_end;             /*!< the ending page id of the
                                       current changed page block in
                                       the page tracking */
+  ulint max_gap;                      /*!< largest changed-page gap to
+                                      combine into one read, in pages of
+                                      page_size (--page-tracking-max-gap
+                                      scaled for compressed tablespaces,
+                                      or the read request cost in
+                                      pages) */
+  /* Statistics accumulated while the file is read, reported by the
+  filter's deinit in one log line; they never influence any decision.
+
+  Shared example used in the field comments below: changed pages
+
+      1,2,3,4   7   20,21          (3 runs of consecutive pages)
+
+  with max_gap = 4. A gap counts the unchanged pages BETWEEN two
+  changed pages (between 4 and 7 it is 2: pages 5,6 - not the id
+  difference 3). The gap of 2 (pages 5,6) is combined across, so
+  [1-7] becomes one read group; the gap of 12 (pages 8..19) is not,
+  so [20-21] starts a second group. Every unchanged page between two
+  changed pages ends up in exactly one of stat_filler_pages (its gap
+  was combined: the page was read) or stat_skipped_pages (its gap was
+  refused: the page was seeked past). */
+  ulint stat_batches;             /*!< pread requests actually issued.
+                                  Example: 2 - one per group; exceeds
+                                  stat_groups only when a group is
+                                  larger than --read-buffer-size and is
+                                  read in buffer-sized sequential
+                                  pieces */
+  ulint stat_total_changed_pages; /*!< pages the server tracked as
+                                  changed for this file: the work that
+                                  must be copied regardless of
+                                  grouping. Example: 7
+                                  (1,2,3,4,7,20,21) */
+  ulint stat_groups;              /*!< read groups formed = distinct
+                                  disk locations read = seeks.
+                                  Example: 2 ([1-7] and [20-21]). The
+                                  log line's "ranges" = stat_groups +
+                                  stat_combined_gaps is what max-gap=0
+                                  would have read: here 3, so combining
+                                  saved one request */
+  ulint stat_combined_gaps;       /*!< gaps of unchanged pages combined
+                                  across (gap <= max_gap), each joining
+                                  two ranges into one read. Example: 1
+                                  (the 2-page gap between 4 and 7) */
+  ulint stat_filler_pages;        /*!< unchanged pages read only as
+                                  filler inside combined gaps, then
+                                  dropped by the incremental write
+                                  filter's LSN check - they cost read
+                                  volume only, never backup size.
+                                  Example: 2 (pages 5,6). read
+                                  amplification = (total_changed +
+                                  filler) / total_changed */
+  ulint stat_skipped_pages;       /*!< unchanged pages in refused gaps
+                                  (gap > max_gap): never read, seeked
+                                  past. Example: 12 (pages 8..19).
+                                  avg gap = (filler + skipped) /
+                                  (ranges - 1) describes how scattered
+                                  the changes are */
+  bool log_stats;             /*!< log grouping and batch stats
+                              for this file */
 };
 
 /* The read filter */
@@ -47,7 +106,9 @@ struct xb_read_filt_t {
                ulint space_id);
   void (*get_next_batch)(xb_fil_cur_t *ctxt, uint64_t *read_batch_start,
                          uint64_t *read_batch_len);
-  void (*deinit)(xb_read_filt_ctxt_t *ctxt);
+  /* called from xb_fil_cur_close with the owning cursor, so the
+  filter can report per-file statistics with full cursor context */
+  void (*deinit)(xb_fil_cur_t *cursor);
   void (*update)(xb_read_filt_ctxt_t *ctxt, uint64_t len,
                  const xb_fil_cur_t *cursor);
 };