add stats

[joshkang97]

Stack from ghstack (oldest at bottom):

• -> add stats #14812

[github-actions]

✅ clang-tidy: No findings on changed lines

Completed in 247.2s.

[github-actions]

🟡 Codex Code Review

Auto-triggered after CI reached the early-review threshold — reviewing commit 6fcf5a5

---

❌ Codex review failed before producing findings.

WARNING: proceeding, even though we could not update PATH: Refusing to create helper binaries under temporary dir "/tmp" (codex_home: AbsolutePathBuf("/tmp/codex-home"))
error: the argument '--base <BRANCH>' cannot be used with '[PROMPT]'

Usage: codex exec review --commit <SHA> --base <BRANCH> --title <TITLE> --model <MODEL> --config <key=value> --dangerously-bypass-approvals-and-sandbox --output-last-message <FILE> [PROMPT]

For more information, try '--help'.

---

ℹ️ About this response

Generated by Codex CLI.
Review methodology: claude_md/code_review.md

Limitations:

• Codex may miss context from files not in the diff
• Large PRs may be truncated
• Always apply human judgment to AI suggestions

Commands:

• /codex-review [context] — Request a code review
• /codex-query <question> — Ask about the PR or codebase

[github-actions]

✅ Claude Code Review

Auto-triggered after CI reached the early-review threshold — reviewing commit 6fcf5a5

---

Summary

Clean, well-patterned change that adds an INGEST_EXTERNAL_FILE_TIME histogram following established StopWatch conventions. All enum values, JNI mappings, and name strings are correct and consistent. No correctness or performance concerns.

High-severity findings (0):

No high-severity findings.

Full review (click to expand)

Findings

🔴 HIGH

None.

🟡 MEDIUM

M1. Missing multi-CF test for "one sample per call" claim -- db/external_sst_file_test.cc

• Issue: The commit message and documentation claim "one sample per call, not per column family," but the test only exercises the single-CF IngestExternalFile() path. There is no test calling IngestExternalFiles() with multiple column families to verify the histogram records exactly one sample.
• Root cause: Test only covers the basic path.
• Suggested fix: Add a test that creates two column families, calls IngestExternalFiles({cf1_arg, cf2_arg}), and asserts hd.count == 1 (not 2). This directly validates the documented per-call semantics.

M2. No test for stats-level gating -- db/external_sst_file_test.cc

• Issue: The StopWatch is gated on stats_level > kExceptTimers. There is no test verifying the histogram is NOT recorded when stats level is kExceptTimers or lower.
• Root cause: Edge case not covered.
• Suggested fix: Add a test (or extend the existing one) that sets stats_level = kExceptTimers, performs an ingestion, and asserts hd.count == 0.

🟢 LOW / NIT

L1. Verbose StopWatch variable name -- db/db_impl/db_impl.cc:6680

• Issue: The variable is named ingest_external_file_sw while other StopWatch instances in the same file use shorter names: sw (line 3144, 4018) or write_sw (db_impl_write.cc:1097).
• Suggested fix: Consider renaming to sw for consistency. This is purely stylistic.

Cross-Component Analysis

Context	Does code execute?	Assumptions hold?	Action needed?
WritePreparedTxnDB	YES (delegates to DBImpl)	YES	None
ReadOnly DB	NO (IngestExternalFiles not supported)	N/A	None
Secondary Instance	NO	N/A	None
User-defined timestamps	YES	YES	None
Concurrent writers	YES (ingestion acquires DB mutex)	YES	None

The StopWatch is purely additive instrumentation on a cold path with RAII lifetime management. It introduces no new synchronization, no new data flows, and no behavioral changes. All execution contexts that reach IngestExternalFiles() already have stats_ and immutable_db_options_.clock properly initialized.

Positive Observations

• Correct placement in IngestExternalFiles() captures both the single-CF IngestExternalFile() and the multi-CF IngestExternalFiles() entry points without double-counting.
• StopWatch and PERF_TIMER_GUARD are complementary (shared Statistics histogram vs. per-thread perf context) -- not redundant.
• Java JNI bindings are complete and consistent (0x43 in both directions).
• HistogramsNameMap entry follows naming convention (rocksdb.<subsystem>.<operation>.micros).
• Release note is concise and appropriate.

---

ℹ️ About this response

Generated by Claude Code.
Review methodology: claude_md/code_review.md

Limitations:

• Claude may miss context from files not in the diff
• Large PRs may be truncated
• Always apply human judgment to AI suggestions

Commands:

• /claude-review [context] — Request a code review
• /claude-query <question> — Ask about the PR or codebase