Skip to content

feat: 为二级压缩引入可插拔的硬件加速 backend (#35077)#35367

Open
uk0 wants to merge 8 commits into
taosdata:mainfrom
uk0:fix/issue-35077-accel-compress
Open

feat: 为二级压缩引入可插拔的硬件加速 backend (#35077)#35367
uk0 wants to merge 8 commits into
taosdata:mainfrom
uk0:fix/issue-35077-accel-compress

Conversation

@uk0

@uk0 uk0 commented May 27, 2026

Copy link
Copy Markdown
Contributor

issue #35077 问:能不能让 TDengine 在硬件支持时用上加速版 zlib/zstd/lz4,不支持时回退默认实现。

改动

二级压缩在 tcompression.c 已经有 dispatch table compressL2Dict[],每个 codec 一组 {name, initFn, comprFn, decomprFn} 函数指针。当前 zlib/zstd/lz4 都是静态链接,无法在运行期替换。这个 PR 复用现有 dispatch table,做最小侵入的运行期 swap:

  • 新增 cmake 选项 BUILD_WITH_ACCEL_COMPRESS(仅 Linux,默认 OFF)。打开后 utillibdl
  • 新增 source/util/src/tcompression_accel.c:选项关闭时编译为空对象 + 一个 no-op stub;选项打开时启动期读 TAOS_COMPRESS_ACCEL_{ZLIB,ZSTD,LZ4}TAOS_COMPRESS_ACCEL=<dir>dlopen 用户指定的 .so,解析 compress2/uncompressZSTD_compress/ZSTD_decompressLZ4_compress_default/LZ4_decompress_safe 这些公开符号,全部成功就替换 compressL2Dict[] 对应 codec 的函数指针;任何一步失败 uWarn 一行,dispatch table 原样不动。
  • tsCompressInit() 末尾无条件调用 tcompressionAccelInit(),stub 模式零开销。
  • 新增 tools/compressBench 微基准(仅 BUILD_TOOLS=ON 编译):直接调 dispatch table 跑 codec × shape × size 矩阵,输出 mean/p50/p95/stdev/throughput/压缩比,per-iter memcmp 校验防止 accel backend 输出错数据。
  • 中文文档 docs/zh/26-tdinternal/11-compress.md 加一节「使用硬件加速的二级压缩库(可选)」。

xz 不在替换范围(走的是 fast-lzma2 没有通用 drop-in 加速版),tsz 也不动(是内部实现)。JNI 路径、writeRaw、其他 codec 全都未动。

使用方式

1. 编译

mkdir build && cd build
cmake -DBUILD_WITH_ACCEL_COMPRESS=ON .. \
      # 可选:如果同时要 compressBench 评测工具
      -DBUILD_TOOLS=ON
make -j$(nproc)

BUILD_WITH_ACCEL_COMPRESS=ON 之后 util 会自动链 libdl未传这个选项时行为与本 PR 之前完全一样,没有任何运行期开销。

2. 替代库的符号约定

替代库必须导出与上游一致的公共符号、保持参数顺序和返回值语义。绝大多数硬件加速版本(Intel QAT/IAA、ISA-L、ARM-optimized 等)本身就是 drop-in 替换:

替代库 需要的符号
libz.so compress2, uncompress
libzstd.so ZSTD_compress, ZSTD_decompress
liblz4.so LZ4_compress_default, LZ4_decompress_safe

3. 启动 taosd 时挂入

任选一种方式:

# 方式 A:单独指定每个 codec 的 .so 路径
export TAOS_COMPRESS_ACCEL_ZLIB=/opt/qat-zlib/libz.so
export TAOS_COMPRESS_ACCEL_ZSTD=/opt/qat-zlib/libzstd.so
export TAOS_COMPRESS_ACCEL_LZ4=/opt/qat-zlib/liblz4.so
taosd

# 方式 B:指定一个目录,按惯例文件名加载(libz.so / libzstd.so / liblz4.so)
export TAOS_COMPRESS_ACCEL=/opt/qat-zlib
taosd

# 禁用:不设置任何 TAOS_COMPRESS_ACCEL* 变量即可(设为空值等同于未设置)
unset TAOS_COMPRESS_ACCEL TAOS_COMPRESS_ACCEL_ZLIB TAOS_COMPRESS_ACCEL_ZSTD TAOS_COMPRESS_ACCEL_LZ4
taosd

环境变量优先级:单独的 TAOS_COMPRESS_ACCEL_<CODEC> > TAOS_COMPRESS_ACCEL=<dir>

4. 确认是否生效

taosd 启动日志(uInfo 级别)会有一行类似:

UTL INFO  accel zlib: loaded from /opt/qat-zlib/libz.so, L2_ZLIB dispatch patched
UTL INFO  accel zstd: loaded from /opt/qat-zlib/libzstd.so, L2_ZSTD dispatch patched
UTL INFO  accel lz4:  loaded from /opt/qat-zlib/liblz4.so,  L2_LZ4  dispatch patched

失败回退会打 uWarn

UTL WARN  accel zlib: dlopen("/opt/wrong/libz.so") failed: cannot open shared object file: No such file or directory

完全没设环境变量时只打一行 INFO:

UTL INFO  accel compression: TAOS_COMPRESS_ACCEL{,_ZLIB,_ZSTD,_LZ4} unset; using stock L2 implementations

5. 评估加速效果

compressBench 可以直接对 dispatch table 跑微基准,绕开 SQL/网络/WAL,结果只反映压缩本身:

# 1. 基线:不设 env var,跑 stock
./build/bin/compressBench --codec all --size 1 --iters 30 --warmup 5 \
    --shape mixed --label stock --csv result.csv

# 2. 切到加速库再跑一次
export TAOS_COMPRESS_ACCEL=/opt/qat-zlib
./build/bin/compressBench --codec all --size 1 --iters 30 --warmup 5 \
    --shape mixed --label accel --csv result.csv

输出每个 codec 的 mean / p50 / p95 / stdev 以及 MB/s 吞吐和压缩比,行尾会标 backend=stock|accel--shape 提供 random / repeating / sequential / mixed 四种数据形态;--size 支持 0.0625(64 KiB)、1(1 MiB)等,覆盖真实列块的常见尺寸。每次 iter 都会 memcmp 校验,accel 输出错数据会立即 abort。

详细用法、注意事项(替代库的二级依赖、container 部署、TSZ/XZ 不在替换范围等)见 docs/zh/26-tdinternal/11-compress.md 新增的「使用硬件加速的二级压缩库(可选)」节。

测试

在 Ubuntu 22.04 / Docker / x86_64 上 BUILD_WITH_ACCEL_COMPRESS=ON BUILD_TOOLS=ON 编译,跑 compressBench 全矩阵:

  • codec ∈ {lz4, zlib, zstd, xz} × shape ∈ {random, repeating, sequential, mixed} × size ∈ {64KiB, 1MiB}
  • 每 cell 30 iter measure + 5 iter warmup,2 轮(共 128 cells / 7,680 次 compress+decompress)
  • stock = 默认(不设 env var,走静态打包的 ext_zlib/lz4/zstd)
  • accel = TAOS_COMPRESS_ACCEL_{ZLIB,ZSTD,LZ4} 指向 system 的 /usr/lib/x86_64-linux-gnu/libz.so.1 等(ABI 完全相同,但是 distro 的 release-optimized build)

compress 吞吐倍数(accel ÷ stock, MB/s)

codec 64KiB random 64KiB repeating 64KiB sequential 64KiB mixed 1MiB random 1MiB repeating 1MiB sequential 1MiB mixed
lz4 4.56× 9.17× 8.43× 7.91× 2.24× 9.44× 6.03× 6.57×
zlib 2.08× 2.58× 1.92× 2.39× 1.81× 2.37× 2.56× 2.26×
zstd 3.83× 3.75× 2.15× 2.88× 4.08× 4.63× 3.11× 3.28×

decompress 吞吐倍数

codec 64KiB random 64KiB repeating 64KiB sequential 64KiB mixed 1MiB random 1MiB repeating 1MiB sequential 1MiB mixed
lz4 0.84× 6.08× 2.06× 2.17× 1.05× 5.09× 1.42× 1.85×
zlib 5.15× 4.90× 2.54× 3.60× 3.46× 3.71× 3.59× 3.71×
zstd 0.95× 9.07× 6.96× 7.24× 1.61× 11.12× 7.90× 6.97×

关键观察

  • dispatch swap 本身零功能成本:所有 cell 下 accel 行的压缩比与 stock 行完全相等,per-iter memcmp 在 7,680 次里 0 失败 —— 切换不影响 on-wire 数据。
  • xz 自动 fallback 工作正确:没设 TAOS_COMPRESS_ACCEL_XZ,xz 的 accel 行 backend= 仍报 stock 且数字与 stock 一致(16/16 cells)。
  • 本测试场景下 system lib 比静态打包快 2–11×:这是个意外但合理的发现 —— TDengine 自己 build 的 ext_zlib/lz4/zstd 与 distro 的 release-optimized build 之间已经有可观差距,并不需要硬件加速器就能看到收益。真实 QAT/IAA/ISA-L 替换叠加这条基线还会更高。

兼容性

  • 默认 OFF:行为完全不变。
  • 选项 ON 但 env var 未设:log 一行 INFO,static 实现继续生效。
  • env var 设了但 dlopen 失败 / 符号缺:log 一行 WARN,static 实现继续生效。
  • 加载成功后该进程的所有列块都走 accel 直到进程退出。
  • 替代库必须与上游 ABI 一致(参数顺序、返回值语义),这是 dlopen drop-in 替换的常规约定。

Refs #35077

uk0 added 4 commits May 28, 2026 03:34
…5077)

Adds an opt-in path for swapping the static zlib/zstd/lz4 used by the L2
dispatch table with ABI-compatible drop-in replacements (e.g. Intel
QAT/IAA accelerated libz, ISA-L's libz, ARM-optimized libzstd).

Mechanism:
- New cmake option BUILD_WITH_ACCEL_COMPRESS (Linux only, OFF by default)
  links util against libdl and defines TD_BUILD_WITH_ACCEL_COMPRESS.
- New translation unit source/util/src/tcompression_accel.c, compiled
  to an empty object when the option is off, exposes
  tcompressionAccelInit(). When the option is on, it reads
  TAOS_COMPRESS_ACCEL / TAOS_COMPRESS_ACCEL_{ZLIB,ZSTD,LZ4}, dlopens
  the named .so files, resolves the public symbols (compress2,
  uncompress, ZSTD_compress, ZSTD_decompress, LZ4_compress_default,
  LZ4_decompress_safe), and rebinds the matching entries in
  compressL2Dict[]. Any failure (env unset, file missing, dlopen
  failure, missing symbol) is logged via uWarn and leaves the stock
  static implementation in place.
- tsCompressInit() calls tcompressionAccelInit() unconditionally; the
  stub keeps callers free of #ifdef plumbing when the option is off.

xz and tsz are intentionally not switchable: tsz is internal, and
fast-lzma2 does not have a common drop-in accelerated counterpart.
…ata#35077)

A small CLI that calls compressL2Dict[codec].comprFn / .decomprFn
directly on synthetic data and reports mean/p50/p95/stdev plus
throughput (MB/s) and compression ratio for each codec, so an operator
can quantify the speedup from an accelerated backend swap (or just
spot a regression).

Flags: --codec {lz4|zlib|zstd|xz|all}, --size MiB, --iters N, --warmup
N, --lvl {low|medium|high}, --shape {random|repeating|sequential|mixed},
--seed N, --csv PATH, --label STR.

Per-iteration memcmp guards against silently-broken accel backends.
Built only when BUILD_TOOLS=ON. Output line ends with
backend=stock|accel determined by comparing the dispatch-table
function pointer against the stock symbols.
Adds a new subsection "使用硬件加速的二级压缩库(可选)" under
docs/zh/26-tdinternal/11-compress.md covering the BUILD_WITH_ACCEL_COMPRESS
cmake option, the TAOS_COMPRESS_ACCEL{,_ZLIB,_ZSTD,_LZ4} env vars,
the symbol contract a replacement .so must satisfy, the startup log
lines that confirm activation or fallback, and how to use the new
compressBench tool to quantify the speedup.
compressBench is a standalone micro-benchmark that runs outside the
TDengine memory/file accounting layer, so it has to call libc
malloc/free/fopen/clock_gettime directly. include/os/os*.h gates
those behind *_FUNC_TAOS_FORBID macros (-Werror=implicit-function-
declaration / -Werror=int-to-pointer-cast) unless ALLOW_FORBID_FUNC
is defined first. Define it at the top of the translation unit,
before any include that pulls in os/, matching the pattern used by
other standalone tools.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces runtime-pluggable accelerated L2 compression backends (zlib, zstd, lz4) for TDengine on Linux, enabling the dynamic loading of hardware-accelerated libraries (e.g., Intel QAT/IAA) at startup. It also includes a new micro-benchmark tool, compressBench, to evaluate compression performance. The code review identified critical security vulnerabilities in the dynamic loading wrapper functions (accelCompress_* and accelDecompress_* for zlib, zstd, and lz4). Specifically, missing boundary checks on outputSize and compressedSize can lead to integer underflows, out-of-bounds reads, and severe buffer overflows during fallback memory copies. Robust input validation and size checks must be added to these functions to prevent potential crashes or exploits.

Comment on lines +68 to +80
static int32_t accelCompress_zlib(const char *const input, const int32_t inputSize, char *const output,
int32_t outputSize, const char type, int8_t lvl) {
unsigned long dstLen = (unsigned long)(outputSize - 1);
int rc = g_zlibAccel.compress2_fn((unsigned char *)(output + 1), &dstLen,
(const unsigned char *)input, (unsigned long)inputSize, lvl);
if (rc == 0 /* Z_OK */) {
output[0] = 1;
return (int32_t)dstLen + 1;
}
output[0] = 0;
memcpy(output + 1, input, inputSize);
return inputSize + 1;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-high high

accelCompress_zlib 中,如果 outputSize <= 1outputSize - 1 会溢出为极大的无符号数(ULONG_MAX),这在调用 compress2_fn 时可能会导致缓冲区溢出。此外,如果 inputSize < 0 也会导致未定义行为。建议在函数开头添加对 outputSizeinputSize 的边界检查,并在回退拷贝时确保输出缓冲区足够大。

static int32_t accelCompress_zlib(const char *const input, const int32_t inputSize, char *const output,
                                  int32_t outputSize, const char type, int8_t lvl) {
  if (outputSize <= 1 || inputSize < 0) {
    return TSDB_CODE_INVALID_PARA;
  }
  unsigned long dstLen = (unsigned long)(outputSize - 1);
  int           rc     = g_zlibAccel.compress2_fn((unsigned char *)(output + 1), &dstLen,
                                                   (const unsigned char *)input, (unsigned long)inputSize, lvl);
  if (rc == 0 /* Z_OK */) {
    output[0] = 1;
    return (int32_t)dstLen + 1;
  }
  if (outputSize - 1 < inputSize) {
    return TSDB_CODE_INVALID_PARA;
  }
  output[0] = 0;
  memcpy(output + 1, input, inputSize);
  return inputSize + 1;
}

Comment on lines +82 to +96
static int32_t accelDecompress_zlib(const char *const input, const int32_t compressedSize, char *const output,
int32_t outputSize, const char type) {
if (input[0] == 1) {
unsigned long len = (unsigned long)outputSize;
int rc = g_zlibAccel.uncompress_fn((unsigned char *)output, &len,
(const unsigned char *)input + 1,
(unsigned long)(compressedSize - 1));
if (rc == 0) return (int32_t)len;
return TSDB_CODE_THIRDPARTY_ERROR;
} else if (input[0] == 0) {
memcpy(output, input + 1, compressedSize - 1);
return compressedSize - 1;
}
return TSDB_CODE_THIRDPARTY_ERROR;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-high high

accelDecompress_zlib 中,如果 compressedSize <= 0,访问 input[0] 会导致越界读取。此外,如果 input[0] == 0memcpy 的长度参数 compressedSize - 1 会溢出为极大的正数,导致严重的缓冲区溢出和崩溃。建议在函数开头添加对 compressedSize 的边界检查,并在拷贝前校验输出缓冲区大小。

static int32_t accelDecompress_zlib(const char *const input, const int32_t compressedSize, char *const output,
                                    int32_t outputSize, const char type) {
  if (compressedSize <= 0) {
    return TSDB_CODE_INVALID_PARA;
  }
  if (input[0] == 1) {
    unsigned long len = (unsigned long)outputSize;
    int           rc  = g_zlibAccel.uncompress_fn((unsigned char *)output, &len,
                                                  (const unsigned char *)input + 1,
                                                  (unsigned long)(compressedSize - 1));
    if (rc == 0) return (int32_t)len;
    return TSDB_CODE_THIRDPARTY_ERROR;
  } else if (input[0] == 0) {
    if (outputSize < compressedSize - 1) {
      return TSDB_CODE_INVALID_PARA;
    }
    memcpy(output, input + 1, compressedSize - 1);
    return compressedSize - 1;
  }
  return TSDB_CODE_THIRDPARTY_ERROR;
}

Comment on lines +98 to +110
static int32_t accelCompress_zstd(const char *const input, const int32_t inputSize, char *const output,
int32_t outputSize, const char type, int8_t lvl) {
size_t len = g_zstdAccel.compress_fn(output + 1, (size_t)(outputSize - 1), input, (size_t)inputSize, lvl);
// ZSTD_isError() returns a value > srcSize for error codes, so the same
// sentinel as the stock path catches both error and ratio-not-worth-it.
if (len > (size_t)inputSize) {
output[0] = 0;
memcpy(output + 1, input, inputSize);
return inputSize + 1;
}
output[0] = 1;
return (int32_t)len + 1;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-high high

accelCompress_zstd 中,如果 outputSize <= 1outputSize - 1 会溢出为 SIZE_MAX,在调用 compress_fn 时可能会导致缓冲区溢出。建议在函数开头添加对 outputSizeinputSize 的边界检查,并在回退拷贝时确保输出缓冲区足够大。

static int32_t accelCompress_zstd(const char *const input, const int32_t inputSize, char *const output,
                                  int32_t outputSize, const char type, int8_t lvl) {
  if (outputSize <= 1 || inputSize < 0) {
    return TSDB_CODE_INVALID_PARA;
  }
  size_t len = g_zstdAccel.compress_fn(output + 1, (size_t)(outputSize - 1), input, (size_t)inputSize, lvl);
  // ZSTD_isError() returns a value > srcSize for error codes, so the same
  // sentinel as the stock path catches both error and ratio-not-worth-it.
  if (len > (size_t)inputSize) {
    if (outputSize - 1 < inputSize) {
      return TSDB_CODE_INVALID_PARA;
    }
    output[0] = 0;
    memcpy(output + 1, input, inputSize);
    return inputSize + 1;
  }
  output[0] = 1;
  return (int32_t)len + 1;
}

Comment on lines +112 to +121
static int32_t accelDecompress_zstd(const char *const input, const int32_t compressedSize, char *const output,
int32_t outputSize, const char type) {
if (input[0] == 1) {
return (int32_t)g_zstdAccel.decompress_fn(output, (size_t)outputSize, input + 1, (size_t)(compressedSize - 1));
} else if (input[0] == 0) {
memcpy(output, input + 1, compressedSize - 1);
return compressedSize - 1;
}
return TSDB_CODE_THIRDPARTY_ERROR;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-high high

accelDecompress_zstd 中,如果 compressedSize <= 0,访问 input[0] 会导致越界读取。此外,如果 input[0] == 0memcpy 的长度参数 compressedSize - 1 会溢出为极大的正数,导致严重的缓冲区溢出和崩溃。建议在函数开头添加对 compressedSize 的边界检查,并在拷贝前校验输出缓冲区大小。

static int32_t accelDecompress_zstd(const char *const input, const int32_t compressedSize, char *const output,
                                    int32_t outputSize, const char type) {
  if (compressedSize <= 0) {
    return TSDB_CODE_INVALID_PARA;
  }
  if (input[0] == 1) {
    return (int32_t)g_zstdAccel.decompress_fn(output, (size_t)outputSize, input + 1, (size_t)(compressedSize - 1));
  } else if (input[0] == 0) {
    if (outputSize < compressedSize - 1) {
      return TSDB_CODE_INVALID_PARA;
    }
    memcpy(output, input + 1, compressedSize - 1);
    return compressedSize - 1;
  }
  return TSDB_CODE_THIRDPARTY_ERROR;
}

Comment on lines +123 to +133
static int32_t accelCompress_lz4(const char *const input, const int32_t inputSize, char *const output,
int32_t outputSize, const char type, int8_t lvl) {
const int32_t n = g_lz4Accel.compress_default_fn(input, output + 1, inputSize, outputSize - 1);
if (n <= 0 || n > inputSize) {
output[0] = 0;
memcpy(output + 1, input, inputSize);
return inputSize + 1;
}
output[0] = 1;
return n + 1;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-high high

accelCompress_lz4 中,如果 outputSize <= 1outputSize - 1 会溢出为负数,在调用 compress_default_fn 时可能会导致未定义行为或缓冲区溢出。建议在函数开头添加对 outputSizeinputSize 的边界检查,并在回退拷贝时确保输出缓冲区足够大。

static int32_t accelCompress_lz4(const char *const input, const int32_t inputSize, char *const output,
                                 int32_t outputSize, const char type, int8_t lvl) {
  if (outputSize <= 1 || inputSize < 0) {
    return TSDB_CODE_INVALID_PARA;
  }
  const int32_t n = g_lz4Accel.compress_default_fn(input, output + 1, inputSize, outputSize - 1);
  if (n <= 0 || n > inputSize) {
    if (outputSize - 1 < inputSize) {
      return TSDB_CODE_INVALID_PARA;
    }
    output[0] = 0;
    memcpy(output + 1, input, inputSize);
    return inputSize + 1;
  }
  output[0] = 1;
  return n + 1;
}

Comment on lines +135 to +149
static int32_t accelDecompress_lz4(const char *const input, const int32_t compressedSize, char *const output,
int32_t outputSize, const char type) {
if (input[0] == 1) {
const int32_t n = g_lz4Accel.decompress_safe_fn(input + 1, output, compressedSize - 1, outputSize);
if (n < 0) {
uError("accel lz4: LZ4_decompress_safe returned %d", n);
return TSDB_CODE_THIRDPARTY_ERROR;
}
return n;
} else if (input[0] == 0) {
memcpy(output, input + 1, compressedSize - 1);
return compressedSize - 1;
}
return TSDB_CODE_THIRDPARTY_ERROR;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-high high

accelDecompress_lz4 中,如果 compressedSize <= 0,访问 input[0] 会导致越界读取。此外,如果 input[0] == 0memcpy 的长度参数 compressedSize - 1 会溢出为极大的正数,导致严重的缓冲区溢出和崩溃。建议在函数开头添加对 compressedSize 的边界检查,并在拷贝前校验输出缓冲区大小。

static int32_t accelDecompress_lz4(const char *const input, const int32_t compressedSize, char *const output,
                                   int32_t outputSize, const char type) {
  if (compressedSize <= 0) {
    return TSDB_CODE_INVALID_PARA;
  }
  if (input[0] == 1) {
    const int32_t n = g_lz4Accel.decompress_safe_fn(input + 1, output, compressedSize - 1, outputSize);
    if (n < 0) {
      uError("accel lz4: LZ4_decompress_safe returned %d", n);
      return TSDB_CODE_THIRDPARTY_ERROR;
    }
    return n;
  } else if (input[0] == 0) {
    if (outputSize < compressedSize - 1) {
      return TSDB_CODE_INVALID_PARA;
    }
    memcpy(output, input + 1, compressedSize - 1);
    return compressedSize - 1;
  }
  return TSDB_CODE_THIRDPARTY_ERROR;
}

…data#35077)

Addresses six security-high inline reviews from gemini-code-assist on
PR taosdata#35367.

Each of accelCompress_{zlib,zstd,lz4} and accelDecompress_{zlib,zstd,lz4}
now guards its input arithmetic:

  - compress wrappers: reject inputSize < 0 || outputSize <= 1, and the
    verbatim-fallback path also refuses if (outputSize - 1) cannot hold
    `inputSize` bytes. Without this, a tiny outputSize would underflow
    `outputSize - 1` to ULONG_MAX / SIZE_MAX and the underlying codec
    would overrun the output buffer.
  - decompress wrappers: reject compressedSize <= 0 (which would also
    have made the input[0] read out-of-bounds), and the verbatim path
    refuses when outputSize cannot hold `compressedSize - 1` bytes.

Failures return TSDB_CODE_INVALID_PARA -- the same error code the rest
of tcompression.c uses for argument-validation rejections -- so a
broken caller / a misbehaving accel library surfaces as a regular
SQL-visible error instead of silently corrupting memory.

The stock implementations in tcompression.c rely on the same caller
invariants but skip the checks. We harden the accel wrappers because
they're behind dlopen and therefore a strictly larger attack surface;
hardening the stock path is intentionally out of scope for this PR.
@uk0

uk0 commented May 27, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the security review — all six bounds-check issues are valid and addressed in commit 8fa8f71.

Each of the six accelCompress_{zlib,zstd,lz4} / accelDecompress_{zlib,zstd,lz4} wrappers now sanity-checks its sizes before doing any arithmetic that would otherwise underflow:

  • Compress wrappers: refuse inputSize < 0 || outputSize <= 1. The verbatim-fallback path additionally refuses if outputSize - 1 < inputSize, so a tiny output buffer can't be memcpy-overrun.
  • Decompress wrappers: refuse compressedSize <= 0 (which would also have made the input[0] read out-of-bounds). The verbatim path refuses if outputSize < compressedSize - 1.
  • Failures return TSDB_CODE_INVALID_PARA — the same code the rest of tcompression.c uses for argument-validation rejections, so a misbehaving accel library surfaces as a regular SQL-visible error instead of silently corrupting memory.

Why only the accel wrappers (the stock implementations in tcompression.c carry the same patterns without the checks): this dispatch lives behind dlopen of an operator-supplied .so, which is a strictly larger attack surface than the statically linked stock path. Hardening the stock path is the same diff but intentionally out of scope for this PR so the change set stays focused on the BUILD_WITH_ACCEL_COMPRESS codepath you're reviewing.

Re-verified end to end on lvm1: compressBench --codec all --shape mixed with TAOS_COMPRESS_ACCEL_{ZLIB,ZSTD,LZ4} pointed at system libs — lz4/zlib/zstd all report backend=accel, compression ratios identical to the stock run, per-iter memcmp 0 failures. xz stays on backend=stock (fallback works as before).

uk0 added 2 commits May 28, 2026 04:51
…a#35077)

The two log-output code blocks I added in the previous commit were
unlabeled, which trips the markdownlint MD040 rule used by the docs
CI on this repo. Tag them as `text` (log output, no real syntax to
highlight).

Fixes:
  docs/zh/26-tdinternal/11-compress.md:72 MD040/fenced-code-language
  docs/zh/26-tdinternal/11-compress.md:79 MD040/fenced-code-language
…ata#35077)

l2CompressImpl_{zlib,zstd,xz} are defined in tcompression.c only on
Linux; on Windows / _TD_DARWIN_64 the dispatch table aliases all of
them to l2CompressImpl_lz4 and the actual functions are absent. The
bench's detect_backend() referenced all three unconditionally, which
broke the link on macOS arm64:

  Undefined symbols for architecture arm64:
    "_l2CompressImpl_xz", "_l2CompressImpl_zlib", "_l2CompressImpl_zstd"

Gate the extern declarations and the switch arms on
!WINDOWS && !_TD_DARWIN_64; on those platforms compare against
l2CompressImpl_lz4 (which is what the stock dispatch table holds
there anyway). BUILD_WITH_ACCEL_COMPRESS is Linux-only so accel
detection on Windows/Darwin can only ever report "stock", matching
the platform reality.
}
} else if (strcmp(argv[i], "--size") == 0) {
NEED_ARG("--size");
size_mib = atof(argv[i]);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using bytes (B) or KiB is better than using MiB, because floating-point numbers are not user-friendly for input and are prone to precision errors in mathematical calculations.

Please also update the documents.


// Resolve "<dir>/<basename>" without overflowing the destination buffer.
// Returns 0 on success, -1 on overflow (with the buffer left empty).
static int joinDirAndName(char *dst, size_t dstSize, const char *dir, const char *base) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. propose change the function name to joinPath.
  2. dir may end with /, propose handle this case so that user is free to include/exclude the end / in the environment variable.
  3. base should be renamed to file.

Comment thread source/util/src/tcompression_accel.c Outdated
uInfo("accel compression: TAOS_COMPRESS_ACCEL{,_ZLIB,_ZSTD,_LZ4} unset; using stock L2 implementations");
return;
}
if (dir != NULL && strcmp(dir, "off") == 0) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use a case-insensitive comparison here, as users might input OFF, Off, or off.

and from the view of design, do we really need the off option as there are only 3 TAOS_COMPRESS_ACCEL_* variables?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed — removed the off option entirely in 3f453e0. The feature is opt-in, so leaving the variables unset is the off switch; a magic string on a path-valued variable was redundant (and case-sensitive, as you noted). Empty values are now treated the same as unset, so TAOS_COMPRESS_ACCEL= still works as an explicit disable in unit files / container images where removing a variable is awkward, with no case-sensitivity involved. Docs table updated to match.

@localvar

localvar commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

@uk0 thanks for the contribution, please check my comments above.

…5077)

Review feedback on taosdata#35367: the feature is opt-in, so unset already means
disabled; a magic string on a path-valued variable was redundant and
case-sensitive. Empty values now count as unset, so TAOS_COMPRESS_ACCEL=
still works as an explicit kill switch where deleting a variable is
awkward (unit files, container images).
@uk0

uk0 commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

Verified the new env-var semantics from 3f453e0 on Ubuntu 22.04 x86_64 (gcc 11.4, BUILD_WITH_ACCEL_COMPRESS=ON, BUILD_TOOLS=ON, Release). Probe is compressBench's backend= field, which compares the live compressL2Dict[] pointer against the stock implementation; every iteration also memcmp-checks decompress(compress(x)) == x.

case env expect result
unset none of the 4 vars set stock
empty values TAOS_COMPRESS_ACCEL= / TAOS_COMPRESS_ACCEL_ZLIB= / all 4 empty stock
off, OFF, Off no longer magic — behaves as a nonexistent dir, graceful fallback stock
bogus dir TAOS_COMPRESS_ACCEL=/nonexistent/dir stock
per-codec .so TAOS_COMPRESS_ACCEL_{ZLIB,ZSTD,LZ4} → system libz.so.1 / libzstd.so.1 / liblz4.so.1 accel ×3
directory convention TAOS_COMPRESS_ACCEL=<dir> with libz.so/libzstd.so/liblz4.so symlinks accel ×3
mixed TAOS_COMPRESS_ACCEL_ZLIB= empty + TAOS_COMPRESS_ACCEL_LZ4 set zlib stock, lz4 accel
xz accel dir set stock (never patched)

17/17 pass.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants