Skip to content

Read core_number from data substrate #421

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
liunyl merged 1 commit into from
Jan 12, 2026
Merged

Read core_number from data substrate #421

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
63 changes: 14 additions & 49 deletions src/mongo/db/modules/eloq/src/eloq_global_options.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#define MONGO_LOG_DEFAULT_COMPONENT ::mongo::logger::LogComponent::kStorage

#include <cstdint>
#include <filesystem>
#include <gflags/gflags.h>
#include <limits>
#include <string>
Expand All @@ -26,19 +27,14 @@

#include "mongo/base/status.h"
#include "mongo/db/modules/eloq/data_substrate/core/include/data_substrate.h"
#include "mongo/db/modules/eloq/data_substrate/tx_service/raft_host_manager/include/INIReader.h"
#include "mongo/db/modules/eloq/data_substrate/core/include/glog_error_logging.h"
#include "mongo/db/modules/eloq/src/eloq_global_options.h"
#include "mongo/db/server_options.h"
#include "mongo/util/log.h"
#include "mongo/util/options_parser/constraints.h"
#include "mongo/util/options_parser/option_description.h"

DECLARE_string(data_substrate_config);
DECLARE_int32(core_number);
DECLARE_bool(bootstrap);
DECLARE_bool(enable_data_store);
const auto NUM_VCPU = std::thread::hardware_concurrency();

DEFINE_string(data_substrate_config, "", "Data Substrate Configuration");
namespace mongo {
EloqGlobalOptions eloqGlobalOptions;

Expand Down Expand Up @@ -73,51 +69,20 @@ Status EloqGlobalOptions::store(const moe::Environment& params,
str::stream() << s << " is not a valid CcProtocol"};
}
}

// read data substrate config file for options necessary
// for eloqdoc storage engine
std::string& data_substrate_config = FLAGS_data_substrate_config;
INIReader ds_config_reader(data_substrate_config);
if (!data_substrate_config.empty() && ds_config_reader.ParseError() != 0) {
MONGO_LOG(1) << "Failed to parse config file: " << data_substrate_config;
return Status{ErrorCodes::InvalidOptions,
str::stream() << "Failed to parse config file: " << data_substrate_config};
MONGO_LOG(1) << "serverGlobalParams.logpath: " << serverGlobalParams.logpath;
std::filesystem::path systemLogPath(serverGlobalParams.logpath);
if (systemLogPath.has_parent_path()) {
static std::filesystem::path logdir = systemLogPath.parent_path();
GFLAGS_NAMESPACE::SetCommandLineOption("log_dir", logdir.c_str());
}
const char* tmp[] = {"eloqdb", nullptr};
char** dummy_argv = const_cast<char**>(tmp);
InitGoogleLogging(dummy_argv);

bool enable_data_store = !CheckCommandLineFlagIsDefault("enable_data_store")
? FLAGS_enable_data_store
: ds_config_reader.GetBoolean("local", "enable_data_store", FLAGS_enable_data_store);

bool bootstrap = !CheckCommandLineFlagIsDefault("bootstrap")
? FLAGS_bootstrap
: ds_config_reader.GetBoolean("local", "bootstrap", FLAGS_bootstrap);
serverGlobalParams.bootstrap = bootstrap;
DataSubstrate::Instance().Init(FLAGS_data_substrate_config);
serverGlobalParams.bootstrap = DataSubstrate::Instance().GetCoreConfig().bootstrap;
MONGO_LOG(1) << "serverGlobalParams.bootstrap: " << serverGlobalParams.bootstrap;

const char* field_core = "core_number";
uint32_t core_num = FLAGS_core_number;
if (CheckCommandLineFlagIsDefault(field_core)) {
if (ds_config_reader.HasValue("local", field_core)) {
core_num = ds_config_reader.GetInteger("local", field_core, 0);
assert(core_num);
} else {
if (!NUM_VCPU) {
MONGO_LOG(1) << "config is missing: " << field_core;
return Status{ErrorCodes::InvalidOptions,
str::stream() << "config is missing: " << field_core};
}
const uint min = 1;
if (enable_data_store) {
core_num = std::max(min, (NUM_VCPU * 3) / 5);
MONGO_LOG(1) << "give cpus to checkpointer " << core_num;
} else {
core_num = std::max(min, (NUM_VCPU * 7) / 10);
}
MONGO_LOG(1) << "config is automatically set: " << field_core << "=" << core_num
<< ", vcpu=" << NUM_VCPU;
}
}
serverGlobalParams.reservedThreadNum = core_num;
serverGlobalParams.reservedThreadNum = DataSubstrate::Instance().GetCoreConfig().core_num;
Comment on lines +82 to +85
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Jan 12, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's examine the file and the specific lines mentioned
cat -n src/mongo/db/modules/eloq/src/eloq_global_options.cpp | head -100 | tail -30

Repository: eloqdata/eloqdoc

Length of output: 1607

🏁 Script executed:

# Search for DataSubstrate class definition and Init method
rg -n "class DataSubstrate" --type cpp

Repository: eloqdata/eloqdoc

Length of output: 42

🏁 Script executed:

# Find where DataSubstrate::Init is defined
rg -n "Init\(" --type cpp -A 5 | grep -A 5 "DataSubstrate"

Repository: eloqdata/eloqdoc

Length of output: 1066

🏁 Script executed:

# Look for DataSubstrate::Instance().Init() usage patterns in the codebase
rg -n "DataSubstrate::Instance\(\)\.Init" --type cpp -B 2 -A 3

Repository: eloqdata/eloqdoc

Length of output: 771

🏁 Script executed:

# Check the complete function that contains lines 82-85
sed -n '70,95p' src/mongo/db/modules/eloq/src/eloq_global_options.cpp

Repository: eloqdata/eloqdoc

Length of output: 1059

🏁 Script executed:

# Find DataSubstrate class definition
rg -n "class DataSubstrate" --type cpp -A 20

Repository: eloqdata/eloqdoc

Length of output: 42

🏁 Script executed:

# Search for Init method definition in DataSubstrate
rg -n "DataSubstrate.*Init\(" --type cpp -B 3 -A 10

Repository: eloqdata/eloqdoc

Length of output: 1446

🏁 Script executed:

# Check for DataSubstrate.h header files
find . -name "*data_substrate*" -type f

Repository: eloqdata/eloqdoc

Length of output: 444

🏁 Script executed:

# Look for Init method with signature
ast-grep --pattern 'Init($$$)'

Repository: eloqdata/eloqdoc

Length of output: 1461

🏁 Script executed:

# Search for files containing DataSubstrate
fd ".*" --type f | xargs rg -l "class DataSubstrate" 2>/dev/null

Repository: eloqdata/eloqdoc

Length of output: 44

🏁 Script executed:

# Search in the eloq directory specifically
fd ".*" src/mongo/db/modules/eloq --type f | xargs rg -l "DataSubstrate" 2>/dev/null | head -20

Repository: eloqdata/eloqdoc

Length of output: 319

🏁 Script executed:

# Look for DataSubstrate header file
find src -name "*.h" -o -name "*.hpp" | xargs rg "class DataSubstrate" 2>/dev/null

Repository: eloqdata/eloqdoc

Length of output: 44

🏁 Script executed:

# Check what files are in the eloq directory
find src/mongo/db/modules/eloq -name "*.h" -o -name "*.cpp" | head -30

Repository: eloqdata/eloqdoc

Length of output: 1433

🏁 Script executed:

# Search all cpp and h files for DataSubstrate class definition
rg "class DataSubstrate" --type cpp --type h -A 30

Repository: eloqdata/eloqdoc

Length of output: 42

🏁 Script executed:

# Search for DataSubstrate::Instance in includes or other patterns
rg "DataSubstrate" --type cpp --type h | head -40

Repository: eloqdata/eloqdoc

Length of output: 1489

🏁 Script executed:

# Check the includes in eloq_global_options.cpp
head -50 src/mongo/db/modules/eloq/src/eloq_global_options.cpp

Repository: eloqdata/eloqdoc

Length of output: 1970

🏁 Script executed:

# Find the data_substrate.h header file
find . -name "data_substrate.h" -type f

Repository: eloqdata/eloqdoc

Length of output: 42

🏁 Script executed:

# Check if it's in a third-party or external directory
find . -path "*data_substrate*" -name "*.h" -type f

Repository: eloqdata/eloqdoc

Length of output: 42

🏁 Script executed:

# Look for the Init method signature in the header
grep -r "Init(" src/mongo/db/modules/eloq/data_substrate --include="*.h" -A 5 2>/dev/null | head -40

Repository: eloqdata/eloqdoc

Length of output: 42

🏁 Script executed:

# Check what's in the data_substrate directory
ls -la src/mongo/db/modules/eloq/data_substrate/ 2>/dev/null || echo "Directory doesn't exist or is empty"

Repository: eloqdata/eloqdoc

Length of output: 168

🏁 Script executed:

# Check if data_substrate is a git submodule or external dependency
cat .gitmodules 2>/dev/null | grep data_substrate || echo "Not found in gitmodules"

Repository: eloqdata/eloqdoc

Length of output: 163

🏁 Script executed:

# Check all includes in eloq_global_options.cpp
grep -n "#include" src/mongo/db/modules/eloq/src/eloq_global_options.cpp

Repository: eloqdata/eloqdoc

Length of output: 663

🏁 Script executed:

# Search for where Init return type might be documented
rg "void.*Init\|Status.*Init\|bool.*Init" src/mongo/db/modules/eloq --type cpp --type h -A 3

Repository: eloqdata/eloqdoc

Length of output: 42

🏁 Script executed:

# Check if there's a return value check pattern elsewhere in similar initialization code
rg "Instance\(\)\.Init" src/mongo/db/modules/eloq --type cpp -B 2 -A 5

Repository: eloqdata/eloqdoc

Length of output: 980

🏁 Script executed:

# Check if there are any comments or documentation about Init() in the eloq files
rg -n "Init" src/mongo/db/modules/eloq/src/eloq_global_options.cpp -B 3 -A 3

Repository: eloqdata/eloqdoc

Length of output: 556

🏁 Script executed:

# Check if the include path gives us any clues
grep -n "data_substrate.h" src/mongo/db/modules/eloq/src/eloq_global_options.cpp

Repository: eloqdata/eloqdoc

Length of output: 140

🏁 Script executed:

# Look at eloq_kv_engine.cpp to see if there's any Init() error handling elsewhere
grep -n "Init\|Shutdown\|EnableEngine" src/mongo/db/modules/eloq/src/eloq_kv_engine.cpp -B 2 -A 3

Repository: eloqdata/eloqdoc

Length of output: 1299

🏁 Script executed:

# Check for Status return patterns in eloq_global_options.cpp
grep -n "Status" src/mongo/db/modules/eloq/src/eloq_global_options.cpp | head -20

Repository: eloqdata/eloqdoc

Length of output: 275

🏁 Script executed:

# Check if there's any documentation or comments about error handling expectations
grep -rn "error\|Error\|fail\|Fail" src/mongo/db/modules/eloq/src/eloq_global_options.cpp

Repository: eloqdata/eloqdoc

Length of output: 201

🏁 Script executed:

# Look at the function signature to understand context
sed -n '55,90p' src/mongo/db/modules/eloq/src/eloq_global_options.cpp

Repository: eloqdata/eloqdoc

Length of output: 1793

🏁 Script executed:

# Check if there are try-catch blocks anywhere in the function
grep -n "try\|catch" src/mongo/db/modules/eloq/src/eloq_global_options.cpp

Repository: eloqdata/eloqdoc

Length of output: 42

Add error handling for DataSubstrate initialization.

The DataSubstrate::Instance().Init() call on line 82 does not check for failure, yet the function returns Status and demonstrates error handling elsewhere (line 68). If initialization fails, the function still returns Status::OK() and subsequent GetCoreConfig() calls may operate on uninitialized state, allowing the server to start with invalid configuration.

Suggested fix: Add error handling 
-    DataSubstrate::Instance().Init(FLAGS_data_substrate_config);
+    auto init_status = DataSubstrate::Instance().Init(FLAGS_data_substrate_config);
+    if (!init_status) {  // Adjust based on actual return type
+        return Status{ErrorCodes::InternalError,
+                      str::stream() << "Failed to initialize DataSubstrate"};
+    }

Note: The exact error handling pattern depends on how Init() signals failure. Refer to the DataSubstrate header for the correct return type and error condition check.

📝 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
DataSubstrate::Instance().Init(FLAGS_data_substrate_config);
serverGlobalParams.bootstrap = DataSubstrate::Instance().GetCoreConfig().bootstrap;
MONGO_LOG(1) << "serverGlobalParams.bootstrap: " << serverGlobalParams.bootstrap;
const char* field_core = "core_number";
uint32_t core_num = FLAGS_core_number;
if (CheckCommandLineFlagIsDefault(field_core)) {
if (ds_config_reader.HasValue("local", field_core)) {
core_num = ds_config_reader.GetInteger("local", field_core, 0);
assert(core_num);
} else {
if (!NUM_VCPU) {
MONGO_LOG(1) << "config is missing: " << field_core;
return Status{ErrorCodes::InvalidOptions,
str::stream() << "config is missing: " << field_core};
}
const uint min = 1;
if (enable_data_store) {
core_num = std::max(min, (NUM_VCPU * 3) / 5);
MONGO_LOG(1) << "give cpus to checkpointer " << core_num;
} else {
core_num = std::max(min, (NUM_VCPU * 7) / 10);
}
MONGO_LOG(1) << "config is automatically set: " << field_core << "=" << core_num
<< ", vcpu=" << NUM_VCPU;
}
}
serverGlobalParams.reservedThreadNum = core_num;
serverGlobalParams.reservedThreadNum = DataSubstrate::Instance().GetCoreConfig().core_num;
auto init_status = DataSubstrate::Instance().Init(FLAGS_data_substrate_config);
if (!init_status) { // Adjust based on actual return type
return Status{ErrorCodes::InternalError,
str::stream() << "Failed to initialize DataSubstrate"};
}
serverGlobalParams.bootstrap = DataSubstrate::Instance().GetCoreConfig().bootstrap;
MONGO_LOG(1) << "serverGlobalParams.bootstrap: " << serverGlobalParams.bootstrap;
serverGlobalParams.reservedThreadNum = DataSubstrate::Instance().GetCoreConfig().core_num;
🤖 Prompt for AI Agents 
In @src/mongo/db/modules/eloq/src/eloq_global_options.cpp around lines 82 - 85,
DataSubstrate::Instance().Init(...) is called without checking its return value;
modify the code around
DataSubstrate::Instance().Init(FLAGS_data_substrate_config) to capture its
Status/return value, check for non-OK/failure, and on error log an explanatory
message and return that non-OK Status (or otherwise propagate the error) instead
of proceeding to call GetCoreConfig(); ensure subsequent uses like
DataSubstrate::Instance().GetCoreConfig(), serverGlobalParams.bootstrap and
serverGlobalParams.reservedThreadNum only run when Init succeeded.

MONGO_LOG(1) << "serverGlobalParams.reservedThreadNum: "
<< serverGlobalParams.reservedThreadNum;

Expand Down
15 changes: 0 additions & 15 deletions src/mongo/db/modules/eloq/src/eloq_kv_engine.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,9 +130,6 @@ Eloq::MongoCatalogFactory catalogFactory;

mongo::MongoSystemHandler mongoSystemHandler;

// data substrate config
DEFINE_string(data_substrate_config, "", "Data Substrate Configuration");

namespace mongo {

extern std::function<std::pair<std::function<void()>, std::function<void(int16_t)>>(int16_t)>
Expand All @@ -151,18 +148,6 @@ EloqKVEngine::EloqKVEngine(const std::string& path) : _dbPath(path) {
log() << "Starting Eloq storage engine. dbPath: " << path;
log() << "Standalone mode: Initializing data substrate...";

std::filesystem::path systemLogPath(serverGlobalParams.logpath);
if (systemLogPath.has_parent_path()) {
static std::filesystem::path logdir = systemLogPath.parent_path();
GFLAGS_NAMESPACE::SetCommandLineOption("log_dir", logdir.c_str());
}
const char* tmp[] = {"eloqdb", nullptr};
char** dummy_argv = const_cast<char**>(tmp);
InitGoogleLogging(dummy_argv);


DataSubstrate::Instance().Init(FLAGS_data_substrate_config);

auto& ds = DataSubstrate::Instance();

DataSubstrate::Instance().EnableEngine(txservice::TableEngine::EloqDoc);
Expand Down