[RFC] MongoDB Storage Backend - #207
Conversation
LeeroyHannigan
added
the
RFC
|
Hi @diegotoledano95, Thank you for the contribution RFC. The RFC looks great, but I did find some gaps while reviewing the reference implementation. Below are the findings, ranging from critical to minor. Please let us know if you need any of the below clarified, we'd be happy to help. Critical FindingsC1. GSI index collection uses GSI keys as document
|
|
The fixes for the gaps detailed in the comment above have been done and are ready for review. You can find them in the The RFC presented in this PR has also been changed to reflect those changes. The |
|
Thanks for the substantial revision. All 11 Critical and 15 Major findings from the first pass are addressed. The core data-plane design is sound: transactional GSI + stream propagation, the netstring composite Conformance (verified live)
Blocking1. Table 2. Binary
The string path already handles this correctly via 3. Field-vs-field conditions evaluate backwards.
Fix: mark 4. Rebase required The branch is based on 5. Steering violations in Non-blocking (worth tracking at merge)
|
|
@LeeroyHannigan Acknowledging the feedback, will start working on the blocking issues. Quick question, do you want to continue reviewing the code as we have on the forked branch? Or do you want to start adding the code here in the current PR or a new PR? |
|
Thanks @diegotoledano95 Would be great to get it here, along with your intended CI. #218 does change how backends register, so if you want to wait until we merge that in, make those changes on your fork and then push here, might be cleanest. |
|
@LeeroyHannigan Will do thanks! Would you have an ETA on #218 ? |
|
@diegotoledano95 #218 has just landed. That should unblock you. |
5f7700d to
947d2a2
Compare
|
@LeeroyHannigan Have pushed the changes for the blocking issues above, and put the code in this branch and PR as requested too. Please let me know what you think, thanks! |
Architecture design for the extenddb-storage-mongodb crate covering: - Collection schema (catalog_db + data_db) - Document structure (_id, pk, sk_*, item_data) - Concurrency model (transactions + optimistic versioning) - GSI synchronous propagation strategy - Stream record storage - Bootstrapper and configuration
Implements the full TableEngine, DataEngine, MetadataEngine, StreamEngine, BackupEngine, WorkerStore, and catalog traits against MongoDB 6.0+. Key design decisions: - Single-item writes (put/delete/update) use MongoDB transactions with snapshot read concern and majority write concern for atomicity - Stream records and GSI sync are in the same transaction as the data write - UpdateItem uses optimistic concurrency (_v version field) with session reuse across retries for performance under contention - Condition expressions compiled to MongoDB query filters via condition.rs - Numbers stored as strings in item_data to preserve DynamoDB 38-digit decimal precision - Binary sort key begins_with uses post-fetch filtering (BSON Binary comparison sorts by length first, making $gte/$lt unreliable for prefix matching) - Simple unconditional SET/REMOVE updates use native MongoDB operators via findOneAndUpdate for lower latency Wiring: adds mongodb feature flag to bin crate, registers backend via inventory, and generalizes cmd_serve backend validation. Requires: MongoDB 6.0+ configured as a replica set (even single-node) for multi-document transactions and snapshot reads.
- extenddb-mongo.toml: integration test config for MongoDB backend using ~/.extenddb/tls paths (portable) and enforce_reserved_keywords=true - extenddb.sample.toml: add [storage.mongodb] section - devtools/run-tests: export EXTENDDB_CONFIG; only set EXTENDDB_TEST_PG_CONNECTION_STRING for postgres URLs
- docs/local-mongodb-setup.md: MongoDB installation and replica set setup - docs/getting-started.md: add MongoDB build/init instructions - AGENTS.md: update architecture, prerequisites, pitfalls for MongoDB
stream_engine.rs and data_engine.rs wrote the shadow event_name column
via format!("{:?}", record.event_name), producing "Insert" / "Modify" /
"Remove". DynamoDB Streams' wire contract is uppercase: "INSERT" /
"MODIFY" / "REMOVE".
Add event_name_ddb_str() in stream_engine.rs to map StreamEventName to
its wire-format string, and use it at both call sites.
Unit test asserts each enum variant maps to the expected uppercase
string.
DynamoDB rejects a KeyConditionExpression sk BETWEEN :lo AND :hi with :lo > :hi as ValidationException. The engine layer's condition evaluator does this check for filter/condition expressions, but the KeyConditionExpression path in Query goes through the storage backend's sort-key filter builder, which was emitting $gte lo, $lte hi with no check — matching zero documents without an error. Add the check in build_sk_filter. Comparison is done in the AttributeValue domain before Decimal128/f64 conversion. Numeric comparison uses f64 for ordering only; values that would lose Decimal128 precision are rejected downstream in sk_to_bson. Unit tests cover S, N, and B ordering.
…lures Previously all four TransactWriteItems condition-failure sites in data_engine.rs (Put, Delete, Update, ConditionCheck) called condition_check_failed_with_item(None), discarding the pre-existing item that had already been loaded into scope. Callers that set ReturnValuesOnConditionCheckFailure=ALL_OLD on their transact op therefore got a CancellationReason with no Item field — inconsistent with DynamoDB, which returns the failing item under that flag. Thread return_values_on_ccf from TransactWriteOp through the mongo crate's OwnedTransactWriteOp, and add a small helper ccf_return_item that gates inclusion on both (a) ALL_OLD requested and (b) the item actually existed. Preserves DDB's guarantee that missing items never manifest as CancellationReason.Item. Adds unit tests for the helper covering all three code paths.
The mongo backend stores numeric partition/sort keys as BSON Decimal128
for correct numeric ordering. Decimal128 supports 34 significant
decimal digits; DynamoDB supports up to 38.
Previously the write path (data/mod.rs::item_to_document), key-filter
path (data/mod.rs::pk_filter), and sort-key comparison path
(data_engine.rs::sk_to_bson) all fell back to f64 on Decimal128 parse
failure. f64 has ~15 digits of precision, so values in the 35-38 digit
range were silently truncated, breaking numeric ordering guarantees on
sort keys (e.g. Query with ScanIndexForward could return items in an
order that disagrees with the callers numeric interpretation).
Reject values that exceed Decimal128 precision at all three sites with
a ValidationException explaining the limit. Document as a
MongoDB-backend-specific behavioral difference in
docs/differences-from-dynamodb.md.
Numbers in non-key attribute positions are unaffected: item_data
stores the DynamoDB number string verbatim inside the {"N": ...} tag
and is never numerically compared by the backend.
Adds unit tests for the write path and pk_filter path at the
34-digit boundary and beyond.
Long function bodies and lint-boundary formatting picked up by cargo fmt after the preceding four fix commits. No behavior change.
…ne.rs
Nested `if let Some(ref sk_cond) = key_condition.sk_condition` around
`if let SortKeyCondition::BeginsWith { .. } = sk_cond` collapsed into
a single pattern. Behavior identical.
Upstream pinned a stricter Rust toolchain in 6c59a25, whose clippy is stricter about the collapsible_if and collapsible_match lints. 16 sites in the original mongo backend contribution now trip these lints: - authorization_store.rs (1 site) - data_engine.rs (11 sites) - metadata_engine.rs (4 sites) Mechanical fix — every site is 'if outer { if inner { ... } }' collapsed to 'if outer && inner { ... }', or the equivalent 'if let' pattern. Applied via 'cargo clippy --fix -p extenddb-storage-mongodb', followed by 'cargo fmt --all' to re-align the resulting blocks. Behavior unchanged. 24 unit tests still pass.
stream_engine.rs::next_sequence_number used a single global counter document (_id: "stream_seq") for every shard of every table. The shard_id argument was accepted but ignored. latest_sequence_number filters by shard_id when reading, so writer and reader disagreed on the sequence-number space: a write to shard B advanced the counter shard A reads back, producing non-contiguous sequence numbers on shard A's GetRecords pages. DynamoDB Streams' contract is that sequence numbers are strictly monotonic within a shard and independent across shards. Key the counter document by shard_id (stream_seq:<shard_id>) so each shard gets its own atomic counter. The 21-digit zero-padded encoding of the sequence number is preserved.
MongoEngine::gsi_cache is a per-process DashMap<table_id, bool>. In a multi-instance deployment sharing a MongoDB catalog, an admin adding a GSI via instance A does not invalidate instance B's cache. Instance B continues to see the cached false and silently skips index updates on subsequent writes — the base table and the newly-added GSI drift out of sync until the cache is reset by a restart. Change the cache value from bool to (bool, Instant) and reject entries older than GSI_CACHE_TTL (60 seconds) on read. When a stale entry is observed, the caller falls through to the catalog query, which returns the authoritative state and refreshes the cache. Introduce three MongoEngine helpers (gsi_cache_get_fresh, gsi_cache_set, gsi_cache_invalidate) so callers don't reach into the DashMap directly. Update the five existing call sites (data_engine.rs sync_indexes and sync_indexes_in_session; table_engine.rs DropTable, CreateGSI, DeleteGSI) to use them.
Only the base-table collection got mongo indexes at CreateTable time. GSI/LSI collections were left to be lazy-created on first index-row insert, and — critically — never got any indexes on their query columns. Every Query / Scan against a secondary index therefore ran a full-collection scan; cost was linear in the index's row count. Add `create_index_data_collection` on MongoEngine and invoke it from all three sites that create a GSI/LSI catalog row: - CreateTable initial GSI list - CreateTable initial LSI list - UpdateTable GSI-create path (async D-C5 backfill flow) The compound index is `(pk, sk?, base_pk, base_sk?)` — the same tuple that `sync_indexes` and `scan_impl` sort by after D-C1's schema change. String sort keys get the `simple` collation so range comparisons stay byte-wise, matching the base table.
The mongo backend generated `stream_label` values via the `time` crate's default Iso8601 formatter — nanosecond precision, `Z` suffix, `.123456789Z` tail. Postgres emits second precision without a timezone (`YYYY-MM-DDThh:mm:ss`) via a to_char cast. Clients parsing labels round-tripped through one backend but not the other, and comparison-based lookups (list-streams pagination, by-label ARN parsing) could see two backend runs produce incompatible ARNs for the same table. Add `format_stream_label` and route all three writer sites — the initial CreateTable label, the UpdateTable re-enable path, and the label-restore branch — through it. Byte-for-byte matches postgres output.
The `StreamEngine::write_stream_record` trait method predates D-C2 — back when stream records were written outside the data-write transaction. Every real caller now routes through `MongoEngine::write_stream_inline_in_session`, which enrolls the stream write in the same session as the base-table mutation so a rolled-back data write can't leave a phantom stream record behind (RFC-0003 §3.2, §5.1). The non-transactional impl remained in the file — 40+ lines of live code that would silently break stream/data atomicity if called externally. Replace the body with an immediate error explaining the supersession. Trait method stays (it's declared upstream in `extenddb-storage`), the body no longer does subtly-wrong work.
`build_sk_filter` computed the exclusive upper bound for `BEGINS_WITH P` as `P + char::MAX` and matched with `$lt`. That misses any stored value equal to `P + char::MAX` — which still begins with `P` — and any value that extends past it (a string whose first char::MAX is followed by more content). Rare in practice but a real correctness gap. Replace `increment_string` with `next_string_prefix`: walk the prefix from the right, find the first char that isn't `char::MAX`, increment it (skipping the surrogate gap via `char::from_u32`), and truncate everything to its right. That yields the least string strictly greater than every P-starting string. Edge case: a prefix consisting entirely of `char::MAX` (or an empty prefix — but the engine layer already rejects that) has no upper bound; drop the `$lt` clause so mongo matches every value ≥ P. Matches DDB's behavior.
The condition compiler in `condition.rs` has latent correctness bugs on numeric operands, set/list/map equality, mixed-type IN lists, and attribute_type with an unvalidated type tag. Option (c) from the D-M6 decision: keep the compiler, expand analyzer tests to lock the buggy shapes out of the pushdown path. - Tighten `attribute_type` in the analyzer: require the placeholder to resolve to a string that is exactly one of the ten DDB type tags (S, N, B, BOOL, NULL, L, M, SS, NS, BS). The compiler interpolates the tag verbatim into the mongo field path, so accepting an arbitrary string like `$ne` would produce a filter clause where "field" starts with a mongo operator. - Add four analyzer regression tests that lock in exclusion of every input shape D-M6 flagged as compile-time-buggy: numeric compares, set/list/map equality, IN, and attribute_type with an invalid tag. Plus a positive test that `attribute_type` with a valid tag still pushes. Compiler bodies stay unchanged — the analyzer is now the load- bearing correctness boundary, and its expansion is checked in with tests.
Rewrites three sections of docs/rfcs/0000-mongodb-backend.md where the
RFC described an initially-planned design that differs from what the
implementation does, plus small errata.
Section rewrites:
- Condition expression evaluation (was "Condition expression
pushdown"). RFC previously described a single-round-trip design in
which compiled MongoDB filters were pushed into findOneAndReplace /
findOneAndDelete. Implementation evaluates conditions in-Rust
against a loaded item inside a MongoDB client session that also
wraps the write. The condition compiler exists as scaffolding for
a future filter-pushdown optimization but is not on the correctness
path. Section rewritten to describe the session-scoped approach,
with the pushdown path documented as an alternative considered.
- Backup. RFC previously described using MongoDB's server-side $out
aggregation stage to per-backup collections named
_backup_{backup_id}_{table_id}. Implementation writes items into a
shared backup_items collection keyed by backup_arn, with backup
metadata in extenddb_catalog.backups. Section rewritten; $out
documented as a future optimization.
- Performance characteristics. The "single-item writes:
transaction-free" claim was too broad. Rewritten to reflect that
the session wrap is what provides DynamoDB's atomicity contract,
with a sessionless fast path planned as a follow-up optimization
for the narrow case of tables with no streams and no GSIs.
Errata:
- Plugin registration: four -> five inventory::submit! calls
(diagnostics store registration was omitted).
- Database layout: 17 -> 18 catalog collections; enumerated the
additional collections; noted iam_group_members and backup_items
auto-create on first insert rather than in migrations.
- Write conflict handling: OCC retry base 100us -> 50us (matches
code at data_engine.rs:706).
- Removed size from the list of supported compiled functions.
- Removed the "all-backends" convenience flag from the roadmap.
- Design decisions summary and implementation summary tables updated
to reflect the rewrites (session-scoped conditional writes; shared
backup_items collection; cmd_serve.rs added as a modified file).
- Numeric sort keys: added a note that values exceeding Decimal128's
34 significant digits are rejected at write/query time (see
docs/differences-from-dynamodb.md).
- Collation: dropped the strength: 3 specifier; the code uses
MongoDB's default (Tertiary) which matches DynamoDB behavior.
No behavioral change to the proposed design.
Rewrites both documents from scratch to describe the MongoDB
backend as it exists today, dropping earlier-planned architecture
that never made it into the shipping code (single-round-trip filter
pushdown as the primary path, sk_b as BSON Binary, index docs
without base-key disambiguation, synchronous inline GSI create,
etc.).
RFC-206 (`docs/rfcs/0000-mongodb-backend.md`):
- Document structure section covers netstring `_id`, typed sort-key
fields including hex-encoded binary, `_v` OCC counter, and the
index-doc base_pk/base_sk_? disambiguation with 4-tuple _id.
- Condition-evaluation section describes the session-scoped
read + evaluate + write flow as the correctness path, with the
analyzer-gated pushdown as an opt-in fast path.
- Query/Scan covers hex-based binary begins_with, sort-key range
preservation on pagination, and the compound index cursor.
- GSI/LSI section covers the CREATING → ACTIVE state machine and
the async gsi_backfill_worker.
- Transactions section covers WriteConflict retry with
TransactionConflict-on-exhaustion.
- Streams section covers per-shard session-scoped sequence
counters, shard_id embedding table_id, 24 h TTL retention,
and idempotent stream-enable.
- Operational requirements adds primary-only readPreference
enforcement.
Design doc (`docs/design/13-storage-mongodb.md`):
- Catalog schemas (§3) now show TableClass / SSE / OnDemandThroughput
persistence, indexes with `backfill_cursor` + `CREATING`.
- Data schemas (§4) document netstring `_id`, `_v`, index docs
with base-key fields and per-index compound query index,
stream_records (TTL + compound query index), stream_shards
(unique on shard_id derived from table_id), counters keyed
per-shard, idempotency_tokens (540 s TTL + unique compound on
account_id+token), and `_backup_{backup_id}` collections.
- §5 rewritten around: session-scoped conditional writes,
analyzer-gated pushdown, base-key disambiguation with compound
cursor pagination, async GSI backfill, session-scoped per-shard
sequence counters, WriteConflict retry, hex sort keys, and the
next_string_prefix upper-bound.
- §12 replaced with a feature-coverage inventory grouped by trait
and background worker.
- §14 adds primary readPreference enforcement.
RFC-0003 §6.2 forbids blanket `#[allow(unused)]` suppressions because they conceal unimplemented trait methods and let latent bugs ship dormant. The crate had `#![allow(unused)]` at the top of `lib.rs`; removing it surfaced 19 real dead-code items. Cleanup: - Delete `MongoEngine::sync_indexes` (non-session variant). Every write path routes through `sync_indexes_in_session` since D-C2; the standalone variant was unused and independently missed the D-C2 through D-C6 fixes (it had the old wrong index-key filter, the `let _ = delete_one` swallow, and no compound-cursor support). Removing it eliminates a shadow implementation that could have been called by mistake. - Delete `data::index_collection_name` — dupe of `data_collection_name`. - Delete `MongoCatalogStore::client` accessor — nothing outside the struct used it. - Delete `MongoEngine.max_connections` field — set at construction and never read (mongo driver's `max_pool_size` already carries the value into the client options). - Delete an unused `use futures::TryStreamExt` inside `sync_indexes_in_session`; the method uses `cursor.next(session)` which comes from a different trait. - Fix the unused-var warning on `item_count` in `backup_engine.rs`. - Strip 14 unused imports via `cargo fix`.
The previous `WorkerStore::process_control_plane_transitions` body
was dead code with two independent problems:
1. Its query filters (`{ "account_id": X, "table_name": Y, ... }`)
read those fields at the top level of the tables catalog
document, but the mongo catalog stores account_id/table_name
inside `_id.account_id` / `_id.table_name`. The filter would
never match a real row.
2. `MongoRuntimeHooks::spawn_workers` never spawns anything that
calls this method, and the mongo backend never puts a table
into a transient CREATING/DELETING state to begin with —
`create_table_impl` writes `TableStatus: ACTIVE` synchronously,
`delete_table_impl` runs collection/tag/stream cleanup inline.
RFC-0003 §6.3 forbids dead-code impls that pretend to be operational.
RFC-0003 §10.1 requires that any trait method needing periodic
maintenance be spawned via `ServerRuntimeHooks::spawn_workers`.
Either fix the impl and wire it up, or state clearly that the
backend has no work to do. The latter is honest here — GSI create
is the one control-plane operation that does need async work, and
that lives in `ttl_worker::gsi_backfill_worker` on the `indexes`
catalog, not the `tables` catalog.
Replace the body with `Ok(Vec::new())`. Update RFC-206 and the
design doc to describe control-plane transitions as inline, and
call out the no-op nature of the WorkerStore trait method.
`sync_indexes_in_session` used `let _ = idx_coll.delete_one(...).await;` when removing an index entry whose base item's GSI-key attribute just changed or was removed. A transient error on that delete was silently discarded, leaving the stale index row live under the old GSI-key value while the new value was upserted. Subsequent Query requests kept returning the stale projection forever. Propagate the error. `sync_indexes_in_session` is called from the data-plane write path inside the same session as the base write, so a real error here rolls back the whole transaction — including the base-table replace/insert — and the client sees a retryable error rather than silent index divergence. RFC-0003 §2.2 (stale entry prevention) + §9.1 (no silent degradation).
Two concurrent `UpdateTable(stream_enabled=true)` calls can both observe "no shards yet" for the same table_id (snapshot isolation lets each transaction see a state that predates the other's insert) and both proceed to `init_stream_shards`. Because `stream_shards.shard_id` carries a unique index — see `bootstrapper.rs` — one of the inserts hits E11000 and surfaces as `StorageError::Internal` → HTTP 500. RFC-0003 §10.3 requires repeated `UpdateTable` calls with the same specification to not corrupt state and either reject cleanly or no-op. Treat E11000 as a no-op here: the shard already exists with the same `(shard_id, table_id)` binding by construction (the shard_id is deterministic from the table_id), so the client's retry sees the expected state without a wire-visible error. Non-duplicate errors still propagate as `Internal`.
…ites
RFC-0003 §4.1 requires that concurrent unconditional single-item
writes never surface a client-visible conflict — "two concurrent
`PutItem` on the same key must both succeed (last-writer-wins)."
The backend previously wrapped every PutItem/DeleteItem/UpdateItem
in a snapshot MongoDB transaction with a 50-attempt WriteConflict
retry loop. Under sustained contention the loop exhausts and
surfaces `StorageError::Internal("too many concurrent write
conflicts")` → HTTP 500, which DDB never emits.
The txn wrapper is only necessary when a write has dependent side
effects — conditional evaluation, stream capture, GSI sync — that
must be atomic with the base write. When none of those apply, the
storage engine's single-doc atomicity is sufficient on its own,
and the txn wrapper is what *creates* the conflict it then has to
retry through.
Split each write into a sessionless fast path:
- **PutItem** — when `condition.is_none() && stream.is_none() &&
gsi_cache_get_fresh == Some(false)`, run a plain
`find_one_and_replace(upsert=true, ReturnDocument::Before)`. No
session, no retry. Concurrent writers converge naturally at the
WiredTiger level; no error is ever emitted for contention alone.
- **DeleteItem** — same gate, plain `find_one_and_delete`.
- **UpdateItem** — extend the existing native fast path to handle
numeric ADD via an aggregation-pipeline update. `$toDecimal`
parses the string-stored `.N` value server-side, `$add` applies
the delta as Decimal128, `$toString` writes it back. 50
concurrent `ADD counter :one` calls now serialize inside
MongoDB with a `_v` bump per apply — no OCC retry, no client-
visible conflict. RFC-0003 §4.4.
Introduces `NativeUpdate::{Doc, Pipeline}` so the update fast path
can dispatch to either the operator-document form (simple
$set/$unset) or the pipeline form (numeric ADD, or a mix).
Session-scoped paths remain for: conditional writes (need read +
check + write atomicity), streams-enabled tables (need base +
stream-record atomicity), GSI-bearing tables (need base + index
atomicity), and updates the pipeline can't express (list_append,
if_not_exists, arithmetic on strings, DELETE from set). Those
paths still retry on WriteConflict, but they only cover cases
that RFC-0003 §4.1's exemption doesn't apply to.
The `stream_shards` unique-index defense from the earlier
`init_stream_shards` idempotency fix means that even the racy
session-scoped path is safer than before.
Adds a pytest module that asserts strict conformance against the
four RFC-0003 §4.x scenarios. Client uses `retries={"max_attempts": 0}`
via the shared conftest fixture so any InternalServerError from the
backend surfaces immediately instead of being masked by the SDK.
Cases covered:
- §4.1 50 concurrent unconditional PutItem on the same key —
all must succeed, no client-visible errors.
- §4.1 50 concurrent conditional PutItem with attribute_not_exists —
exactly one wins, the rest fail with
ConditionalCheckFailedException. No InternalServerError.
- §4.4 50 threads × 20 iterations of `UpdateItem ADD counter :one`
on the same key — every increment applies, final counter
equals 1000, no lost updates, no client-visible errors.
- §4.1 50 concurrent unconditional DeleteItem on the same key —
all succeed, item ends up absent.
These are the specific scenarios the sessionless fast path fix
targets. Prior to the fix, `TestRfc0003UnconditionalPutOnHotKey`
and `TestRfc0003AtomicCounterAdd` would surface InternalServerError
after the 50-attempt WriteConflict retry ceiling exhausted.
…lict RFC-0003 §4.3: when a single-item write conflicts with an in-flight `TransactWriteItems` on the same item, the backend must return `TransactionConflictException` — never `InternalServerError`. The backend may retry transient conflicts internally, but must not exhaust retries and surface an unmapped error. The mongo backend's put/delete/update paths exhaust their 50-retry WriteConflict loop and returned `StorageError::Internal(...)` → HTTP 500. That's the exact case §4.3 forbids. Two-part fix: 1. Add `StorageError::TransactionConflict(String)` to the shared storage trait surface and map it in `engine/src/create_table.rs::storage_err_to_dynamo` to `DynamoDbError::TransactionConflictException`. The variant is general — any backend can emit it when a contention path exhausts internal retry. 2. Mongo `put_item_impl` / `delete_item_impl` / `update_item_impl` now return `StorageError::TransactionConflict` on ceiling exhaustion instead of `StorageError::Internal`. With the Phase 6 sessionless fast paths, ceiling-exhaustion is only reachable on the session-scoped path — i.e. writes on GSI-bearing or streams-enabled tables — where §4.3's applicability is exact.
Derive Zeroize and ZeroizeOnDrop so the base64-encoded AES-256-GCM master encryption key is scrubbed from memory when the credential store is dropped. Matches the DbCredentialStore in the postgres backend.
Inspect the parsed ClientOptions in MongoEngine::new and emit a WARN log when the URI does not enable TLS. Without this warning, an operator who configures a bare mongodb://host:27017 URI has no indication that credentials and data are traversing the network in cleartext.
…tring MongoBootstrapper::record_data_connection previously wrote the raw connection string into the settings collection under data_connection_string. When the URI carries a userinfo password (mongodb://user:pass@host/...), the plaintext password sat at rest in the catalog and was readable by anyone with read access to the extenddb_catalog database. Add redact_connection_string, which replaces the password component of the userinfo with `<redacted>`, and apply it before the upsert. The scheme, username, host, port, and query string are preserved so the stored value remains a useful reference. Unit tests cover the standard mongodb:// scheme, mongodb+srv://, bare URIs with no userinfo, username-only URIs, `@` characters that appear only in the query string (authSource=admin), and inputs without a URI scheme.
MongoCredentialStore::lookup_user_credential read the is_active field with unwrap_or(true), so a record whose is_active field was missing, absent, or of the wrong BSON type was treated as active. Combined with a partial write during key rotation or a schema mismatch after a migration, this could authenticate a credential that should have been inactive. Change the default to false: if the flag cannot be read as a bool, the credential is rejected. Correct records with is_active: true continue to authenticate normally.
The runner was implicitly postgres-only in two places: it greps the
config for `backend = "postgres"` before extracting a pg connection
string, and it runs `test_cli_lifecycle.py` (postgres-only) whenever
that connection string is set. Both worked accidentally on a mongo
config today — the postgres-backend grep just missed and everything
downstream was a no-op — but the coupling to config-file contents is
fragile.
Add an explicit `--backend {postgres,mongodb}` flag (default
`postgres` for backward compat). The flag gates the two postgres-only
paths and prints the backend in the target-info block. Everything
else — health check, credential provisioning, throttling +
import/export config mutation, pytest / rust / external / catalog-
check suites — stays backend-agnostic and needs no change.
Unblocks a mongo CI workflow that can delegate to `run-tests` the
same way `.github/workflows/integration.yml` does for postgres.
Nothing in the mongo backend has been verified against 6.x; local development, the bench-compare harness, and the container tag used in the planned CI workflow all use `mongo:7`. Bring the docs in line — stating 6.0+ implies a support surface we don't test and can't stand behind. Documentation-only change. No code touches the mongo-driver version floor; that's controlled by the `mongodb` crate's own minimum.
`devtools/run-tests` is a runner, not an orchestrator — it assumes the server is already up at `$EXTENDDB_TEST_ENDPOINT`. The postgres CI workflow supplies the server lifecycle (init, serve, poll /health) inline before delegating to `run-tests`. Local mongo runs had no equivalent — the bench-compare harness recreated the lifecycle each time by hand. `devtools/run-mongodb-tests` fills that gap: one entry point that spins up a `mongo:7` single-node replica set in Docker, initializes and serves extenddb against it, then delegates to `devtools/run-tests --backend mongodb`. Teardown on exit; `--keep` leaves everything up for post-run inspection. Arguments after `--` are forwarded to `run-tests` verbatim so callers can pick the suite (`--pytest`, `--comprehensive`, `--parallel`, `--filter …`). Default is `--pytest --comprehensive --parallel`. The mongo CI workflow (a follow-up commit on this branch) can call this script directly and drop most of its shell-level orchestration.
Upstream db0baba added `account_id` to `Storage::get_stream_records` so GetRecords is scoped to the shards owning account; the mongo backend still implemented the old 4-arg signature and returned records without an ownership check, so a caller could read another accounts stream records by presenting a forged shard iterator. Add the `account_id` parameter and an ownership guard that mirrors storage-postgres: resolve shard_id -> table_id from `stream_shards` (data db), then confirm a `tables` catalog document with that table_id is owned by the calling account (account_id lives inside the compound `_id`, so the comparison is done in Rust after a single table_id lookup). When the shard is unowned or absent, return ValidationException("Invalid ShardIterator") — matching DynamoDB, which does not distinguish "exists but not yours" from "does not exist". Verified by tests/test_cross_account_isolation.py::TestStreamAccountScoping ::test_shard_iterator_only_returns_owning_account_records against the mongo backend. Also syncs Cargo.lock (extenddb-storage-mongodb 0.1.0 -> 0.1.2) to the workspace version bump pulled in by the rebase.
…eering Start the server via extenddb serve (which daemonizes itself) instead of serve --foreground with a background &, and stop it via extenddb stop instead of kill. Set server.run_dir to the test output dir so serve and stop share an isolated PID-file location. Removes the manually-managed server.pid file.
pushdown.rs admitted Field <op> Field for all types, but a plain field type is unknown at compile time, so the emitted $expr compared the raw tagged subdocuments. Two Number fields (stored string-encoded) then compared lexically, so counter_a < counter_b evaluated backwards in both directions. Mark Field vs Field NotPushable so it falls back to the in-Rust evaluator, consistent with the existing N and B literal exclusions. Adds a regression test locking every comparator.
Binary sort keys are stored as lowercase hex strings, so begins_with is a
string-prefix range over the hex encoding. The upper bound was computed as
hex(increment_bytes(prefix)) -- incrementing the raw bytes then re-encoding
-- which is not the next prefix in fixed-width hex space and widens the
range. begins_with(0x2F,0xFF) produced ["2fff","3000") and wrongly matched
the stored key 0x30 ("30"); begins_with(0xFF) produced an empty range and
dropped every match.
Use next_string_prefix on the hex encoding, mirroring the string sort-key
path: sk_b >= hex(B) AND sk_b < next_string_prefix(hex(B)), dropping the
upper bound when the prefix is empty. Removes the now-unused increment_bytes
helper. Adds a regression test for both wire-level repros.
CreateTable and RestoreTableFromBackup now write the catalog row as CREATING with a status_transition_at timestamp when control_plane_delay_seconds > 0 (default 0.25), and return CREATING; a new background control_plane_worker flips rows to ACTIVE once the scheduled transition time passes. When the delay is 0 the row is written ACTIVE directly. Matches the postgres backend and real DynamoDB, which report CREATING before a table is usable. DeleteTable stays inline (no DELETING state). Restore delegates row creation to create_table and no longer forces the table ACTIVE inline, so it enters the same CREATING window; the data is copied via $out before the worker flips the table to ACTIVE. Data-plane key-schema resolution against a non-ACTIVE table now returns ResourceNotFoundException (TableNotFound) instead of ResourceInUse, matching DynamoDB and the postgres backend. Restores WorkerStore::process_control_plane_transitions (fixing the compound _id query the previous no-op replaced) and spawns the poller from MongoRuntimeHooks::spawn_workers. Reverts the RFC and design-doc language that described control-plane transitions as inline. Fixes the conformance tests put_item_on_creating_table_returns_not_found and restore_table_from_backup.
…ndDB#218 main Rebase onto upstream main after PR ExtendDB#218 (serve lib decoupling), which replaced inventory backend registration with an explicit set_backend/Backend model and split the CLI into extenddb-app. Also adapts to backup-trait and worker changes and to new backup_arn_scoping conformance tests pulled in by the rebase. - Replace the six inventory::submit! blocks with a single extenddb_storage_mongodb::backend() constructor plus a server_components_factory fn, mirroring the postgres backend. - Drop the now-removed inventory dependency. - Feature-gate the thin bin: install the mongodb backend under --features mongodb, else postgres. - Scope describe_backup and delete_backup to account_id (added to the BackupEngine trait upstream); exclude DELETED backups from describe_backup so a deleted backup reads as BackupNotFoundException. - Give backup ARNs a timestamp-plus-8-hex-char random id so they are not guessable from creation time alone. - Return the spawned worker JoinHandles from spawn_workers, whose trait signature now requires Vec<JoinHandle<()>>.
Rebase onto upstream main (6dcb14c), whose per-index consumed-capacity work added global_secondary_indexes and local_secondary_indexes to TableKeyInfo. Load all secondary indexes from the catalog in table_key_info_from_doc and populate both lists (via a new index_info_from_doc helper) so per-index consumed capacity is computed from the cached TableKeyInfo without an extra describe_table per write, matching the postgres backend. has_lsi is now derived from the LSI list.
947d2a2 to
d9fe4da
Compare
2 participants
What
Adds docs/rfcs/0000-mongodb-backend.md, a draft RFC for adding MongoDB as a optional ExtendDB storage backend.
Why
MongoDB is a natural fit as an additional database target: data model alignment; high read/write throughput through horizontal scalability; infrastructure fit.
DynamoDB and MongoDB share the same data model approach - documents stored as schema-less JSON-like data. MongoDBs document model maps directly to the approach taken by DynamoDB with each item stored as a MongoDB BSON document with no impedance mismatch at the data model level. Unlike relational databases, the translation from JSON to BSON is direct without complicated relational mapping techniques required.
This PR proposes the RFC tracked by the below issue.
Closes #206
Related forked implementation code
Testing done
git diff --checkpython docs/build-docs.pyChecklist
cargo fmt --check) (No Rust code was changed)ADR / RFC: This PR