Skip to content

MIGRATION-860 Hot Doc Check script - #192

Open
sababich wants to merge 7 commits into
mongodb:masterfrom
sababich:MF-TASK-MIGRATION-860
Open

MIGRATION-860 Hot Doc Check script#192
sababich wants to merge 7 commits into
mongodb:masterfrom
sababich:MF-TASK-MIGRATION-860

Conversation

@sababich

Copy link
Copy Markdown
Collaborator

PR Title
Add hot document spread check script and documentation updates

Summary
This PR adds a script to identify hot documents that may explain high applier spread disparity during migrations. It also documents it in the toolbox index.

Problem / Context
When diagnosing slow CEA phase, it is useful to quickly detect whether one or a few documents are disproportionately receiving writes in a recent time window. This script combines spread and throughput gates.

What Changed

Added hot document analyzer script:
migration/toolbox/hotDocSpreadCheck/hot-doc-spread-check.js

Added detailed tool documentation:
migration/toolbox/hotDocSpreadCheck/hot_doc_spread_check_readme.md

Added toolbox index entry for this new tool:
migration/toolbox/README.md

Key script behavior

  • Watches change stream events for insert/update/replace/delete.
  • Aggregates per-document counts and computes:
    • event share
    • spread disparity estimate
    • per-document CPS
    • total-window CPS
  • Returns hot document candidates only when all configured gates pass.

Copilot AI review requested due to automatic review settings July 23, 2026 09:12

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds a new mongosh-based diagnostic tool to detect “hot” documents by sampling recent change-stream writes and applying spread-disparity + throughput gates, and wires it into the migration toolbox documentation.

Changes:

  • Added hot-doc-spread-check.js script to scan change streams and compute per-document hotness metrics with configurable thresholds and output formats.
  • Added detailed tool README documenting usage, tuning, and output interpretation.
  • Updated the toolbox index to include the new tool entry.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
migration/toolbox/README.md Adds an index entry for the new Hot Doc Spread Check tool.
migration/toolbox/hotDocSpreadCheck/README.md Documents purpose, usage patterns, tuning guidance, and sample output for the tool.
migration/toolbox/hotDocSpreadCheck/hot-doc-spread-check.js Implements the change-stream sampling, aggregation, gating logic, and JSON/Markdown reporting.
Comments suppressed due to low confidence (1)

migration/toolbox/hotDocSpreadCheck/hot-doc-spread-check.js:409

  • Same issue as JSON output: prefixing with "./" prevents writing to absolute paths and is unnecessary for relative paths.
    fs.writeFileSync("./" + markdownPath, buildMarkdownReport());

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

The following example shows what a successful run can look like:

```bash
mongosh "mongodb+srv://Admin:Qwerty123@cluster0.jrmrq.mongodb.net/" --file hot-doc-spread-check.js
Comment on lines +529 to +531
## Sources

- [EP: Automatically detect and mitigate hot docs](https://docs.google.com/document/d/1mHBMjpeYnQKJyxAWjhUL7OBhGJ2733uakUWZZCsp5p4)
Comment on lines +400 to +403
fs.writeFileSync(
"./" + jsonPath,
EJSON.stringify(output, null, 2, { relaxed: false })
);

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.

@sababich is this ./ intentional/required? Otherwise I tend to access with Copilot here.

Copilot AI review requested due to automatic review settings July 23, 2026 09:23

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

Comment on lines +192 to +195
if (evt.ns.db === "admin" || evt.ns.db === "config" || evt.ns.db === "local" || evt.ns.coll.startsWith("system.")) {
if (stopReason === "caught-up") break;
continue;
}

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.

Agree that we should be able to force processing of internal namespaces somehow.

Comment thread migration/toolbox/hotDocSpreadCheck/README.md Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 23, 2026 09:28

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (2)

migration/toolbox/hotDocSpreadCheck/hot-doc-spread-check.js:403

  • outputFile is documented as an output path, but the script always prefixes it with "./" when writing. This breaks absolute paths (e.g., "/tmp/out.json" becomes ".//tmp/out.json") and is inconsistent with other toolbox scripts that write to the provided path directly (e.g., getBusiestCollection/get-busiest-collections.js:187, probIndexesComplete/probIndexesComplete.js:98).
    fs.writeFileSync(
      "./" + jsonPath,
      EJSON.stringify(output, null, 2, { relaxed: false })
    );

migration/toolbox/hotDocSpreadCheck/hot-doc-spread-check.js:411

  • Markdown output is also written with a forced "./" prefix, which prevents using absolute output paths and diverges from other toolbox scripts that write to the given path directly.
  if (outputFormat === "markdown" || outputFormat === "both") {
    const markdownPath = outputFormat === "markdown" ? deriveOutputPath("markdown") : "hot-doc-spread-check.md";
    fs.writeFileSync("./" + markdownPath, buildMarkdownReport());
    print(`Wrote Markdown results to ${markdownPath}`);
  }

Comment thread migration/toolbox/hotDocSpreadCheck/README.md Outdated
Comment on lines +1 to +5
# Hot Doc Spread Check

## Purpose

`hot-doc-spread-check.js` is a mongosh script that scans recent change activity and identifies documents that are hot enough to plausibly explain high applier skew.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 23, 2026 09:35

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

migration/toolbox/hotDocSpreadCheck/hot-doc-spread-check.js:186

  • The "caught-up" stop condition is checked after fetching an event but the event is still counted/aggregated before the loop exits. This can include at least one post-open (out-of-window) change-stream event in the sample, skewing counts and potentially tripping gates unexpectedly.
    matchedEventsSeen++;
    lastEventAt = Date.now();
    lastClusterTime = evt.clusterTime;

    if (evt.clusterTime && evt.clusterTime.t * 1000 >= stopAtMs) {
      stopReason = "caught-up";
    }

migration/toolbox/hotDocSpreadCheck/README.md:5

  • PR description says the detailed documentation was added as hot_doc_spread_check_readme.md, but the PR actually introduces hotDocSpreadCheck/README.md and the toolbox index links to that. Consider updating the PR description (or renaming files/links) to avoid confusion for readers.
# Hot Doc Spread Check

## Purpose

`hot-doc-spread-check.js` is a mongosh script that scans recent change activity and identifies documents that are hot enough to plausibly explain high applier skew.

}
}

validateNumber("lookbackMs", CFG.lookbackMs, true);
Copilot AI review requested due to automatic review settings July 23, 2026 09:47

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (3)

migration/toolbox/hotDocSpreadCheck/hot-doc-spread-check.js:401

  • Writing output files with a hard-coded "./" prefix breaks absolute output paths (e.g. "/tmp/report.json" becomes ".///tmp/report.json"). It also makes it harder to intentionally write outside the current working directory.
      "./" + jsonPath,

migration/toolbox/hotDocSpreadCheck/hot-doc-spread-check.js:409

  • Same output-path issue for Markdown: prefixing with "./" prevents using absolute paths for markdownPath.
    fs.writeFileSync("./" + markdownPath, buildMarkdownReport());

migration/toolbox/hotDocSpreadCheck/README.md:5

  • PR description says the tool documentation was added as hot_doc_spread_check_readme.md, but the PR adds migration/toolbox/hotDocSpreadCheck/README.md instead. Please update the PR description (or rename the file) so it matches the actual change.
# Hot Doc Spread Check

## Purpose

`hot-doc-spread-check.js` is a mongosh script that scans recent change activity and identifies documents that are hot enough to plausibly explain high applier skew.

Comment thread migration/toolbox/hotDocSpreadCheck/README.md
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 23, 2026 09:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

migration/toolbox/hotDocSpreadCheck/hot-doc-spread-check.js:410

  • The script prepends "./" to the configured output file paths. This breaks absolute paths (e.g. "/tmp/out.json" becomes ".//tmp/out.json") and is inconsistent with other toolbox scripts that pass the filename directly (e.g. migration/toolbox/getBusiestCollection/get-busiest-collections.js:187). Write to the path as provided instead of forcing a relative prefix.
  if (outputFormat === "json" || outputFormat === "both") {
    const jsonPath = outputFormat === "json" ? deriveOutputPath("json") : DEFAULTS.outputFile;
    fs.writeFileSync(
      "./" + jsonPath,
      EJSON.stringify(output, null, 2, { relaxed: false })
    );
    print(`Wrote JSON results to ${jsonPath}`);
  }

  if (outputFormat === "markdown" || outputFormat === "both") {
    const markdownPath = outputFormat === "markdown" ? deriveOutputPath("markdown") : "hot-doc-spread-check.md";
    fs.writeFileSync("./" + markdownPath, buildMarkdownReport());
    print(`Wrote Markdown results to ${markdownPath}`);

}
);

cs.disableBlockWarnings();

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.

Looks like a good suggestion

Comment on lines +133 to +135
const perNs = Object.create(null);
const nsOrder = [];
const opMap = {

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.

@sababich do you agree this could be a problem for large datasets?

@niccottrell niccottrell 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.

@sababich Copilot has raised some good points that merit your consideration before we approve this.

Comment on lines +400 to +403
fs.writeFileSync(
"./" + jsonPath,
EJSON.stringify(output, null, 2, { relaxed: false })
);

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.

@sababich is this ./ intentional/required? Otherwise I tend to access with Copilot here.

Comment on lines +192 to +195
if (evt.ns.db === "admin" || evt.ns.db === "config" || evt.ns.db === "local" || evt.ns.coll.startsWith("system.")) {
if (stopReason === "caught-up") break;
continue;
}

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.

Agree that we should be able to force processing of internal namespaces somehow.

Comment on lines +133 to +135
const perNs = Object.create(null);
const nsOrder = [];
const opMap = {

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.

@sababich do you agree this could be a problem for large datasets?

}
);

cs.disableBlockWarnings();

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.

Looks like a good suggestion

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.

3 participants