Merge branch 'unstable' into persist_cluster-enabled_option

.github/workflows/kvrocks.yaml

@@ -127,6 +127,10 @@ jobs:
           - name: Ubuntu GCC
             os: ubuntu-20.04
             compiler: gcc
+          - name: SonarCloud with Coverage
+            os: ubuntu-22.04
+            compiler: gcc
+            sonarcloud: -DCMAKE_CXX_FLAGS=--coverage
           - name: Ubuntu Clang
             os: ubuntu-20.04
             compiler: clang
@@ -192,6 +196,8 @@ jobs:
             with_speedb: -DENABLE_SPEEDB=ON
 
     runs-on: ${{ matrix.os }}
+    env:
+      SONARCLOUD_OUTPUT_DIR: sonarcloud-data
     steps:
       - name: Setup macOS
         if: ${{ startsWith(matrix.os, 'macos') }}
@@ -222,17 +228,34 @@ jobs:
           pushd redis-6.2.7 && BUILD_TLS=yes make -j$NPROC redis-cli && mv src/redis-cli $HOME/local/bin/ && popd
 
       - uses: actions/checkout@v3
+        with:
+          fetch-depth: 0
       - uses: actions/setup-python@v4
         with:
           python-version: 3.x
       - uses: actions/setup-go@v3
         with:
           go-version-file: 'tests/gocase/go.mod'
 
+      - name: Install gcovr 5.0
+        run: pip install gcovr==5.0 # 5.1 is not supported
+        if: ${{ matrix.sonarcloud }}
+
+      - name: Install sonar-scanner and build-wrapper
+        uses: SonarSource/sonarcloud-github-c-cpp@v2
+        if: ${{ matrix.sonarcloud }}
+
       - name: Build Kvrocks
+        if: ${{ !matrix.sonarcloud }}
         run: |
-          ./x.py build -j$NPROC --unittest --compiler ${{ matrix.compiler }} ${{ matrix.without_jemalloc }} ${{ matrix.without_luajit }} \
-            ${{ matrix.with_ninja }} ${{ matrix.with_sanitizer }} ${{ matrix.with_openssl }} ${{ matrix.new_encoding }} ${{ matrix.with_speedb }} ${{ env.CMAKE_EXTRA_DEFS }}
+          ./x.py build -j$NPROC --unittest --compiler ${{ matrix.compiler }} ${{ matrix.without_jemalloc }} \
+            ${{ matrix.without_luajit }} ${{ matrix.with_ninja }} ${{ matrix.with_sanitizer }} ${{ matrix.with_openssl }} \
+            ${{ matrix.new_encoding }} ${{ matrix.with_speedb }} ${{ env.CMAKE_EXTRA_DEFS }}
+
+      - name: Build Kvrocks (SonarCloud)
+        if: ${{ matrix.sonarcloud }}
+        run: |
+          build-wrapper-linux-x86-64 --out-dir ${{ env.SONARCLOUD_OUTPUT_DIR }} ./x.py build -j$NPROC --unittest --compiler ${{ matrix.compiler }} ${{ matrix.sonarcloud }}
 
       - name: Setup Coredump
         if: ${{ startsWith(matrix.os, 'ubuntu') }}
@@ -292,6 +315,25 @@ jobs:
           path: |
             ./build/kvrocks
             ./coredumps/*
+      
+      - name: Collect coverage into one XML report
+        if: ${{ matrix.sonarcloud }}
+        run: |
+          gcovr --sonarqube > ${{ env.SONARCLOUD_OUTPUT_DIR }}/coverage.xml
+      
+      - name: Add event information
+        if: ${{ matrix.sonarcloud }}
+        env:
+          GITHUB_EVENT_JSON: ${{ toJson(github.event) }}
+        run: |
+          echo "$GITHUB_EVENT_JSON" | tee ${{ env.SONARCLOUD_OUTPUT_DIR }}/github-event.json
+      
+      - name: Upload SonarCloud data
+        if: ${{ matrix.sonarcloud }}
+        uses: actions/upload-artifact@v3
+        with:
+          name: sonarcloud-data
+          path: ${{ env.SONARCLOUD_OUTPUT_DIR }}
 
   check-docker:
     name: Check Docker image

.github/workflows/sonar.yaml

@@ -0,0 +1,84 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+name: SonarCloud Analysis
+
+on:
+  workflow_run:
+    workflows: [CI]
+    types: [completed]
+
+jobs:
+  sonarcloud:
+    name: Upload to SonarCloud
+    runs-on: ubuntu-22.04
+    if: github.event.workflow_run.conclusion == 'success' && github.repository_owner == 'apache'
+    steps:
+      - uses: actions/checkout@v3
+        with:
+          repository: ${{ github.event.workflow_run.head_repository.full_name }}
+          ref: ${{ github.event.workflow_run.head_sha }}
+          fetch-depth: 0
+      - name: Install sonar-scanner and build-wrapper
+        uses: SonarSource/sonarcloud-github-c-cpp@v2
+      - name: 'Download code coverage'
+        uses: actions/github-script@v6
+        with:
+          script: |
+            let allArtifacts = await github.rest.actions.listWorkflowRunArtifacts({
+               owner: context.repo.owner,
+               repo: context.repo.repo,
+               run_id: context.payload.workflow_run.id,
+            });
+            let matchArtifact = allArtifacts.data.artifacts.filter((artifact) => {
+              return artifact.name == "sonarcloud-data"
+            })[0];
+            let download = await github.rest.actions.downloadArtifact({
+               owner: context.repo.owner,
+               repo: context.repo.repo,
+               artifact_id: matchArtifact.id,
+               archive_format: 'zip',
+            });
+            let fs = require('fs');
+            fs.writeFileSync(`${process.env.GITHUB_WORKSPACE}/sonarcloud-data.zip`, Buffer.from(download.data));
+      - name: 'Unzip code coverage'
+        run: |
+          unzip sonarcloud-data.zip -d sonarcloud-data
+          ls -a sonarcloud-data
+
+      - uses: actions/setup-python@v4
+        with:
+          python-version: 3.x
+      - name: Configure Kvrocks
+        run: |
+          ./x.py build -j$(nproc) --compiler gcc --skip-build
+
+      - name: Run sonar-scanner
+        env:
+          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+          SONAR_TOKEN: ${{ secrets.SONARCLOUD_TOKEN }}
+        run: |
+          PR_NUMBER=$(jq -r '.number | select (.!=null)' sonarcloud-data/github-event.json)
+          echo "The PR number is $PR_NUMBER"
+
+          sonar-scanner \
+            --define sonar.cfamily.build-wrapper-output="sonarcloud-data" \
+            --define sonar.coverageReportPaths=sonarcloud-data/coverage.xml \
+            --define sonar.projectKey=apache_kvrocks \
+            --define sonar.organization=apache \
+            --define sonar.scm.revision=${{ github.event.workflow_run.head_sha }} \
+            --define sonar.pullrequest.key=$PR_NUMBER

README.md

@@ -64,6 +64,12 @@ sudo bash cmake.sh --skip-license --prefix=/usr
 # enable gcc and make in devtoolset-11
 source /opt/rh/devtoolset-11/enable
 
+# openSUSE / SUSE Linux Enterprise
+sudo zypper install -y gcc11 gcc11-c++ make wget git autoconf automake python3 curl cmake
+
+# Arch Linux
+sudo pacman -Sy --noconfirm autoconf automake python3 git wget which cmake make gcc
+
 # macOS
 brew install git cmake autoconf automake libtool openssl
 # please link openssl by force if it still cannot be found after installing

kvrocks.conf

@@ -572,6 +572,17 @@ migrate-sequence-gap 10000
 # Default: 4096MB
 rocksdb.block_cache_size 4096
 
+# Specify the type of cache used in the block cache.
+# Accept value: "lru", "hcc"
+# "lru" stands for the cache with the LRU(Least Recently Used) replacement policy.
+#
+# "hcc" stands for the Hyper Clock Cache, a lock-free cache alternative
+# that offers much improved CPU efficiency vs. LRU cache under high parallel
+# load or high contention.
+#
+# default lru
+rocksdb.block_cache_type lru
+
 # A global cache for table-level rows in RocksDB. If almost always point
 # lookups, enlarging row cache may improve read performance. Otherwise,
 # if we enlarge this value, we can lessen metadata/subkey block cache size.
@@ -838,6 +849,7 @@ rocksdb.write_options.sync no
 
 # If yes, writes will not first go to the write ahead log,
 # and the write may get lost after a crash.
+# You must keep wal enabled if you use replication.
 #
 # Default: no
 rocksdb.write_options.disable_wal no

src/cluster/slot_migrate.cc

@@ -151,11 +151,10 @@ Status SlotMigrator::CreateMigrationThread() {
 
 void SlotMigrator::loop() {
   while (true) {
-    std::unique_lock<std::mutex> ul(job_mutex_);
-    while (!isTerminated() && !migration_job_) {
-      job_cv_.wait(ul);
+    {
+      std::unique_lock<std::mutex> ul(job_mutex_);
+      job_cv_.wait(ul, [&] { return isTerminated() || migration_job_; });
     }
-    ul.unlock();
 
     if (isTerminated()) {
       clean();

src/commands/cmd_function.cc

@@ -53,18 +53,18 @@ struct CommandFunction : Commander {
         with_code = true;
       }
 
-      return lua::FunctionList(srv, libname, with_code, output);
+      return lua::FunctionList(srv, conn, libname, with_code, output);
     } else if (parser.EatEqICase("listfunc")) {
       std::string funcname;
       if (parser.EatEqICase("funcname")) {
         funcname = GET_OR_RET(parser.TakeStr());
       }
 
-      return lua::FunctionListFunc(srv, funcname, output);
+      return lua::FunctionListFunc(srv, conn, funcname, output);
     } else if (parser.EatEqICase("listlib")) {
       auto libname = GET_OR_RET(parser.TakeStr().Prefixed("expect a library name"));
 
-      return lua::FunctionListLib(srv, libname, output);
+      return lua::FunctionListLib(srv, conn, libname, output);
     } else if (parser.EatEqICase("delete")) {
       auto libname = GET_OR_RET(parser.TakeStr());
       if (!lua::FunctionIsLibExist(conn, libname)) {

src/commands/cmd_geo.cc

@@ -150,7 +150,7 @@ class CommandGeoDist : public CommandGeoBase {
     if (s.IsNotFound()) {
       *output = conn->NilString();
     } else {
-      *output = redis::BulkString(util::Float2String(GetDistanceByUnit(distance)));
+      *output = conn->Double(GetDistanceByUnit(distance));
     }
     return Status::OK();
   }
@@ -215,8 +215,7 @@ class CommandGeoPos : public Commander {
       if (iter == geo_points.end()) {
         list.emplace_back(conn->NilString());
       } else {
-        list.emplace_back(conn->MultiBulkString(
-            {util::Float2String(iter->second.longitude), util::Float2String(iter->second.latitude)}));
+        list.emplace_back(redis::Array({conn->Double(iter->second.longitude), conn->Double(iter->second.latitude)}));
       }
     }
     *output = redis::Array(list);
@@ -331,21 +330,40 @@ class CommandGeoRadius : public CommandGeoBase {
         std::vector<std::string> one;
         one.emplace_back(redis::BulkString(geo_point.member));
         if (with_dist_) {
-          one.emplace_back(redis::BulkString(util::Float2String(GetDistanceByUnit(geo_point.dist))));
+          one.emplace_back(conn->Double(GetDistanceByUnit(geo_point.dist)));
         }
         if (with_hash_) {
-          one.emplace_back(redis::BulkString(util::Float2String(geo_point.score)));
+          one.emplace_back(conn->Double(geo_point.score));
         }
         if (with_coord_) {
-          one.emplace_back(
-              conn->MultiBulkString({util::Float2String(geo_point.longitude), util::Float2String(geo_point.latitude)}));
+          one.emplace_back(redis::Array({conn->Double(geo_point.longitude), conn->Double(geo_point.latitude)}));
         }
         list.emplace_back(redis::Array(one));
       }
     }
     return redis::Array(list);
   }
 
+  static std::vector<CommandKeyRange> Range(const std::vector<std::string> &args) {
+    int store_key = 0;
+
+    // Check for the presence of the stored key in the command args.
+    for (size_t i = 6; i < args.size(); i++) {
+      // For the case when a user specifies both "store" and "storedist" options,
+      // the second key will override the first key. The behavior is kept the same
+      // as in ParseRadiusExtraOption method.
+      if ((util::ToLower(args[i]) == "store" || util::ToLower(args[i]) == "storedist") && i + 1 < args.size()) {
+        store_key = (int)i + 1;
+        i++;
+      }
+    }
+
+    if (store_key > 0) {
+      return {{1, 1, 1}, {store_key, store_key, 1}};
+    }
+    return {{1, 1, 1}};
+  }
+
  protected:
   double radius_ = 0;
   bool with_coord_ = false;
@@ -509,14 +527,13 @@ class CommandGeoSearch : public CommandGeoBase {
         std::vector<std::string> one;
         one.emplace_back(redis::BulkString(geo_point.member));
         if (with_dist_) {
-          one.emplace_back(redis::BulkString(util::Float2String(GetDistanceByUnit(geo_point.dist))));
+          one.emplace_back(conn->Double(GetDistanceByUnit(geo_point.dist)));
         }
         if (with_hash_) {
-          one.emplace_back(redis::BulkString(util::Float2String(geo_point.score)));
+          one.emplace_back(conn->Double(geo_point.score));
         }
         if (with_coord_) {
-          one.emplace_back(
-              conn->MultiBulkString({util::Float2String(geo_point.longitude), util::Float2String(geo_point.latitude)}));
+          one.emplace_back(redis::Array({conn->Double(geo_point.longitude), conn->Double(geo_point.latitude)}));
         }
         output.emplace_back(redis::Array(one));
       }
@@ -607,6 +624,8 @@ class CommandGeoSearchStore : public CommandGeoSearch {
     return Status::OK();
   }
 
+  static std::vector<CommandKeyRange> Range(const std::vector<std::string> &args) { return {{1, 1, 1}, {2, 2, 1}}; }
+
  private:
   bool store_distance_ = false;
   std::string store_key_;
@@ -649,6 +668,26 @@ class CommandGeoRadiusByMember : public CommandGeoRadius {
 
     return Status::OK();
   }
+
+  static std::vector<CommandKeyRange> Range(const std::vector<std::string> &args) {
+    int store_key = 0;
+
+    // Check for the presence of the stored key in the command args.
+    for (size_t i = 5; i < args.size(); i++) {
+      // For the case when a user specifies both "store" and "storedist" options,
+      // the second key will override the first key. The behavior is kept the same
+      // as in ParseRadiusExtraOption method.
+      if ((util::ToLower(args[i]) == "store" || util::ToLower(args[i]) == "storedist") && i + 1 < args.size()) {
+        store_key = (int)i + 1;
+        i++;
+      }
+    }
+
+    if (store_key > 0) {
+      return {{1, 1, 1}, {store_key, store_key, 1}};
+    }
+    return {{1, 1, 1}};
+  }
 };
 
 class CommandGeoRadiusReadonly : public CommandGeoRadius {
@@ -665,11 +704,12 @@ REDIS_REGISTER_COMMANDS(MakeCmdAttr<CommandGeoAdd>("geoadd", -5, "write", 1, 1,
                         MakeCmdAttr<CommandGeoDist>("geodist", -4, "read-only", 1, 1, 1),
                         MakeCmdAttr<CommandGeoHash>("geohash", -3, "read-only", 1, 1, 1),
                         MakeCmdAttr<CommandGeoPos>("geopos", -3, "read-only", 1, 1, 1),
-                        MakeCmdAttr<CommandGeoRadius>("georadius", -6, "write", 1, 1, 1),
-                        MakeCmdAttr<CommandGeoRadiusByMember>("georadiusbymember", -5, "write", 1, 1, 1),
+                        MakeCmdAttr<CommandGeoRadius>("georadius", -6, "write", CommandGeoRadius::Range),
+                        MakeCmdAttr<CommandGeoRadiusByMember>("georadiusbymember", -5, "write",
+                                                              CommandGeoRadiusByMember::Range),
                         MakeCmdAttr<CommandGeoRadiusReadonly>("georadius_ro", -6, "read-only", 1, 1, 1),
                         MakeCmdAttr<CommandGeoRadiusByMemberReadonly>("georadiusbymember_ro", -5, "read-only", 1, 1, 1),
                         MakeCmdAttr<CommandGeoSearch>("geosearch", -7, "read-only", 1, 1, 1),
-                        MakeCmdAttr<CommandGeoSearchStore>("geosearchstore", -8, "write", 1, 1, 1))
+                        MakeCmdAttr<CommandGeoSearchStore>("geosearchstore", -8, "write", CommandGeoSearchStore::Range))
 
 }  // namespace redis