Convert MongoDB from standalone to replica set#4184
Conversation
Co-authored-by: imnasnainaec <6411521+imnasnainaec@users.noreply.github.com>
Co-authored-by: imnasnainaec <6411521+imnasnainaec@users.noreply.github.com>
This comment was marked as resolved.
This comment was marked as resolved.
Co-authored-by: imnasnainaec <6411521+imnasnainaec@users.noreply.github.com>
github-actions
Bot
added
documentation
This comment was marked as resolved.
This comment was marked as resolved.
Co-authored-by: imnasnainaec <6411521+imnasnainaec@users.noreply.github.com>
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as outdated.
This comment was marked as outdated.
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughChanges convert the MongoDB deployment from a standalone daemon to a single-node replica set: add replica-set init scripts, start/management scripts for local development, Helm/container lifecycle hooks and env, update connection strings, and adjust related tooling/config files. Changes
Sequence Diagram(s)sequenceDiagram
participant K8s as "Kubernetes Pod"
participant Container as "DB Container\n(entrypoint + postStart)"
participant Mongod as "mongod"
participant InitJS as "00-replica-set.js"
participant App as "Application / Client"
K8s->>Container: start pod -> run container
Container->>Mongod: launch mongod (--replSet rs0)
Container->>Container: postStart: wait for mongod ready
Container->>InitJS: execute /opt/thecombine/00-replica-set.js
InitJS->>Mongod: connect via mongosh, run rs.initiate / rs.reconfig
Mongod-->>InitJS: PRIMARY confirmed
App->>Mongod: connect using mongodb://.../?replicaSet=rs0
Mongod-->>App: accept connections (replica-set primary)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
deploy/helm/thecombine/charts/database/templates/database.yaml (1)
49-54: Consider parameterizing replica-set values in chart config.
rs0anddatabase:27017are hardcoded here. Moving them to Helm values reduces drift risk across deployment, scripts, and connection strings.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@deploy/helm/thecombine/charts/database/templates/database.yaml` around lines 49 - 54, The template hardcodes the replica set name and host/port; change database/templates/database.yaml to use Helm values instead of "rs0" and "database:27017" by replacing the args entry ("--replSet", "rs0") and the env var MONGO_INITDB_REPLICA_HOST value with templated values (e.g., .Values.database.replicaSetName and .Values.database.hostPort or similar), and add corresponding defaults in values.yaml (e.g., database.replicaSetName: "rs0", database.hostPort: "database:27017"); ensure the template uses proper Helm quoting/template syntax so the rendered args and env.value are valid.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.vscode/tasks.json:
- Around line 30-38: The "run-mongo" VSCode task currently invokes "mongod"
directly with args ("--dbpath", "${workspaceFolder}/mongo_database",
"--replSet", "rs0") which enables replica-set mode but does not run
rs.initiate(), causing first-run connection failures; update the "run-mongo"
task to run the existing initialization script instead (invoke "npm run
database" or the equivalent npm script used to start and initialize the DB) so
the startup sequence performs replica set initialization automatically; keep the
task label "run-mongo" and replace the "command":"mongod" invocation with a
command that runs the npm script (ensuring the task type and args are adjusted
to execute the npm script).
In `@scripts/startDatabase.js`:
- Around line 51-66: The startup currently logs readiness/init failures and
drops mongod exit codes, allowing the process to succeed incorrectly; update the
logic around waitForMongo() and initReplicaSet() (functions referenced) to treat
failures as fatal (throw or return falsy should cause process.exit with a
non-zero code), ensure initReplicaSet() errors are awaited and checked (or
caught and cause process.exit(1)), and replace the final Promise that resolves
the mongod "close" event with logic that reads the child process exit code and
calls process.exit(code || 1) so any non-zero or unexpected termination results
in a non-zero exit; also ensure the SIGINT/SIGTERM handlers forward termination
(stop/kill mongod) but do not swallow failure—propagate error/exit accordingly.
---
Nitpick comments:
In `@deploy/helm/thecombine/charts/database/templates/database.yaml`:
- Around line 49-54: The template hardcodes the replica set name and host/port;
change database/templates/database.yaml to use Helm values instead of "rs0" and
"database:27017" by replacing the args entry ("--replSet", "rs0") and the env
var MONGO_INITDB_REPLICA_HOST value with templated values (e.g.,
.Values.database.replicaSetName and .Values.database.hostPort or similar), and
add corresponding defaults in values.yaml (e.g., database.replicaSetName: "rs0",
database.hostPort: "database:27017"); ensure the template uses proper Helm
quoting/template syntax so the rendered args and env.value are valid.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
.gitignore.vscode/tasks.jsonBackend/appsettings.jsonREADME.mddatabase/init/00-replica-set.jsdeploy/helm/thecombine/charts/database/templates/database.yamlpackage.jsonscripts/setupMongo.jsscripts/startDatabase.js
💤 Files with no reviewable changes (1)
- scripts/setupMongo.js
This comment was marked as resolved.
This comment was marked as resolved.
Co-authored-by: imnasnainaec <6411521+imnasnainaec@users.noreply.github.com>
Fixed in d0942b0. The |
|
@copilot In deploy/helm/thecombine/charts/database/templates/database.yaml, the postStart lifecycle hook (lines 52-70) needs to be updated so that when rs.status() succeeds (replica set already initialized), it also checks whether the current member host matches the value of MONGO_INITDB_REPLICA_HOST. If they differ (which happens after a pod restart with a new IP), it should reconfigure the replica set member to use the new host. The mongosh --eval script on line 63 should be changed from: to something like: This ensures the replica set member address stays in sync with the current pod IP after restarts. |
…d restart Co-authored-by: imnasnainaec <6411521+imnasnainaec@users.noreply.github.com>
Fixed in 369677c. The postStart |
|
|
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as outdated.
This comment was marked as outdated.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
scripts/startDatabase.js (1)
35-39:⚠️ Potential issue | 🟠 MajorStill hard-killing
mongodon Ctrl+C / SIGTERM.Line 37 and Lines 61-63 still funnel shutdown through
forceExit(), and Lines 43-56 immediately sendSIGKILLbeforeprocess.exit(). That leaves local stops as unclean MongoDB shutdowns and bypasses the childexitpropagation in Lines 150-159 on signal-driven exits.Also applies to: 43-56, 61-64, 150-159
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/startDatabase.js` around lines 35 - 39, The current Ctrl+C / SIGTERM handlers call forceExit() and send SIGKILL immediately which causes unclean mongod shutdowns and bypasses the child exit propagation; change the keypress and signal handlers (the process.stdin.on("keypress") handler and SIGINT/SIGTERM listeners) to attempt a graceful shutdown by sending SIGTERM to the mongod child process (use the existing child process variable and its .kill('SIGTERM') method), then wait for the child's 'exit' event (or a short timeout fallback) before calling forceExit() as a last resort; remove any immediate SIGKILL/send of SIGKILL prior to waiting so the child exit propagation code (the child.on('exit') logic) runs on signal-driven exits.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@deploy/helm/thecombine/charts/database/templates/database.yaml`:
- Around line 69-72: The postStart hook uses the construct `if ! mongosh --quiet
--host 127.0.0.1 /opt/thecombine/00-replica-set.js; then exit $?; fi` which ends
up using the exit status of the `!` operator instead of the original `mongosh`
exit code; update the hook around the `mongosh` invocation (the postStart block
calling 00-replica-set.js) so that you capture the actual exit status of
`mongosh` and propagate it as the container exit (e.g., run `mongosh` and
store/inspect its exit code, then `exit` with that code when it fails) to ensure
a failing replica set check prevents the pod from starting.
In `@scripts/startDatabase.js`:
- Around line 115-124: initReplicaSet currently returns as soon as rs.initiate
is accepted but doesn't wait for the node to become PRIMARY; add a new
waitForPrimary() that polls mongosh evaluating db.hello().isWritablePrimary with
the existing retry pattern (use maxAttempts and retryIntervalSeconds like
waitForMongo()), make initReplicaSet remain responsible for initiating the set
and in main() call waitForPrimary() after initReplicaSet() succeeds so the
startup waits until the replica set member is elected PRIMARY before proceeding.
---
Duplicate comments:
In `@scripts/startDatabase.js`:
- Around line 35-39: The current Ctrl+C / SIGTERM handlers call forceExit() and
send SIGKILL immediately which causes unclean mongod shutdowns and bypasses the
child exit propagation; change the keypress and signal handlers (the
process.stdin.on("keypress") handler and SIGINT/SIGTERM listeners) to attempt a
graceful shutdown by sending SIGTERM to the mongod child process (use the
existing child process variable and its .kill('SIGTERM') method), then wait for
the child's 'exit' event (or a short timeout fallback) before calling
forceExit() as a last resort; remove any immediate SIGKILL/send of SIGKILL prior
to waiting so the child exit propagation code (the child.on('exit') logic) runs
on signal-driven exits.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f52f0ac3-f410-465b-b3a7-95cecbd87a08
📒 Files selected for processing (7)
.gitignoredatabase/Dockerfiledatabase/init/00-replica-set.jsdeploy/helm/thecombine/charts/database/templates/database.yamldocs/deploy/README.mdpackage.jsonscripts/startDatabase.js
🚧 Files skipped from review as they are similar to previous changes (1)
- .gitignore
| async function initReplicaSet() { | ||
| try { | ||
| const result = runMongosh( | ||
| [ | ||
| "--eval", | ||
| `try { rs.status() } catch { rs.initiate({ _id: '${replSetName}', members: [{ _id: 0, host: 'localhost:27017' }] }) }`, | ||
| ], | ||
| { stdio: "inherit" } | ||
| ); | ||
| return result.status === 0; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "[scripts/startDatabase.js]"
sed -n '115,168p' scripts/startDatabase.js
echo
echo "[database/init/00-replica-set.js]"
sed -n '37,65p' database/init/00-replica-set.jsRepository: sillsdev/TheCombine
Length of output: 2181
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Full context of scripts/startDatabase.js ==="
wc -l scripts/startDatabase.js
echo
echo "=== Lines 1-50 (variable definitions) ==="
sed -n '1,50p' scripts/startDatabase.js
echo
echo "=== Lines 150-170 (checking the 'Also applies to' lines) ==="
sed -n '150,170p' scripts/startDatabase.js
echo
echo "=== Searching for maxAttempts and retryIntervalSeconds ==="
rg "maxAttempts|retryIntervalSeconds" scripts/startDatabase.jsRepository: sillsdev/TheCombine
Length of output: 2202
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== waitForMongo function (to see the retry pattern) ==="
sed -n '65,110p' scripts/startDatabase.js
echo
echo "=== runMongosh function (to verify how it's called) ==="
rg -A 5 "function runMongosh" scripts/startDatabase.js
echo
echo "=== Check getErrorMessage function ==="
rg -A 3 "function getErrorMessage" scripts/startDatabase.jsRepository: sillsdev/TheCombine
Length of output: 1708
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Checking the complete main() function (lines 130-174) ==="
sed -n '130,174p' scripts/startDatabase.js
echo
echo "=== Verify package.json line 30 to confirm parallel startup ==="
sed -n '25,35p' package.jsonRepository: sillsdev/TheCombine
Length of output: 2118
Add PRIMARY readiness check after replica set initialization.
initReplicaSet() returns success as soon as rs.initiate() is accepted, but the replica set member is not usable until it elects itself PRIMARY. Since package.json starts backend and database in parallel (line 30: run-p), the startup can race this window. The Kubernetes bootstrap in database/init/00-replica-set.js already implements the correct pattern with db.hello().isWritablePrimary; apply the same here.
Add a waitForPrimary() function that polls for db.hello().isWritablePrimary with the same retry pattern as waitForMongo(), then call it in main() after initReplicaSet() succeeds. Required variables (maxAttempts, retryIntervalSeconds) are already defined.
Suggested fix
async function initReplicaSet() {
try {
const result = runMongosh(
[
"--eval",
`try { rs.status() } catch { rs.initiate({ _id: '${replSetName}', members: [{ _id: 0, host: 'localhost:27017' }] }) }`,
],
{ stdio: "inherit" }
);
return result.status === 0;
} catch (error) {
console.error(`Replica set init failed: ${getErrorMessage(error)}`);
return false;
}
}
+
+async function waitForPrimary() {
+ for (let i = 0; i < maxAttempts; i++) {
+ try {
+ const result = runMongosh([
+ "--eval",
+ "quit(db.hello().isWritablePrimary ? 0 : 1)",
+ ]);
+ if (result.status === 0) {
+ return true;
+ }
+ } catch (error) {
+ console.warn(`MongoDB primary check failed: ${getErrorMessage(error)}`);
+ }
+
+ await new Promise((res) => setTimeout(res, retryIntervalSeconds * 1000));
+ }
+
+ return false;
+}
async function main() {
setUpInterruptHandling();
@@ -161,6 +181,10 @@ async function main() {
if (!(await initReplicaSet())) {
console.error("Replica set initialization failed");
forceExit(1);
}
+ if (!(await waitForPrimary())) {
+ console.error("Replica set did not become PRIMARY in time");
+ forceExit(1);
+ }
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/startDatabase.js` around lines 115 - 124, initReplicaSet currently
returns as soon as rs.initiate is accepted but doesn't wait for the node to
become PRIMARY; add a new waitForPrimary() that polls mongosh evaluating
db.hello().isWritablePrimary with the existing retry pattern (use maxAttempts
and retryIntervalSeconds like waitForMongo()), make initReplicaSet remain
responsible for initiating the set and in main() call waitForPrimary() after
initReplicaSet() succeeds so the startup waits until the replica set member is
elected PRIMARY before proceeding.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Search for waitForPrimary in startDatabase.js ==="
rg -n "waitForPrimary" scripts/startDatabase.js || echo "NOT FOUND"
echo
echo "=== Full file content ==="
cat scripts/startDatabase.js
Length of output: 4305
@imnasnainaec, not yet fixed. The waitForPrimary() function is absent from scripts/startDatabase.js, and main() still returns immediately after initReplicaSet() without polling for PRIMARY election. The race condition with run-p (parallel backend + database startup) remains.
Here's the change needed — it slots cleanly between initReplicaSet and main:
/** Wait until the replica set member becomes PRIMARY. */
async function waitForPrimary() {
for (let i = 0; i < maxAttempts; i++) {
try {
const result = runMongosh([
"--eval",
"quit(db.hello().isWritablePrimary ? 0 : 1)",
]);
if (result.status === 0) {
return true;
}
} catch (error) {
console.warn(`MongoDB primary check failed: ${getErrorMessage(error)}`);
}
if (i < maxAttempts - 1) {
await new Promise((res) => setTimeout(res, retryIntervalSeconds * 1000));
}
}
return false;
}And in main(), after the existing initReplicaSet block:
if (!(await waitForPrimary())) {
console.error("Replica set did not become PRIMARY in time");
forceExit(1);
}
|
Subsumed in #4198 |
MongoDB standalone mode does not support multi-document transactions, which are required for atomic merge operations across
FrontierCollectionandMergeCollection. This converts to a single-member replica set (rs0) in both development and Docker/Kubernetes environments.Development (
npm start)scripts/startDatabase.js(new): Replacespredatabase+databasenpm scripts and the now-removedsetupMongo.js. Spawnsmongod --replSet rs0, polls until ready, callsrs.initiate(), then stays alive until mongod exits. Failures are fatal: SIGINT/SIGTERM handlers are registered early, startup/init failures callprocess.exit(1), and mongod's exit code is propagated viaprocess.exit(exitCode ?? 1).scripts/setupMongo.js(removed): No longer needed; its directory-creation logic is handled bystartDatabase.js.package.json:databasescript →node scripts/startDatabase.js;predatabaseremoved..gitignore: ReplacedsetupMongo.jsexception withstartDatabase.jsexception.Docker / Kubernetes
database/init/00-replica-set.js(new): Placed in/docker-entrypoint-initdb.d/, runs on first container startup. Callsrs.initiate()with a configurable advertised hostname viaMONGO_INITDB_REPLICA_HOST(defaults tolocalhost:27017). Waits up to 30s for PRIMARY election before allowing subsequent init scripts to run.database.yaml(Kubernetes Helm): Addsargs: ["--replSet", "rs0"]and setsMONGO_INITDB_REPLICA_HOSTto the pod IP so the replica set member hostname is tracked per pod. The postStart lifecycle hook initializes the replica set on first start and reconfigures the member host (rs.reconfig) when the pod IP changes after a restart, keeping the replica set config in sync.Configuration
appsettings.json: Connection strings now explicitly include?replicaSet=rs0..vscode/tasks.json:run-mongotask updated to invokenpm run database(which runsscripts/startDatabase.js) instead of callingmongoddirectly, ensuring replica set initialization is performed automatically on first run.README.md: MongoDB Compass connection string updated tomongodb://localhost:27017/?replicaSet=rs0.✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.
This change is
Summary by CodeRabbit
New Features
Documentation