Skip to content

fix: skip data init on restart if data already exists (#612)#1

Draft
richardsimmonds wants to merge 1 commit into
mainfrom
fix/612-restart-loop-duplicate-key
Draft

fix: skip data init on restart if data already exists (#612)#1
richardsimmonds wants to merge 1 commit into
mainfrom
fix/612-restart-loop-duplicate-key

Conversation

@richardsimmonds

Copy link
Copy Markdown
Owner

Problem

When using documentdb-local with a persistent volume and --init-data true, the container enters a restart loop on the second boot. The built-in sample data JS files use insertMany with hardcoded _id values, which causes a DuplicateKey error when the init scripts run again against an already-seeded database. The init failure triggers exit 1, and Docker restarts the container — creating an infinite loop.

The same issue affects custom data initialization via --init-data-path if the user's JS files use hardcoded _id values.

Fixes documentdb#612

Fix

Adds marker-file idempotency to both data initialization sections in emulator_entrypoint.sh:

  • Sample data init: Before running the sample data init scripts, the entrypoint checks for $DATA_PATH/.sample_data_initialized. If the marker file exists, initialization is skipped with an informative log message. On successful first-time init, the marker file is created.

  • Custom data init: Same pattern using $DATA_PATH/.custom_data_initialized.

The marker files live in $DATA_PATH (the persistent volume mount), so they survive container restarts.

Edge cases handled

  • Skip message is informative: When init is skipped, the log message explains that data was already initialized and tells users how to delete the marker file to force re-seeding.
  • No marker on failure: If the init script fails, the marker file is NOT written. This allows the init to be retried on the next boot (e.g., if the failure was transient).
  • No behavior change when init is disabled: When --init-data is not set (the default), neither init section runs and no marker files are created.
  • No flag/API changes: This is purely an internal idempotency guard — no new flags, no changed defaults.

Files changed

  • documentdb-local/scripts/emulator_entrypoint.sh — marker file logic around both init sections
  • documentdb-local/scripts/documentdb_local_tests/test_emulator_entrypoint.py — 7 new test cases covering marker creation, skip-on-second-boot, skip message content, no-marker-on-failure, and no-marker-when-disabled

Signed-off-by: richardsimmonds <richardsimmonds314@gmail.com>

@patty-chow patty-chow left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Review: LGTM ✅

Summary

Clean, well-scoped fix for issue documentdb#612. The marker-file idempotency pattern is the right approach for preventing duplicate-key errors on container restart.

Checklist

DCO Sign-off: ✅ Present (Signed-off-by: richardsimmonds <richardsimmonds314@gmail.com>)

Correctness: ✅

  • Marker files are written to $DATA_PATH (the persistent volume), so they survive container restarts.
  • touch is placed after successful init, inside the success branch — no marker on failure.
  • The failure branch calls exit 1, so control never falls through to the marker creation.
  • custom_data_initialized=true is correctly set when the marker is found (used by the "No initialization data loaded" message at line 589).

Edge Cases: ✅

  • Partial init failure: exit 1 is called and no marker is created — correct. On next restart, init will be re-attempted.
  • Missing DATA_PATH: Created earlier in the script (line 361-364) with sudo mkdir -p and proper ownership/permissions.
  • If touch itself fails (e.g., disk full), no marker is created and init will re-run on next restart — this is the safe default behavior.

Bash Safety: ✅

  • All marker path variables are properly double-quoted: "$CUSTOM_DATA_MARKER", "$SAMPLE_DATA_MARKER".
  • No set -e in this script, so no unexpected interaction with the if guards.
  • No race conditions — single-threaded entrypoint, marker creation is sequential.

Test Coverage: ✅ (7 new tests, all passing)

  1. test_sample_data_init_creates_marker_file — first boot creates marker
  2. test_sample_data_init_skipped_on_second_boot — second boot skips
  3. test_sample_data_skip_message_mentions_marker_deletion — UX: tells user how to re-seed
  4. test_custom_data_init_creates_marker_file — custom data path marker
  5. test_custom_data_init_skipped_on_second_boot — custom data skip on restart
  6. test_sample_data_no_marker_when_init_disabled — no marker when feature off
  7. test_sample_data_no_marker_on_failure — no marker on failure (critical safety test)

Style: ✅ Follows existing conventions — same indentation, echo patterns, variable naming.

Scope: ✅ Minimal, focused diff. Only the two init sections are modified; tests cleanly appended.

Minor Suggestion (non-blocking)

Consider adding a test_custom_data_no_marker_on_failure test to mirror the existing test_sample_data_no_marker_on_failure. The code paths are structurally identical so this is very low risk, but it would make the test matrix symmetric. Not blocking approval on this.

All 17 tests (10 existing + 7 new) pass locally. Ship it! 🚢

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

documentdb-local restart loop on second boot: sample-data seed re-runs and fails with DuplicateKey (regression of #479)

2 participants