Add listener_uri stress test flag for pluggable EventListener

[hx235]

Summary:
Adds a --listener_uri flag to db_stress that creates an EventListener via ObjectLibrary from the given URI and attaches it to the DB options.

Test:

• Compilation
• e2e test will be done next internally with a customized listener

[meta-codesync]

@hx235 has exported this pull request. If you are a Meta employee, you can view the originating Diff in D104750476.

[github-actions]

⚠️ clang-tidy: 1 warning(s) on changed lines

Completed in 80.1s.

Summary by check

Check	Count
concurrency-mt-unsafe	1
Total	1

Details

db_stress_tool/db_stress_test_base.cc (1 warning(s))

db_stress_tool/db_stress_test_base.cc:3973:9: warning: function is not thread safe [concurrency-mt-unsafe]

[hx235]

db_stress_tool/db_stress_test_base.cc:3973:9: warning: function is not thread safe [concurrency-mt-unsafe]

exit(1) is widely used in db stress test

[github-actions]

🟡 Codex Code Review

Auto-triggered after CI passed — reviewing commit 52029eb

---

❌ 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 passed — reviewing commit 52029eb

---

Summary

Small, low-risk change adding a --listener_uri flag to db_stress for pluggable EventListener creation. The implementation is functionally correct — the listener is properly created before DB::Open() and error handling follows db_stress conventions. One medium-severity consistency issue: the PR uses ObjectRegistry::Default()->NewSharedObject directly instead of the idiomatic EventListener::CreateFromString API used elsewhere in the codebase.

High-severity findings (0):

No high-severity findings.

Full review (click to expand)

Findings

🔴 HIGH

None.

🟡 MEDIUM

M1. Should use EventListener::CreateFromString instead of ObjectRegistry::Default()->NewSharedObject — db_stress_test_base.cc:3965

• Issue: The PR creates the listener via ObjectRegistry::Default()->NewSharedObject<EventListener>(), but the idiomatic RocksDB pattern for Customizable objects is to use the type's CreateFromString static method. EventListener::CreateFromString exists (db/event_helpers.cc:13) and is used elsewhere (options/db_options.cc:563). The existing db_stress precedent for URI-based object creation also uses CreateFromString (e.g., SecondaryCache::CreateFromString at db_stress_test_base.cc:133).
• Root cause: Using the lower-level ObjectRegistry API instead of the higher-level Customizable API.
• Suggested fix:

  if (!FLAGS_listener_uri.empty()) {
    ConfigOptions config_options;
    std::shared_ptr<EventListener> listener;
    Status s = EventListener::CreateFromString(
        config_options, FLAGS_listener_uri, &listener);
    if (!s.ok() || !listener) {
      fprintf(stderr, "Failed to create listener from URI '%s': %s\n",
              FLAGS_listener_uri.c_str(), s.ToString().c_str());
      exit(1);
    }
    options_.listeners.emplace_back(std::move(listener));
  }

  Note: CreateFromString can return OK with null result (see db_options.cc:564 which checks both), so a null check is warranted with this API.

🟢 LOW / NIT

L1. Placement after RegisterAdditionalListeners() is slightly inconsistent — db_stress_test_base.cc:3965

• Issue: RegisterAdditionalListeners() is a virtual method for subclasses to add listeners. The URI-based listener is added in the base class after this call, which means it's not overridable. This is fine for the current use case but could be confusing — a reader might expect all listener setup to happen in RegisterAdditionalListeners().
• Suggested fix: Consider adding the URI listener inside the base RegisterAdditionalListeners() implementation, or document that this is intentionally separate.

L2. Error message format differs from other URI flags — db_stress_test_base.cc:3970

• Issue: The error message format "Failed to create listener from URI '%s': %s" differs slightly from the existing pattern "No secondary cache registered matching string: %s status=%s" at line 137.
• Suggested fix: Minor — consider aligning the format for consistency.

Cross-Component Analysis

Context	Affected?	Assessment
DB reopen	Yes	Safe — options_.listeners.clear() at line 3960 rebuilds the list each time
Fault injection during Open	No	Listener is added before fault injection setup; no interaction
ReadOnly DB	Yes	Safe — listeners are added to options regardless of open mode
Transaction DB	Yes	Safe — listener is added before the txn/non-txn open branch
Multi-ops txn stress	Yes	Safe — RegisterAdditionalListeners() runs first, URI listener appends after

Positive Observations

• Correct lifecycle management: listener is added to options_.listeners before DB::Open(), and the vector is cleared and rebuilt on each reopen.
• Proper error handling with exit(1), consistent with other db_stress flag validation.
• The ObjectRegistry::NewSharedObject API guarantees a non-null shared_ptr on Status::OK(), so the current code is safe from null pointer issues (though switching to CreateFromString would need a null check).
• Flag naming (listener_uri) and description follow established conventions (env_uri, fs_uri, secondary_cache_uri).

---

ℹ️ 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

[meta-codesync]

@hx235 has imported this pull request. If you are a Meta employee, you can view this in D104750476.

[meta-codesync]

@hx235 merged this pull request in a2c96df.