Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,7 @@ compressed when the client sends `Accept-Encoding`.
| `HFS_MONGODB_SEARCH_TIMEOUT_MS` | *(unset)* | Soft MongoDB search/count budget (ms), 1–2147483647, shared by admission, planning, pages, totals and inline includes. Unset or blank preserves existing behavior. Checks occur before commands and matching batches, never per ID. Completed search results are returned even if late; unfinished planning, retries and additional batches can still time out. Active commands/cursor reads may overrun; detected expiry returns an error without partial results. New commands receive the remaining maxTimeMS; implicit getMore retains its cursor budget. The HTTP timeout is separate. |
| `HFS_MONGODB_PROBE_TIMEOUT_MS` | `10` | Server execution budget for optional cardinality and ordered-page probes (ms), from 1 through 2147483647. Unset or blank preserves the default. A probe’s own timeout falls back to normal matching; the optional whole-search deadline still applies. |
| `HFS_MONGODB_ADAPTIVE_OFFSET_PAGING` | `true` | Samples eligible no-total offsets and caches their execution strategy. `false` disables adaptive offset selection; other search plans are unchanged. |
| `HFS_MONGODB_MISSING_METADATA` | `true` | Records absent search parameters on each resource (`missing_params`) and builds `idx_resources_missing_params` so `:missing=true` can use them. `false` stops recording them (each write removes the field), skips the index build, and answers `:missing=true` from the value indexes instead; results are the same. An existing index is not dropped. |
| `HFS_MONGODB_CONNECT_TIMEOUT_MS` | `5000` | TCP handshake timeout (ms) |
| `HFS_MONGODB_BROAD_SEARCH_CONCURRENCY` | *(unset: no limit)* | How many potentially broad standard searches run at once per process; others wait for a slot. Reads and narrow searches (`_id`, a reference or uri, a date with `eq`/`ap`, `identifier=system\|code`) bypass this admission limit. A value at or above the pool size is accepted with a startup warning; `0` fails startup. |
| `HFS_MONGODB_COUNT_BY_TENANT_MAX_TIME_MS` | `25000` | Server execution budget (`maxTimeMS`) for the cross-tenant resource count behind the Tenants UI, `GET /admin/tenants`, the existence check of `DELETE /admin/tenants/{id}` and the console tenant metrics. No other query carries it. Past it the server stops the count and the call fails with a timeout error, never a zero count. A caller outside the HTTP request timeout, such as the Tenants UI, sees that error directly and shows its error state instead of counts. The REST endpoints answer `504` only when this budget is below `HFS_REQUEST_TIMEOUT`, as the defaults are (25 s against 30 s). The request timeout starts when the request arrives, before the count, so with a budget at or above it the request timeout answers `408` first; the budget still stops the count on the server a moment later. Keep it below `HFS_REQUEST_TIMEOUT` when raising either. It bounds server execution only, not server selection, waiting for a pooled connection or network time. Must be 1 to 2147483647; anything else fails startup. The 25 s default is policy (five seconds under the default request timeout), not a large-store measurement. The count is a covered scan of the `idx_resources_live_tenant` index, which boot creates; for its build cost on an existing store see `docs/mongodb/tenant-count-index.md`. |
Expand Down
71 changes: 70 additions & 1 deletion crates/persistence/src/backends/mongodb/backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,14 @@ pub struct MongoBackendConfig {
#[serde(default = "default_adaptive_offset_paging")]
pub adaptive_offset_paging: bool,

/// Records each resource's absent search parameters in
/// `resources.missing_params` and builds `idx_resources_missing_params`
/// (#1984, #2006). `false` stops recording them (a write removes the
/// field), skips the index build and sends every `:missing=true` search
/// through the value-index route, which returns the same results.
#[serde(default = "default_missing_metadata")]
pub missing_metadata: bool,

/// Server execution budget for optional planning probes, in milliseconds.
/// Does not bound network latency or resource/count execution.
#[serde(default = "default_probe_timeout_ms")]
Expand Down Expand Up @@ -606,6 +614,14 @@ impl MongoBackendConfig {
_ => return Err("HFS_MONGODB_ADAPTIVE_OFFSET_PAGING must be true or false".into()),
}
}
if let Some(raw) = env("HFS_MONGODB_MISSING_METADATA") {
match raw.trim().to_ascii_lowercase().as_str() {
"" => {}
"true" | "1" | "yes" | "on" => self.missing_metadata = true,
"false" | "0" | "no" | "off" => self.missing_metadata = false,
_ => return Err("HFS_MONGODB_MISSING_METADATA must be true or false".into()),
}
}
if let Some(raw) = env("HFS_MONGODB_PROBE_TIMEOUT_MS") {
let raw = raw.trim();
if !raw.is_empty() {
Expand Down Expand Up @@ -778,6 +794,10 @@ fn default_adaptive_offset_paging() -> bool {
true
}

fn default_missing_metadata() -> bool {
true
}

fn default_bundle_transaction_budget() -> Duration {
super::retry::DEFAULT_BUNDLE_TRANSACTION_BUDGET
}
Expand All @@ -800,6 +820,7 @@ impl Debug for MongoBackendConfig {
// is classified as secret or not.
let Self {
adaptive_offset_paging,
missing_metadata,
probe_timeout_ms,
search_timeout_ms,
connection_string,
Expand Down Expand Up @@ -828,6 +849,7 @@ impl Debug for MongoBackendConfig {
} = self;
f.debug_struct("MongoBackendConfig")
.field("adaptive_offset_paging", adaptive_offset_paging)
.field("missing_metadata", missing_metadata)
.field("probe_timeout_ms", probe_timeout_ms)
.field("search_timeout_ms", search_timeout_ms)
.field(
Expand Down Expand Up @@ -873,6 +895,7 @@ impl Default for MongoBackendConfig {
fn default() -> Self {
Self {
adaptive_offset_paging: default_adaptive_offset_paging(),
missing_metadata: default_missing_metadata(),
probe_timeout_ms: default_probe_timeout_ms(),
search_timeout_ms: None,
connection_string: default_connection_string(),
Expand Down Expand Up @@ -1301,7 +1324,16 @@ impl MongoBackend {
let db = self.get_database().await?;
schema::initialize_schema_async(&db).await?;

let missing_tx = self.missing_index_tx.lock().await.take();
let mut missing_tx = self.missing_index_tx.lock().await.take();
if !self.config.missing_metadata
&& let Some(tx) = missing_tx.take()
{
// Disabled (#2006): nothing reads or maintains the index, so it is
// neither built nor required. An existing one is left in place.
let _ = tx.send(Some(BuildOutcome::Skipped {
missing: Vec::new(),
}));
}
let missing_handle = missing_tx.map(|tx| {
let builder = MissingIndexBuilder::new(db.clone(), self.config.index_build);
tokio::spawn(async move {
Expand Down Expand Up @@ -2312,6 +2344,43 @@ mod tests {
);
}

#[test]
fn missing_metadata_defaults_on_and_can_be_disabled() {
let default = MongoBackendConfig::default();
assert!(default.missing_metadata);
let mut serialized = serde_json::to_value(&default).unwrap();
serialized
.as_object_mut()
.unwrap()
.remove("missing_metadata");
assert!(
serde_json::from_value::<MongoBackendConfig>(serialized)
.unwrap()
.missing_metadata
);
for (raw, enabled) in [
("on", true),
(" TRUE ", true),
("off", false),
("0", false),
("", true),
] {
let mut config = default.clone();
config
.apply_search_env(|name| {
(name == "HFS_MONGODB_MISSING_METADATA").then(|| raw.to_string())
})
.unwrap();
assert_eq!(config.missing_metadata, enabled, "{raw:?}");
}
assert!(
MongoBackendConfig::default()
.apply_search_env(|name| (name == "HFS_MONGODB_MISSING_METADATA")
.then(|| "sometimes".to_string()))
.is_err()
);
}

#[test]
fn broad_search_admission_validates_configuration() {
let mut config = MongoBackendConfig::default();
Expand Down
10 changes: 9 additions & 1 deletion crates/persistence/src/backends/mongodb/search_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1632,6 +1632,14 @@ fn contained_composite_value_stages(
Ok(first)
}

/// Whether `missing_params` records `name` when absent. `_id` and
/// `_lastUpdated` live on every resource document and `:missing` on them
/// never reads the array, so recording them would only add index keys
/// (#2006).
pub(super) fn records_missing_metadata(name: &str) -> bool {
!matches!(name, "_id" | "_lastUpdated")
}

impl MongoBackend {
fn supports_indexed_missing(
&self,
Expand Down Expand Up @@ -1669,7 +1677,7 @@ impl MongoBackend {
resource_type: &str,
param: &SearchParameter,
) -> bool {
!matches!(param.name.as_str(), "_id" | "_lastUpdated")
records_missing_metadata(&param.name)
&& matches!(param.modifier, Some(SearchModifier::Missing))
&& param
.values
Expand Down
33 changes: 18 additions & 15 deletions crates/persistence/src/backends/mongodb/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3598,23 +3598,26 @@ impl MongoBackend {
// unparseable date, a Period end that is not a date) is as
// absent as one that extracted nothing: without this it would
// match neither `:missing=true` nor `:missing=false` (#2000).
let missing = extraction
.missing
.into_iter()
.map(|parameter| parameter.name)
.chain(
extraction
.present
.into_iter()
.filter(|parameter| {
!own.iter().any(|row| row_marks_present(row, parameter))
})
.map(|parameter| parameter.name),
)
.collect::<Vec<_>>();
let missing = self.config().missing_metadata.then(|| {
extraction
.missing
.into_iter()
.map(|parameter| parameter.name)
.chain(
extraction
.present
.into_iter()
.filter(|parameter| {
!own.iter().any(|row| row_marks_present(row, parameter))
})
.map(|parameter| parameter.name),
)
.filter(|name| super::search_impl::records_missing_metadata(name))
.collect::<Vec<_>>()
});
(
own,
Some(missing),
missing,
(!extraction.failures.is_empty()).then(|| extraction.failures.join("; ")),
)
}
Expand Down
156 changes: 156 additions & 0 deletions crates/persistence/tests/mongodb/missing_index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2492,3 +2492,159 @@ async fn mongodb_missing_metadata_count_drives_from_selective_positive_filter()
}
db.drop().await.unwrap();
}

/// `_id` and `_lastUpdated` are columns of every resource document and
/// `:missing` on them never reads `missing_params`, so they are not recorded
/// even though Synthea-style resources carry no `meta.lastUpdated` (#2006).
#[tokio::test]
async fn mongodb_missing_metadata_omits_resource_columns() {
let Some(backend) = create_backend("missing_columns").await else {
return;
};
let tenant = create_tenant("missing-columns");
backend
.create(
&tenant,
"Observation",
observation("no-meta", vec![]),
FhirVersion::default(),
)
.await
.unwrap();
let db = backend.get_database().await.unwrap();
let names = missing_parameters(&db, &tenant, "Observation", "no-meta").await;
assert!(
names.iter().any(|name| name == "_tag"),
"other absent meta parameters are still recorded: {names:?}"
);
for column in ["_id", "_lastUpdated"] {
assert!(
!names.iter().any(|name| name == column),
"{column}: {names:?}"
);
}
}

/// `missing_metadata: false` (#2006): writes leave no `missing_params` and
/// clear what an enabled writer left, the index is not built, and
/// `:missing` answers exactly as with metadata.
#[tokio::test]
async fn mongodb_missing_metadata_disabled_keeps_missing_results() {
let Some(connection_string) = shared_mongo::connection_string().await else {
return;
};
let database_name = build_test_database_name("missing_disabled");
let config = MongoBackendConfig {
connection_string,
database_name: database_name.clone(),
data_dir: Some(repo_data_dir()),
..Default::default()
};
let tenant = create_tenant("missing-disabled");

// An enabled writer first, as on a deployment that later opts out.
let Some(enabled) = build_backend(config.clone()).await else {
return;
};
register_parameters(&enabled, &tenant).await;
enabled
.create(
&tenant,
"Observation",
observation("written-enabled", vec![]),
FhirVersion::default(),
)
.await
.unwrap();
let db = enabled.get_database().await.unwrap();
assert!(
!missing_parameters(&db, &tenant, "Observation", "written-enabled")
.await
.is_empty()
);
db.collection::<Document>("resources")
.drop_index("idx_resources_missing_params")
.await
.unwrap();
drop(enabled);

let Some(disabled) = build_backend(MongoBackendConfig {
missing_metadata: false,
..config
})
.await
else {
return;
};
assert!(matches!(
disabled.wait_for_missing_index_build().await,
Some(BuildOutcome::Skipped { .. })
));
let listed = db
.collection::<Document>("resources")
.list_index_names()
.await
.unwrap();
assert!(
!listed
.iter()
.any(|name| name == "idx_resources_missing_params"),
"the disabled backend must not build the index: {listed:?}"
);

for (id, values) in [
("absent", vec![]),
("null", extensions(true)),
("present", extensions(false)),
] {
disabled
.create(
&tenant,
"Observation",
observation(id, values),
FhirVersion::default(),
)
.await
.unwrap();
}
let current = disabled
.read(&tenant, "Observation", "written-enabled")
.await
.unwrap()
.unwrap();
disabled
.update(
&tenant,
&current,
observation("written-enabled", extensions(false)),
)
.await
.unwrap();
let without_metadata = db
.collection::<Document>("resources")
.count_documents(doc! {
"tenant_id": tenant.tenant_id().as_str(), "resource_type": "Observation",
"missing_params": { "$exists": false },
})
.await
.unwrap();
assert_eq!(
without_metadata, 4,
"no Observation may keep missing_params"
);

for parameter in &PARAMETERS {
for missing in [true, false] {
let expected: &[&str] = if missing {
&["absent", "null"]
} else {
&["present", "written-enabled"]
};
let mut query = missing_query(parameter, missing);
for total in [TotalMode::Accurate, TotalMode::Estimate] {
query.total = Some(total);
assert_results(&disabled, &tenant, &query, expected).await;
}
}
}
}
Loading
Loading