Fix host manager

[lokax]

Summary by CodeRabbit

• Bug Fixes

  • Ensure host-manager address and port default to defined values when not provided.
  • Consistently pass host-manager address, port, and executable path to services to avoid startup inconsistencies and duplicate initializations.

• Chores

  • Updated an internal submodule reference.

[coderabbitai]

Caution

Review failed

The pull request is closed.

Walkthrough

Eloq KV engine now creates mutable local copies for host-manager parameters, fills defaults (IP, port, binary path) when missing, and always passes the finalized hmIP/hmPort/hmBinPath when starting the tx/datastore service; global options ensure hostManagerAddr is set to ("", 0) if no HM IP/port provided.

Changes

Cohort / File(s)	Change Summary
KV engine — host-manager parameter defaulting & startup src/mongo/db/modules/eloq/src/eloq_kv_engine.cpp	Converts hmIP and hmBinPath from const std::string& to mutable std::string copies; when forking and hmIP is empty sets it to local host, when hmPort is 0 sets it to local port+4, derives hmBinPath when empty, and always passes references to the finalized hmIP/hmPort/hmBinPath when starting the tx/datastore service (removes conditional nullptr passing).
Global options — ensure hostManagerAddr assigned src/mongo/db/modules/eloq/src/eloq_global_options.cpp	Adds an else branch in EloqGlobalOptions::store to initialize hostManagerAddr to an empty host and port 0 when neither HM IP nor HM port are provided.
Submodule pointer update src/mongo/db/modules/eloq/tx_service/...	Updates the tx_service submodule commit reference (hash changed); no code changes in this diff.

Sequence Diagram(s)

sequenceDiagram
    participant C as EloqKVEngine
    participant R as LocalAddressResolver
    participant S as Tx/DataStoreService
    participant Fork as ForkDecision
    participant HM as HostManagerProcess

    rect rgba(220,235,255,0.45)
    C->>R: resolve local host/port
    R-->>C: localHost, localPort
    note right of C: create mutable copies: hmIP, hmBinPath
    C->>C: if hmIP=="" -> hmIP = localHost
    C->>C: if hmPort==0 -> hmPort = localPort + 4
    C->>C: if hmBinPath=="" -> derive binary path
    end

    rect rgba(230,255,230,0.35)
    C->>S: startTxService(&hmIP, &hmPort, &hmBinPath)
    S-->>C: service started
    end

    C->>Fork: evaluate FORK_HM_PROCESS
    alt fork enabled
        Fork->>HM: fork & exec host manager with hmIP,hmPort,hmBinPath
        HM-->>C: PID/status
    else no fork
        Fork-->>C: skip host-manager
    end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Review focus:
  • Ensure defaulting happens before any consumer uses the values and lifetimes of passed references are safe.
  • Validate binary-path derivation and platform edge cases.
  • Confirm the tx/datastore service API expects non-null pointers and behaves with the provided values.

Possibly related PRs

• Update submodules #227 — Updates the eloq tx_service submodule reference (same area as this PR).
• Fix host manager  #283 — Modifies the same functions in eloq_kv_engine.cpp and the same defaulting logic in eloq_global_options.cpp.
• update submodules #198 — Also updates the tx_service submodule pointer (related submodule commit change).

Suggested labels

trigger-ci

Suggested reviewers

• lzxddz

Poem

🐰 I hopped through lines and fixed the trail,

Filled empty hosts where defaults would fail.
I set the port, the path, the start,
Passed tidy args to every part.
A rabbit's tweak — small, sound, and hale.

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	The title 'Fix host manager' is vague and generic, lacking specific detail about what aspect of host manager is being fixed or what the actual change accomplishes.	Revise the title to be more specific about the fix, such as 'Initialize host manager parameters with defaults when not provided' or 'Fix host manager parameter initialization in forked mode'.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 754bbcd and e895af3.

📒 Files selected for processing (3)

• src/mongo/db/modules/eloq/src/eloq_global_options.cpp (1 hunks)
• src/mongo/db/modules/eloq/src/eloq_kv_engine.cpp (2 hunks)
• src/mongo/db/modules/eloq/tx_service (1 hunks)

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 31a3dd2 and cdd9787.

📒 Files selected for processing (1)

• src/mongo/db/modules/eloq/src/eloq_kv_engine.cpp (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

src/mongo/db/modules/eloq/src/eloq_kv_engine.cpp (1)

src/mongo/db/server_options.h (1)

• bootstrap (86-269)

🔇 Additional comments (1)

src/mongo/db/modules/eloq/src/eloq_kv_engine.cpp (1)

324-324: Early initialization of initDataStoreService() is safe and has no ordering dependencies.

Verification confirms that all variables accessed by initDataStoreService() (eloqGlobalOptions members, serverGlobalParams.bootstrap, _catalogFactory, _dbPath) are initialized before line 324. The method does not depend on any state initialized after it (ngConfigs, forkHostManager, node IDs, host manager addresses), so the early call introduces no ordering issues.

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 03dec79 and 11d6516.

📒 Files selected for processing (2)

• src/mongo/db/modules/eloq/src/eloq_global_options.cpp (1 hunks)
• src/mongo/db/modules/eloq/src/eloq_kv_engine.cpp (2 hunks)

🔇 Additional comments (3)

src/mongo/db/modules/eloq/src/eloq_global_options.cpp (1)

716-718: Good defensive initialization.

Ensuring hostManagerAddr is always assigned prevents undefined behavior downstream. The ("", 0) sentinel value clearly signals "no host manager configured" for consumption by the KV engine initialization logic.

src/mongo/db/modules/eloq/src/eloq_kv_engine.cpp (2)

320-323: LGTM: Const reference issue resolved.

Changing from const references to mutable copies correctly addresses the past review feedback and enables the defaulting logic applied at lines 330-336.

---

567-569: Consistent parameter passing approach.

Always passing host-manager addresses (rather than conditional nullptrs) aligns with the initialization guarantees from eloq_global_options.cpp. This approach simplifies the Start() interface by eliminating null-pointer checks.