Skip to content

Commit 8f392cb

Browse files
PXB-3747 : Report uncompressed_backup_size for --compress backups
https://perconadev.atlassian.net/browse/PXB-3747 Problem ------- For --compress backups, backup_size is the compressed size (it comes from the leaf). Operators also need the logical, pre-compression size to answer "how much real data did this backup represent?" and to derive the effective compression ratio from a single xtrabackup_info record. Design ------ Why two data pipelines exist xtrabackup maintains two top-level data pipelines on the backup side because not every file benefits from compression: * ds_data -- normal InnoDB tablespaces, redo, metadata. Compressed when --compress is on. * ds_uncompressed_data -- server-encrypted InnoDB tablespaces ("do not compress encrypted tablespaces", xtrabackup.cc) and RocksDB SST files (already compressed internally). Bypasses the compress wrapper because ciphertext and pre-compressed content are incompressible, so the CPU cost buys nothing. Both pipelines share their encrypt / xbstream / leaf tail as an init-time ctxt-sharing optimization: ds_data --> compress --> encrypt --> xbstream --> leaf ^ | ds_uncompressed_data -----------------> encrypt --> xbstream ----+ Why the counter cannot live on any ctxt That shared tail makes "attach a counter to the ctxt" broken no matter which ctxt is picked: * Leaf ctxt: in xbstream mode ds_data, ds_redo, ds_meta, and ds_uncompressed_data all terminate at the same leaf. A leaf-ctxt counter would sum bytes from every pipeline, and for the compressed pipeline those bytes are post-compression -- neither the logical total nor the on-disk total. * Encrypt / xbstream / buffer ctxts: ds_data and ds_uncompressed_data share these too. Counting there mixes post-compression bytes (from ds_data) with raw bytes (from ds_uncompressed_data); the sum is meaningless. The counter must attach where a single logical write unambiguously enters exactly one pipeline -- on the per-file handle returned by the top-level ds_open(). Each top-level open creates a fresh ds_file_t, the caller decides whether that file's bytes count, and ds_write / ds_write_sparse bump the counter with the length supplied at the entry, before any wrapper transforms it: xtrabackup top-level open | ds_tracked_open(..., xb_get_metrics()) | | | +-- nullptr when !--compress (no cost) v ds_file_t { metrics -> xb_backup_metrics } | ds_write(len) / ds_write_sparse(packed_len) | if (file->metrics) metrics->add_uncomp_size(len) v [ compress / encrypt / buffer / xbstream wrappers ] | v leaf (already counted as backup_size by the previous commit) xb_backup_metrics.get_uncomp_size() | v uncompressed_backup_size (xtrabackup_info + error log) Because arming is opt-in and scoped to the top-level opens on ds_data / ds_redo / ds_meta / ds_uncompressed_data, the hot-path cost is one atomic add per write on --compress runs; non-compress runs skip even that. The same xb_backup_metrics aggregates both pipelines, so server-encrypted IBDs and RocksDB SSTs are counted exactly once, at their logical size. Arming is driven by xb_get_metrics(), which returns a real counter only when --compress is active, so non-compress runs emit neither the field nor the log line. Tests ----- t/backup_size_compress.sh covers --compress target-dir, --compress xbstream, --compress + --encrypt, incremental chains, RocksDB, server-encrypted InnoDB, redo-log encryption, and sparse files. Every scenario asserts backup_size matches the compressed on-disk size and, after decompression, uncompressed_backup_size matches the decompressed tree size byte-for-byte.
1 parent 70a001d commit 8f392cb

24 files changed

Lines changed: 712 additions & 189 deletions

storage/innobase/xtrabackup/src/backup_copy.cc

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ Place, Suite 330, Boston, MA 02111-1307 USA
5555
#include <chrono>
5656
#include <fstream>
5757
#include <functional>
58+
#include <iomanip>
5859
#include <queue>
5960
#include <set>
6061
#include <sstream>
@@ -377,7 +378,7 @@ bool backup_file_print(const char *filename, const char *message, int len) {
377378
stat.st_mtime = time(nullptr);
378379
stat.st_size = len;
379380

380-
dstfile = ds_open(ds_data, filename, &stat);
381+
dstfile = ds_tracked_open(ds_data, filename, &stat, xb_get_metrics());
381382
if (dstfile == NULL) {
382383
xb::error() << "cannot open the destination stream for " << filename;
383384
goto error;
@@ -576,7 +577,8 @@ bool copy_file(ds_ctxt_t *datasink, const char *src_file_path,
576577

577578
strncpy(dst_name, cursor.rel_path, sizeof(dst_name));
578579

579-
dstfile = ds_open(datasink, trim_dotslash(dst_file_path), &cursor.statinfo);
580+
dstfile = ds_tracked_open(datasink, trim_dotslash(dst_file_path),
581+
&cursor.statinfo, xb_get_metrics());
580582
if (dstfile == NULL) {
581583
xb::error() << "cannot open the destination stream for " << dst_name;
582584
goto error;
@@ -1594,10 +1596,6 @@ bool backup_start(Backup_context &context) {
15941596
return (true);
15951597
}
15961598

1597-
/** Render a byte count as a human-friendly string (GiB / MiB / KiB /
1598-
bytes) for the backup-complete log line.
1599-
@param[in] bytes raw byte count
1600-
@return newly-constructed std::string, e.g. "12.34 MiB". */
16011599
static std::string human_readable(unsigned long long bytes) {
16021600
char buf[64];
16031601
if (bytes >= 1ULL << 30)
@@ -1671,14 +1669,36 @@ bool backup_finish(Backup_context &context) {
16711669
unsigned long long backup_size = get_final_backup_size();
16721670

16731671
/* A successful backup always produces on-disk output, so the leaf
1674-
counter must be non-zero here. ut_ad is a no-op in release; warn +
1675-
skip on release to avoid hiding a silent reporting failure. */
1672+
counter must be non-zero here. ut_ad is a no-op in release; warn
1673+
+ skip on release to avoid hiding a silent reporting failure. */
16761674
ut_ad(backup_size > 0);
16771675
if (backup_size == 0) {
16781676
xb::warn() << "Backup size reporting failed: leaf counter returned 0";
16791677
} else {
16801678
xb::info() << "Backup size: " << human_readable(backup_size) << " ("
16811679
<< backup_size << " bytes)";
1680+
1681+
if (xtrabackup_compress != XTRABACKUP_COMPRESS_NONE) {
1682+
unsigned long long uncompressed_backup_size =
1683+
get_uncompressed_backup_size();
1684+
1685+
/* With --compress, at least one data file must flow through a
1686+
top-level ds_tracked_open() with metrics armed, so the counter
1687+
cannot be zero. */
1688+
ut_ad(uncompressed_backup_size > 0);
1689+
if (uncompressed_backup_size == 0) {
1690+
xb::warn() << "Uncompressed backup size reporting failed: metrics"
1691+
" counter is 0 despite --compress";
1692+
} else {
1693+
xb::info() << "Uncompressed backup size: "
1694+
<< human_readable(uncompressed_backup_size) << " ("
1695+
<< uncompressed_backup_size << " bytes)";
1696+
1697+
double ratio = (double)uncompressed_backup_size / (double)backup_size;
1698+
xb::info() << "Compression ratio: " << std::fixed
1699+
<< std::setprecision(2) << ratio << "x";
1700+
}
1701+
}
16821702
}
16831703
}
16841704

storage/innobase/xtrabackup/src/backup_mysql.cc

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -627,7 +627,6 @@ bool get_mysql_vars(MYSQL *connection) {
627627
my_strdup(PSI_NOT_INSTRUMENTED, innodb_directories_var, MYF(MY_FAE));
628628
}
629629

630-
631630
if (!check_if_param_set("innodb_log_file_size") && innodb_log_file_size_var) {
632631
char *endptr;
633632

@@ -1849,12 +1848,7 @@ char *get_xtrabackup_info(MYSQL *connection) {
18491848
format_time(history_start_time, buf_start_time, time_buf_size);
18501849
format_time(history_end_time, buf_end_time, time_buf_size);
18511850

1852-
/* Sample the leaf's bytes_written counter now, right before
1853-
xtrabackup_info is itself written to ds_data. Everything that was
1854-
meant to flow through ds_data for this backup is already accounted
1855-
for at this point (see the reorder in xtrabackup.cc's backup finish
1856-
flow). */
1857-
const unsigned long long backup_size = get_final_backup_size();
1851+
unsigned long long backup_size = get_final_backup_size();
18581852

18591853
ut_a(uuid);
18601854
ut_a(server_version);
@@ -1910,10 +1904,26 @@ char *get_xtrabackup_info(MYSQL *connection) {
19101904
xtrabackup_encrypt ? "Y" : "N", /* encrypted */
19111905
ddl_lock_type_to_str(static_cast<lock_ddl_type_t>(opt_lock_ddl))
19121906
.c_str(), /* lock-ddl */
1913-
backup_size);
1907+
backup_size); /* backup_size */
19141908

19151909
ut_a(ret != 0);
19161910

1911+
if (xtrabackup_compress != XTRABACKUP_COMPRESS_NONE) {
1912+
unsigned long long uncompressed_backup_size =
1913+
get_uncompressed_backup_size();
1914+
1915+
char *tmp = NULL;
1916+
int ret2 = asprintf(&tmp, "%suncompressed_backup_size = %llu\n", result,
1917+
uncompressed_backup_size);
1918+
if (ret2 < 0) {
1919+
xb::warn() << "Failed to append uncompressed_backup_size to"
1920+
<< " xtrabackup_info: " << strerror(errno);
1921+
} else {
1922+
free(result);
1923+
result = tmp;
1924+
}
1925+
}
1926+
19171927
free(server_version);
19181928
return result;
19191929
}

storage/innobase/xtrabackup/src/datasink.cc

Lines changed: 78 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,9 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA
2020

2121
#include "datasink.h"
2222
#include <my_base.h>
23+
#include <atomic>
24+
#include <cassert>
25+
#include <cstdint>
2326
#include "common.h"
2427
#include "ds_buffer.h"
2528
#include "ds_compress.h"
@@ -37,8 +40,17 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA
3740
#include "ds_xbstream.h"
3841
#include "msg.h"
3942

40-
/************************************************************************
41-
Create a datasink of the specified type */
43+
/** Global aggregate metrics for the backup pipelines. Incremented by
44+
ds_write() / ds_write_sparse() whenever the file's metrics pointer is
45+
non-null. Armed at top-level backup open sites via ds_tracked_open()
46+
with xb_get_metrics() (xtrabackup.h), which returns &xb_backup_metrics
47+
when --compress is active and nullptr otherwise. */
48+
xb_metrics xb_backup_metrics;
49+
50+
/** See datasink.h for contract.
51+
@param[in] root root path / destination the datasink writes into
52+
@param[in] type which datasink to instantiate
53+
@return a datasink context, or nullptr for unknown type. */
4254
ds_ctxt_t *ds_create(const char *root, ds_type_t type) {
4355
datasink_t *ds;
4456
ds_ctxt_t *ctxt;
@@ -95,13 +107,7 @@ ds_ctxt_t *ds_create(const char *root, ds_type_t type) {
95107
ctxt = ds->init(root);
96108
if (ctxt != NULL) {
97109
ctxt->datasink = ds;
98-
/* Per-datasink init() routines use my_malloc() which does not zero
99-
memory, so fields not explicitly assigned by init() (notably
100-
pipe_ctxt) can hold garbage. Normalize them here so every caller
101-
sees a clean ds_ctxt_t regardless of which leaf/wrapper produced
102-
it. In particular ds_leaf() relies on pipe_ctxt == nullptr at the
103-
terminal node to stop walking. */
104-
ctxt->pipe_ctxt = nullptr;
110+
ctxt->pipe_ctxt = NULL;
105111
} else {
106112
msg("Error: failed to initialize datasink.\n");
107113
exit(EXIT_FAILURE);
@@ -110,61 +116,101 @@ ds_ctxt_t *ds_create(const char *root, ds_type_t type) {
110116
return ctxt;
111117
}
112118

113-
/************************************************************************
114-
Open a datasink file */
119+
/** Pure dispatcher. Each *_open() initializes the framework-owned
120+
fields on its freshly allocated ds_file_t via ds_init_file() (see
121+
datasink.h). The debug assertion catches any *_open implementation
122+
that forgets to call ds_init_file(): ds_write / ds_close would later
123+
chase a NULL datasink/ctxt and crash with no clue about the root
124+
cause.
125+
@param[in] ctxt pipeline to open through
126+
@param[in] path path relative to the pipeline root
127+
@param[in] stat size/mode hints for downstream datasinks
128+
@return newly opened file, or nullptr on error. */
115129
ds_file_t *ds_open(ds_ctxt_t *ctxt, const char *path, MY_STAT *stat) {
116-
ds_file_t *file;
117-
118-
file = ctxt->datasink->open(ctxt, path, stat);
119-
if (file != NULL) {
120-
file->datasink = ctxt->datasink;
121-
}
130+
ds_file_t *file = ctxt->datasink->open(ctxt, path, stat);
131+
assert(file == nullptr ||
132+
(file->datasink != nullptr && file->ctxt != nullptr));
133+
return file;
134+
}
122135

136+
/** ds_open() + ds_track_metrics(), in one call. See the declaration
137+
in datasink.h for when to prefer this over a raw ds_open().
138+
@param[in] ctxt pipeline to open through
139+
@param[in] path path relative to the pipeline root
140+
@param[in] stat size/mode hints for downstream datasinks
141+
@param[in,out] metrics metrics instance to bind; nullptr disables tracking
142+
@return newly opened file, or nullptr on error. */
143+
ds_file_t *ds_tracked_open(ds_ctxt_t *ctxt, const char *path, MY_STAT *stat,
144+
xb_metrics *metrics) {
145+
ds_file_t *file = ds_open(ctxt, path, stat);
146+
ds_track_metrics(file, metrics);
123147
return file;
124148
}
125149

126-
/************************************************************************
127-
Write to a datasink file.
150+
/** Write a contiguous buffer through the owning datasink's write slot.
151+
Adds @p len to file->metrics when tracking is bound.
152+
@param[in,out] file ds_file_t previously returned by ds_open
153+
@param[in] buf bytes to write
154+
@param[in] len number of bytes at @p buf
128155
@return 0 on success, 1 on error. */
129156
int ds_write(ds_file_t *file, const void *buf, size_t len) {
157+
if (file->metrics != nullptr) {
158+
file->metrics->add_uncomp_size(len);
159+
}
130160
return file->datasink->write(file, buf, len);
131161
}
132162

133-
/************************************************************************
134-
Check if sparse files are supported.
135-
@return 1 if yes. */
163+
/** Check whether @p file's datasink supports sparse writes.
164+
@param[in] file file to probe
165+
@return 1 if write_sparse is implemented, 0 otherwise. */
136166
int ds_is_sparse_write_supported(ds_file_t *file) {
137167
if (file->datasink->write_sparse != nullptr) {
138168
return 1;
139169
}
140170
return 0;
141171
}
142172

143-
/************************************************************************
144-
Write sparse chunk if supported.
173+
/** Forward a sparse chunk through the owning datasink's write_sparse
174+
slot. Adds the sum of sparse_map[i].len (the packed payload, holes
175+
excluded) to file->metrics when tracking is bound.
176+
@param[in,out] file ds_file_t previously returned by ds_open
177+
@param[in] buf packed buffer containing the data bytes
178+
@param[in] len size of @p buf
179+
@param[in] sparse_map_size number of entries in @p sparse_map
180+
@param[in] sparse_map per-chunk (skip, len)
181+
@param[in] punch_hole_supported true if the destination filesystem
182+
can physically punch holes
145183
@return 0 on success, 1 on error. */
146184
int ds_write_sparse(ds_file_t *file, const void *buf, size_t len,
147185
size_t sparse_map_size, const ds_sparse_chunk_t *sparse_map,
148186
bool punch_hole_supported) {
149187
if (file->datasink->write_sparse != nullptr) {
188+
/* Count the packed payload only: holes do not occupy disk. */
189+
if (file->metrics != nullptr) {
190+
size_t packed_len = 0;
191+
for (size_t i = 0; i < sparse_map_size; i++) {
192+
packed_len += sparse_map[i].len;
193+
}
194+
file->metrics->add_uncomp_size(packed_len);
195+
}
150196
return file->datasink->write_sparse(file, buf, len, sparse_map_size,
151197
sparse_map, punch_hole_supported);
152198
}
153199
return 1;
154200
}
155201

156-
/************************************************************************
157-
Close a datasink file.
158-
@return 0 on success, 1, on error. */
202+
/** Close a datasink file.
203+
@param[in,out] file ds_file_t previously returned by ds_open
204+
@return 0 on success, 1 on error. */
159205
int ds_close(ds_file_t *file) { return file->datasink->close(file); }
160206

161-
/************************************************************************
162-
Destroy a datasink handle */
207+
/** Destroy a datasink handle.
208+
@param[in,out] ctxt datasink ctxt returned by ds_create / ds_set_pipe */
163209
void ds_destroy(ds_ctxt_t *ctxt) { ctxt->datasink->deinit(ctxt); }
164210

165-
/************************************************************************
166-
Set the destination pipe for a datasink (only makes sense for compress and
167-
tmpfile). */
211+
/** Wire @p ctxt's output into @p pipe_ctxt.
212+
@param[in,out] ctxt wrapper datasink context
213+
@param[in,out] pipe_ctxt next-stage datasink the wrapper writes into */
168214
void ds_set_pipe(ds_ctxt_t *ctxt, ds_ctxt_t *pipe_ctxt) {
169215
ctxt->pipe_ctxt = pipe_ctxt;
170216
}

0 commit comments

Comments
 (0)