Skip to content

Commit 71c4694

Browse files
jihuayujihuayu
authored
Merge branch 'unstable' into feat-set-ifeq
2 parents eb4f468 + d942f53 commit 71c4694

4 files changed

Lines changed: 85 additions & 11 deletions

File tree

‎.github/workflows/kvrocks.yaml‎

Lines changed: 33 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,9 @@ concurrency:
3232
group: ${{ github.workflow }}-${{ github.event_name }}-${{ github.event.number || github.run_id }}
3333
cancel-in-progress: true
3434

35+
env:
36+
KVROCKS_DEPS_CACHE_DIR: build-deps
37+
3538
jobs:
3639
precondition:
3740
name: Precondition
@@ -72,6 +75,18 @@ jobs:
7275
runs-on: ubuntu-24.04
7376
steps:
7477
- uses: actions/checkout@v6
78+
- name: Restore Kvrocks dependency cache
79+
id: cache-kvrocks-deps
80+
uses: actions/cache@v5
81+
with:
82+
path: ${{ env.KVROCKS_DEPS_CACHE_DIR }}
83+
key: kvrocks-full-deps-${{ hashFiles('x.py', 'CMakeLists.txt', 'cmake/**/*.cmake') }}
84+
enableCrossOsArchive: true
85+
- name: Fetch Kvrocks dependencies
86+
if: ${{ steps.cache-kvrocks-deps.outputs.cache-hit != 'true' }}
87+
run: |
88+
./x.py fetch-deps ${{ env.KVROCKS_DEPS_CACHE_DIR }}
89+
./x.py fetch-deps ${{ env.KVROCKS_DEPS_CACHE_DIR }} -D ENABLE_LUAJIT=OFF
7590
- uses: actions/setup-go@v6
7691
with:
7792
go-version-file: 'tests/gocase/go.mod'
@@ -87,7 +102,7 @@ jobs:
87102
run: ./x.py check format --clang-format-path clang-format-18
88103
- name: Check with clang-tidy
89104
run: |
90-
./x.py build --skip-build
105+
./x.py build --skip-build --dep-dir ${{ env.KVROCKS_DEPS_CACHE_DIR }}
91106
./x.py check tidy -j $(nproc) --clang-tidy-path clang-tidy-18 --run-clang-tidy-path run-clang-tidy-18
92107
- name: Lint with golangci-lint
93108
run: ./x.py check golangci-lint
@@ -317,6 +332,12 @@ jobs:
317332
- uses: actions/checkout@v6
318333
with:
319334
fetch-depth: 0
335+
- name: Restore Kvrocks dependency cache
336+
uses: actions/cache@v5
337+
with:
338+
path: ${{ env.KVROCKS_DEPS_CACHE_DIR }}
339+
key: kvrocks-full-deps-${{ hashFiles('x.py', 'CMakeLists.txt', 'cmake/**/*.cmake') }}
340+
enableCrossOsArchive: true
320341
- uses: actions/setup-python@v6
321342
if: ${{ !matrix.arm_linux }}
322343
with:
@@ -339,19 +360,19 @@ jobs:
339360
run: |
340361
./x.py build -j$NPROC --unittest --compiler ${{ matrix.compiler }} ${{ matrix.without_jemalloc }} \
341362
${{ matrix.without_luajit }} ${{ matrix.with_ninja }} ${{ matrix.with_sanitizer }} ${{ matrix.with_openssl }} \
342-
${{ matrix.new_encoding }} ${{ env.CMAKE_EXTRA_DEFS }}
363+
${{ matrix.new_encoding }} ${{ env.CMAKE_EXTRA_DEFS }} --dep-dir ${{ env.KVROCKS_DEPS_CACHE_DIR }}
343364
344365
- name: Build Kvrocks (SonarCloud)
345366
if: ${{ matrix.sonarcloud }}
346367
run: |
347-
build-wrapper-linux-x86-64 --out-dir ${{ env.SONARCLOUD_OUTPUT_DIR }} ./x.py build -j$NPROC --compiler ${{ matrix.compiler }} --skip-build
368+
build-wrapper-linux-x86-64 --out-dir ${{ env.SONARCLOUD_OUTPUT_DIR }} ./x.py build -j$NPROC --compiler ${{ matrix.compiler }} --skip-build --dep-dir ${{ env.KVROCKS_DEPS_CACHE_DIR }}
348369
cp -r build _build
349-
build-wrapper-linux-x86-64 --out-dir ${{ env.SONARCLOUD_OUTPUT_DIR }} ./x.py build -j$NPROC --unittest --compiler ${{ matrix.compiler }} ${{ matrix.sonarcloud }}
370+
build-wrapper-linux-x86-64 --out-dir ${{ env.SONARCLOUD_OUTPUT_DIR }} ./x.py build -j$NPROC --unittest --compiler ${{ matrix.compiler }} ${{ matrix.sonarcloud }} --dep-dir ${{ env.KVROCKS_DEPS_CACHE_DIR }}
350371
351372
- name: Build Kvrocks (RISC-V)
352373
if: ${{ matrix.riscv_toolchain }}
353374
run: |
354-
./x.py build -j$NPROC --unittest --toolchain ${{ matrix.toolchain_file }}
375+
./x.py build -j$NPROC --unittest --toolchain ${{ matrix.toolchain_file }} --dep-dir ${{ env.KVROCKS_DEPS_CACHE_DIR }}
355376
356377
- name: Setup Coredump
357378
if: ${{ startsWith(matrix.os, 'ubuntu') }}
@@ -618,6 +639,12 @@ jobs:
618639
pushd redis-6.2.14 && USE_JEMALLOC=no make -j$NPROC redis-server && mv src/redis-server $HOME/local/bin/ && popd
619640
620641
- uses: actions/checkout@v6
642+
- name: Restore Kvrocks dependency cache
643+
uses: actions/cache@v5
644+
with:
645+
path: ${{ env.KVROCKS_DEPS_CACHE_DIR }}
646+
key: kvrocks-full-deps-${{ hashFiles('x.py', 'CMakeLists.txt', 'cmake/**/*.cmake') }}
647+
enableCrossOsArchive: true
621648
- uses: actions/setup-go@v6
622649
if: ${{ !startsWith(matrix.image, 'opensuse') }}
623650
with:
@@ -626,7 +653,7 @@ jobs:
626653

627654
- name: Build Kvrocks
628655
run: |
629-
./x.py build -j$NPROC --unittest --compiler ${{ matrix.compiler }} ${{ matrix.disable_jemalloc }}
656+
./x.py build -j$NPROC --unittest --compiler ${{ matrix.compiler }} ${{ matrix.disable_jemalloc }} --dep-dir ${{ env.KVROCKS_DEPS_CACHE_DIR }}
630657
631658
- name: Run Unit Test
632659
run: |

‎src/commands/cmd_tdigest.cc‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -466,9 +466,7 @@ class CommandTDigestMerge : public Commander {
466466
} else {
467467
options_.compression = *compression;
468468
}
469-
}
470-
471-
if (parser.EatEqICase(kOverrideArg)) {
469+
} else if (parser.EatEqICase(kOverrideArg)) {
472470
if (options_.override_flag) { // override already set
473471
return {Status::RedisParseErr, errWrongNumOfArguments};
474472
}

‎tests/gocase/unit/type/tdigest/tdigest_test.go‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -642,6 +642,53 @@ func tdigestTests(t *testing.T, configs util.KvrocksServerConfigs) {
642642
validation(newDestKey2)
643643
})
644644

645+
// https://github.com/apache/kvrocks/issues/3447
646+
t.Run("tdigest.merge with COMPRESSION parameter (issue #3447)", func(t *testing.T) {
647+
keyPrefix := "tdigest_merge_compression_"
648+
649+
srcKey := keyPrefix + "src"
650+
destKey := keyPrefix + "dest"
651+
652+
require.NoError(t, rdb.Do(ctx, "TDIGEST.CREATE", srcKey, "compression", "100").Err())
653+
require.NoError(t, rdb.Do(ctx, "TDIGEST.ADD", srcKey, "1", "2", "3", "4", "5").Err())
654+
655+
// This should succeed, not return "ERR wrong keyword"
656+
// Issue #3447: TDIGEST.MERGE dest 1 src COMPRESSION 100 fails with "ERR wrong keyword"
657+
require.NoError(t, rdb.Do(ctx, "TDIGEST.MERGE", destKey, 1, srcKey, "COMPRESSION", "100").Err())
658+
659+
// Verify the merge worked
660+
rsp := rdb.Do(ctx, "TDIGEST.INFO", destKey)
661+
require.NoError(t, rsp.Err())
662+
info := toTdigestInfo(t, rsp.Val())
663+
require.EqualValues(t, 100, info.Compression)
664+
require.EqualValues(t, 5, info.Observations)
665+
})
666+
667+
// Test COMPRESSION + OVERRIDE combination
668+
t.Run("tdigest.merge with COMPRESSION and OVERRIDE together", func(t *testing.T) {
669+
keyPrefix := "tdigest_merge_compression_override_"
670+
671+
srcKey := keyPrefix + "src"
672+
destKey := keyPrefix + "dest"
673+
674+
require.NoError(t, rdb.Do(ctx, "TDIGEST.CREATE", srcKey, "compression", "100").Err())
675+
require.NoError(t, rdb.Do(ctx, "TDIGEST.ADD", srcKey, "1", "2", "3", "4", "5").Err())
676+
677+
// Create destination key first to test OVERRIDE
678+
require.NoError(t, rdb.Do(ctx, "TDIGEST.CREATE", destKey, "compression", "50").Err())
679+
require.NoError(t, rdb.Do(ctx, "TDIGEST.ADD", destKey, "10", "20").Err())
680+
681+
// COMPRESSION + OVERRIDE should work together
682+
require.NoError(t, rdb.Do(ctx, "TDIGEST.MERGE", destKey, 1, srcKey, "COMPRESSION", "101", "OVERRIDE").Err())
683+
684+
// Verify the merge worked with new compression
685+
rsp := rdb.Do(ctx, "TDIGEST.INFO", destKey)
686+
require.NoError(t, rsp.Err())
687+
info := toTdigestInfo(t, rsp.Val())
688+
require.EqualValues(t, 101, info.Compression)
689+
require.EqualValues(t, 5, info.Observations) // src data replaced dest data due to OVERRIDE
690+
})
691+
645692
t.Run("tdigest.revrank with different arguments", func(t *testing.T) {
646693
keyPrefix := "tdigest_revrank_"
647694

‎x.py‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -162,10 +162,10 @@ def build(dir: str, jobs: Optional[int] = None, ninja: bool = False, unittest: b
162162
run(cmake, *options, verbose=True, cwd=dir)
163163

164164

165-
def fetch_deps(dir: str) -> None:
165+
def fetch_deps(dir: str, D: List[str] = []) -> None:
166166
dir = os.path.abspath(dir)
167167
with TemporaryDirectory(prefix="kvrocks-fetch-deps-") as build_dir:
168-
build(build_dir, dep_dir=dir, skip_build=True)
168+
build(build_dir, D=D, dep_dir=dir, skip_build=True)
169169

170170

171171
def get_source_files(dir: Path) -> List[str]:
@@ -418,6 +418,8 @@ def test_go(dir: str, cli_path: str, rest: List[str]) -> None:
418418
)
419419
parser_fetch_deps.add_argument('dir', metavar='DEP_DIR', nargs='?', default='build-deps',
420420
help="directory to store fetched archives of dependencies")
421+
parser_fetch_deps.add_argument('-D', action='append', metavar='key=value',
422+
help='extra CMake definitions used to determine fetched dependencies')
421423
parser_fetch_deps.set_defaults(func=fetch_deps)
422424

423425
parser_package = subparsers.add_parser(

0 commit comments

Comments
 (0)