From 174e40864f29ef2e879a7fbcefc9dbcb24ce090d Mon Sep 17 00:00:00 2001 From: Satya Bodapati Date: Tue, 14 Apr 2026 14:06:40 +0100 Subject: [PATCH] PXB-3658: Remove redundant fallocate(PUNCH_HOLE) from local_write_sparse JIRA: https://perconadev.atlassian.net/browse/PXB-3658 Problem: When writing page-compressed tables, local_write_sparse() calls fallocate(PUNCH_HOLE) on gaps that were already created as sparse holes via lseek. This is redundant for newly created files -- lseek past unwritten space already produces a sparse hole with no allocated blocks. Each redundant fallocate call costs ~13us of kernel overhead. Benchmark (10GB COMPRESSION='zlib' table, ~1M sparse page entries): - xbstream restore: ~32% faster (1.48x speedup) - Identical file sizes (apparent and allocated) with and without patch Fix: Remove the fallocate(PUNCH_HOLE) block, and the my_tell()/seek variable that only existed to support it, from local_write_sparse in ds_local.cc. local_write_sparse() lacked a posix_fadvise(DONTNEED) call, unlike local_write() which already has one. Without it, page-compressed table restores can accumulate dirty pages in the page cache. Add the same fadvise hint at the end of local_write_sparse() to allow the kernel to evict written pages and reduce memory pressure during large restores. Scenarios where this fix helps: - xtrabackup --backup --target-dir (local backup of compressed tables) - xtrabackup --copy-back (restoring compressed tables from backup) - xbstream -x (extracting compressed tables from stream) Scenarios where this fix does NOT help: - --compress=lz4/zstd backup + --decompress restore: pages are written densely, restore_sparseness() must punch truly-allocated blocks - Incremental delta apply: destination file already exists with allocated blocks, os_file_punch_hole() is needed - Tables without COMPRESSION='zlib': no sparse chunks, so local_write_sparse is never called Test: storage/innobase/xtrabackup/test/suites/compression/pxb_3658_sparse_alloc.sh Thank you Xinyu Zhao for reporting the issue and suggesting a patch. --- storage/innobase/xtrabackup/src/ds_local.cc | 11 +- .../compression/pxb_3658_sparse_alloc.sh | 326 ++++++++++++++++++ 2 files changed, 328 insertions(+), 9 deletions(-) create mode 100755 storage/innobase/xtrabackup/test/suites/compression/pxb_3658_sparse_alloc.sh diff --git a/storage/innobase/xtrabackup/src/ds_local.cc b/storage/innobase/xtrabackup/src/ds_local.cc index eb94c5cd7706..7f1df087b700 100644 --- a/storage/innobase/xtrabackup/src/ds_local.cc +++ b/storage/innobase/xtrabackup/src/ds_local.cc @@ -168,14 +168,12 @@ static int local_write_sparse(ds_file_t *file, const void *buf, size_t len, [[maybe_unused]] bool punch_hole_supported) { auto local_file = ((ds_local_file_t *)file->ptr); File fd = local_file->fd; - [[maybe_unused]] ulonglong seek = 0; const uchar *ptr = static_cast(buf); for (size_t i = 0; i < sparse_map_size; ++i) { my_off_t rc; - seek = my_tell(fd, MYF(MY_WME)); rc = my_seek(fd, sparse_map[i].skip, MY_SEEK_CUR, MYF(MY_WME)); if (rc == MY_FILEPOS_ERROR) { return 1; @@ -186,13 +184,6 @@ static int local_write_sparse(ds_file_t *file, const void *buf, size_t len, return 1; } -#ifdef HAVE_FALLOC_PUNCH_HOLE_AND_KEEP_SIZE - if (punch_hole_supported) { - fallocate(fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE, seek, - sparse_map[i].skip); - } -#endif - ptr += sparse_map[i].len; } /* to track if last page is sparse */ @@ -201,6 +192,8 @@ static int local_write_sparse(ds_file_t *file, const void *buf, size_t len, } else local_file->last_seek = 0; + posix_fadvise(fd, 0, 0, POSIX_FADV_DONTNEED); + return 0; } diff --git a/storage/innobase/xtrabackup/test/suites/compression/pxb_3658_sparse_alloc.sh b/storage/innobase/xtrabackup/test/suites/compression/pxb_3658_sparse_alloc.sh new file mode 100755 index 000000000000..5b1dcfb4a91a --- /dev/null +++ b/storage/innobase/xtrabackup/test/suites/compression/pxb_3658_sparse_alloc.sh @@ -0,0 +1,326 @@ +# +# PXB-3658: Verify that page-compressed tables remain sparse after +# backup/restore, and that allocated disk size (du) exactly matches +# the original server files. +# +# Strategy: restart the server after inserting data to force a clean +# shutdown (all dirty pages flushed, checkpoint complete). This gives +# us a deterministic .ibd state. After backup + prepare (which is a +# no-op for data pages since redo log is empty) + copy-back, the +# restored .ibd must have the exact same allocated size. +# +# Tests five restore paths: +# 1. Local backup (no streaming) +# 2. xbstream extract (no xtrabackup compression) -- SPARSE chunks +# 3. xbstream extract --decompress (with lz4 compression) -- dense + restore_sparseness via xbstream +# 4. Local backup with --compress=zstd + xtrabackup --decompress -- restore_sparseness via xtrabackup +# 5. Streamed backup with --compress=zstd + xbstream extract (no decompress) + xtrabackup --decompress +# + +. inc/common.sh +. inc/keyring_file.sh + +require_lz4 +require_zstd + +get_allocated_size() { + du --block-size=1 "$1" | awk '{print $1}' +} + +get_apparent_size() { + stat --printf "%s" "$1" +} + +check_sparse_exact() { + local filepath=$1 + local label=$2 + local expected_alloc=$3 + local tolerance=${4:-0} + + if ! is_sparse_file "$filepath" ; then + die "$label: $filepath is NOT sparse" + fi + + local apparent=$(get_apparent_size "$filepath") + local allocated=$(get_allocated_size "$filepath") + + vlog "$label: apparent=$apparent allocated=$allocated expected=$expected_alloc tolerance=$tolerance" + + local diff=$(( allocated - expected_alloc )) + # absolute value + if [ "$diff" -lt 0 ] ; then + diff=$(( -diff )) + fi + + if [ "$diff" -gt "$tolerance" ] ; then + die "$label: allocated size mismatch: got $allocated, expected $expected_alloc (diff=$diff, tolerance=$tolerance)" + fi + + vlog "$label: PASS (diff=$diff within tolerance=$tolerance)" +} + +setup_and_get_baseline() { + run_cmd $MYSQL $MYSQL_ARGS test < $topdir/backup.xbs + +rm -rf $topdir/backup && mkdir $topdir/backup +xbstream -x -v -C $topdir/backup < $topdir/backup.xbs + +xtrabackup --prepare --target-dir=$topdir/backup + +check_sparse_exact "$topdir/backup/test/t_zlib.ibd" \ + "xbstream t_zlib (after prepare)" "$orig_zlib_alloc" + +stop_server +rm -rf $mysql_datadir + +xtrabackup --copy-back --target-dir=$topdir/backup + +start_server + +check_sparse_exact "$mysql_datadir/test/t_zlib.ibd" \ + "xbstream t_zlib (after copy-back)" "$orig_zlib_alloc" + +if is_sparse_file "$mysql_datadir/test/t_plain.ibd" ; then + die "t_plain.ibd should NOT be sparse (xbstream path)" +fi + +verify_db_state test + +run_cmd $MYSQL $MYSQL_ARGS test < $topdir/backup.xbs + +rm -rf $topdir/backup && mkdir $topdir/backup +xbstream -x -v -C $topdir/backup --decompress < $topdir/backup.xbs + +xtrabackup --prepare --target-dir=$topdir/backup + +check_sparse_exact "$topdir/backup/test/t_zlib.ibd" \ + "xbstream-decompress t_zlib (after prepare)" "$orig_zlib_alloc" + +stop_server +rm -rf $mysql_datadir + +xtrabackup --copy-back --target-dir=$topdir/backup + +start_server + +check_sparse_exact "$mysql_datadir/test/t_zlib.ibd" \ + "xbstream-decompress t_zlib (after copy-back)" "$orig_zlib_alloc" + +if is_sparse_file "$mysql_datadir/test/t_plain.ibd" ; then + die "t_plain.ibd should NOT be sparse (decompress path)" +fi + +verify_db_state test + +run_cmd $MYSQL $MYSQL_ARGS test < $topdir/backup.xbs + +rm -rf $topdir/backup && mkdir $topdir/backup +xbstream -x -v -C $topdir/backup < $topdir/backup.xbs + +xtrabackup --decompress --target-dir=$topdir/backup + +xtrabackup --prepare --target-dir=$topdir/backup + +check_sparse_exact "$topdir/backup/test/t_zlib.ibd" \ + "stream-zstd-xb-decompress t_zlib (after prepare)" "$orig_zlib_alloc" + +stop_server +rm -rf $mysql_datadir + +xtrabackup --copy-back --target-dir=$topdir/backup + +start_server + +check_sparse_exact "$mysql_datadir/test/t_zlib.ibd" \ + "stream-zstd-xb-decompress t_zlib (after copy-back)" "$orig_zlib_alloc" + +if is_sparse_file "$mysql_datadir/test/t_plain.ibd" ; then + die "t_plain.ibd should NOT be sparse (stream zstd xb-decompress path)" +fi + +verify_db_state test + +run_cmd $MYSQL $MYSQL_ARGS test <