Skip to content

Start data substrate when store_handler is nullptr only if data store is enabled #309

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
githubzilla merged 4 commits into from
Nov 29, 2025
Merged

Start data substrate when store_handler is nullptr only if data store is enabled #309

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ca17610
Start data substrate when store_handler is nullptr only if data store…
githubzilla Nov 28, 2025
3332909
Fix build fail caused by mismatch of log_state and store_type
githubzilla Nov 29, 2025
57bb5de
Add pre-commit
githubzilla Nov 29, 2025
ec42c04
Fix clang-format
githubzilla Nov 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,6 @@
aws
abseil-cpp

# pre-commit
.pre-commit-config.yaml

# Swap files
*.swp
*.swo
Expand Down
21 changes: 21 additions & 0 deletions .pre-commit-config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# See https://pre-commit.com/hooks.html for more hooks

repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v2.3.0
hooks:
- id: trailing-whitespace
files: \.(cpp|cxx|cc|c|h|hpp|hxx|hh)$
exclude: (^include/redis|^src/redis|^include/vector|^src/vector|^data_substrate/|^tests/|^lua|^fpconv)
- id: end-of-file-fixer
files: \.(cpp|cxx|cc|c|h|hpp|hxx|hh)$
exclude: (^include/redis|^src/redis|^include/vector|^src/vector|^data_substrate/|^tests/|^lua|^fpconv)
- id: check-added-large-files

- repo: https://github.com/pre-commit/mirrors-clang-format
rev: v18.1.8
hooks:
- id: clang-format
files: \.(cpp|cxx|cc|c|h|hpp|hxx|hh)$
exclude: (^include/redis/|^src/redis/|^include/vector/|^src/vector/|^data_substrate/|^tests/|^lua/|^fpconv/)
args: [--style=file, --fallback-style=none, --verbose]
Comment on lines +7 to +21
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 29, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Inconsistent path patterns across hooks may cause mismatches.

The exclusion patterns are inconsistent between the first two hooks and the clang-format hook:

  • Lines 9, 12: Use ^lua|^fpconv (no trailing slashes)
  • Line 20: Use ^lua/|^fpconv/ (with trailing slashes)

This inconsistency could cause the trailing-whitespace and end-of-file-fixer hooks to behave differently from clang-format when matching excluded directories. Regex patterns without trailing slashes may match files or directories with those names anywhere in the path, whereas patterns with slashes explicitly target directory boundaries.

Standardize the exclusion patterns for consistency:

- - id: trailing-whitespace
-   files: \.(cpp|cxx|cc|c|h|hpp|hxx|hh)$
-   exclude: (^include/redis|^src/redis|^include/vector|^src/vector|^data_substrate/|^tests/|^lua|^fpconv)
- - id: end-of-file-fixer
-   files: \.(cpp|cxx|cc|c|h|hpp|hxx|hh)$
-   exclude: (^include/redis|^src/redis|^include/vector|^src/vector|^data_substrate/|^tests/|^lua|^fpconv)
+ - id: trailing-whitespace
+   files: \.(cpp|cxx|cc|c|h|hpp|hxx|hh)$
+   exclude: (^include/redis/|^src/redis/|^include/vector/|^src/vector/|^data_substrate/|^tests/|^lua/|^fpconv/)
+ - id: end-of-file-fixer
+   files: \.(cpp|cxx|cc|c|h|hpp|hxx|hh)$
+   exclude: (^include/redis/|^src/redis/|^include/vector/|^src/vector/|^data_substrate/|^tests/|^lua/|^fpconv/)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- id: trailing-whitespace
files: \.(cpp|cxx|cc|c|h|hpp|hxx|hh)$
exclude: (^include/redis|^src/redis|^include/vector|^src/vector|^data_substrate/|^tests/|^lua|^fpconv)
- id: end-of-file-fixer
files: \.(cpp|cxx|cc|c|h|hpp|hxx|hh)$
exclude: (^include/redis|^src/redis|^include/vector|^src/vector|^data_substrate/|^tests/|^lua|^fpconv)
- id: check-added-large-files
- repo: https://github.com/pre-commit/mirrors-clang-format
rev: v18.1.8
hooks:
- id: clang-format
files: \.(cpp|cxx|cc|c|h|hpp|hxx|hh)$
exclude: (^include/redis/|^src/redis/|^include/vector/|^src/vector/|^data_substrate/|^tests/|^lua/|^fpconv/)
args: [--style=file, --fallback-style=none, --verbose]
- id: trailing-whitespace
files: \.(cpp|cxx|cc|c|h|hpp|hxx|hh)$
exclude: (^include/redis/|^src/redis/|^include/vector/|^src/vector/|^data_substrate/|^tests/|^lua/|^fpconv/)
- id: end-of-file-fixer
files: \.(cpp|cxx|cc|c|h|hpp|hxx|hh)$
exclude: (^include/redis/|^src/redis/|^include/vector/|^src/vector/|^data_substrate/|^tests/|^lua/|^fpconv/)
- id: check-added-large-files
- repo: https://github.com/pre-commit/mirrors-clang-format
rev: v18.1.8
hooks:
- id: clang-format
files: \.(cpp|cxx|cc|c|h|hpp|hxx|hh)$
exclude: (^include/redis/|^src/redis/|^include/vector/|^src/vector/|^data_substrate/|^tests/|^lua/|^fpconv/)
args: [--style=file, --fallback-style=none, --verbose]
🤖 Prompt for AI Agents 
In .pre-commit-config.yaml around lines 7 to 21, the exclude regexes are
inconsistent (some entries use ^lua|^fpconv without trailing slashes while
clang-format uses ^lua/|^fpconv/), which can cause different matching behavior
across hooks; standardize the exclusion patterns across all hooks by making them
the same form (for example, append trailing slashes to the patterns on lines 9
and 12 to match the clang-format style: use ^lua/ and ^fpconv/), and ensure any
other hook exclude lines are updated to the same normalized regex so all hooks
behave consistently.

5 changes: 5 additions & 0 deletions concourse/scripts/pr.ent.bash
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,11 @@ kv_store_types=("ELOQDSS_ROCKSDB_CLOUD_S3" "ROCKSDB")
for bt in "${build_types[@]}"; do
for kst in "${kv_store_types[@]}"; do
rm -rf /home/$current_user/workspace/eloqkv/eloq_data
if [ "$kst" == "ELOQDSS_ROCKSDB_CLOUD_S3" ]; then
txlog_log_state="ROCKSDB_CLOUD_S3"
elif [ "$kst" == "ROCKSDB" ]; then
txlog_log_state="ROCKSDB"
fi
run_build_ent $bt $kst

source my_env/bin/activate
Expand Down
6 changes: 3 additions & 3 deletions include/redis_command.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1231,7 +1231,7 @@ struct PublishCommand : public DirectCommand
{
PublishCommand() = default;
PublishCommand(std::string_view chan, std::string_view msg)
: chan_(chan), message_(msg){};
: chan_(chan), message_(msg) {};
void Execute(RedisServiceImpl *redis_impl,
RedisConnectionContext *ctx) override;

Expand Down Expand Up @@ -7647,12 +7647,12 @@ struct TTLCommand : public RedisCommand
{
public:
TTLCommand()
: is_pttl_(false), is_expire_time_(false), is_pexpire_time_(false){};
: is_pttl_(false), is_expire_time_(false), is_pexpire_time_(false) {};

explicit TTLCommand(bool is_pttl, bool is_expire_time, bool is_pexpire_time)
: is_pttl_(is_pttl),
is_expire_time_(is_expire_time),
is_pexpire_time_(is_pexpire_time){};
is_pexpire_time_(is_pexpire_time) {};

std::unique_ptr<TxCommand> Clone() override;

Expand Down
9 changes: 6 additions & 3 deletions src/redis_service.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
* <http://www.gnu.org/licenses/>.
*
*/
#include "redis_service.h"

#include <absl/types/span.h>
#include <bthread/mutex.h>
#include <bthread/task_group.h>
Expand Down Expand Up @@ -60,7 +62,6 @@
#include "redis_connection_context.h"
#include "redis_handler.h"
#include "redis_metrics.h"
#include "redis_service.h"
#include "redis_stats.h"
#include "redis_string_match.h"
#include "sharder.h"
Expand Down Expand Up @@ -189,8 +190,10 @@ bool RedisServiceImpl::Init(brpc::Server &brpc_server)
LOG(ERROR) << "Error: TxService is not initialized.";
return false;
}
bool enable_store =
DataSubstrate::GetGlobal()->GetCoreConfig().enable_data_store;
store_hd_ = DataSubstrate::GetGlobal()->GetStoreHandler();
if (store_hd_ == nullptr)
if (enable_store && store_hd_ == nullptr)
{
LOG(ERROR) << "Error: DataStoreHandler is not initialized.";
return false;
Expand Down Expand Up @@ -899,7 +902,7 @@ void RedisServiceImpl::RedisClusterSlots(std::vector<SlotInfo> &info)
}
}
} // end-if
} // end-for
} // end-for

if (info.size() > 1)
{
Expand Down