codeant-ai-for-open-source[bot] commented on code in PR #44009:
URL: https://github.com/apache/superset/pull/44009#discussion_r3995239198
##########
superset/versioning/metrics.py:
##########
@@ -52,3 +52,28 @@ def incr_capture_error(stage: str) -> None:
stats_logger_manager.instance.incr(f"{_CAPTURE_METRIC_PREFIX}.{stage}.error")
except Exception: # pylint: disable=broad-except
logger.exception("versioning: failed to emit capture-error metric")
+
+
+def emit_capture_timing(stage: str, duration_ms: float) -> None:
+ """Emit the write-path latency for one capture *stage*, in milliseconds.
+
+ The documented recovery lever for capture trouble is the
+ ``ENABLE_VERSIONING_CAPTURE`` kill-switch, flipped on save-path
+ slowdown โ this series
+ (``superset.versioning.capture.<stage>.latency``) is the signal an
+ operator alerts on before flipping it. :func:`incr_capture_error`
+ covers *loss*; this covers *slowdown*. The series includes every
+ commit on the session โ commits touching no versioned entity still
+ pay the listener overhead โ so alerts belong on upper percentiles,
+ not the mean. Best-effort under the same fail-open posture: metrics
+ emission must never itself break a user's save.
+ """
+ # pylint: disable=import-outside-toplevel
+ try:
+ from superset.extensions import stats_logger_manager
+
+ stats_logger_manager.instance.timing(
+ f"{_CAPTURE_METRIC_PREFIX}.{stage}.latency", duration_ms
+ )
+ except Exception: # pylint: disable=broad-except
+ logger.exception("versioning: failed to emit capture-latency metric")
Review Comment:
**Suggestion:** Because this path runs on every commit, a failed metrics
backend logs a traceback for every save, causing severe log amplification and
added latency during an outage. [performance]
**Assessment:** ๐ `Major` ยท ๐ `Occurrence: Rarely`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=a90f01f449a4432f97d2d81ce535da12&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=a90f01f449a4432f97d2d81ce535da12&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/versioning/metrics.py
**Line:** 78:79
**Comment:**
*Performance: Because this path runs on every commit, a failed metrics
backend logs a traceback for every save, causing severe log amplification and
added latency during an outage.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44009&comment_hash=9885527ca47650e77200eb0700b03b09c3c98b39f2bf98f3bf90c881853ae243&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44009&comment_hash=9885527ca47650e77200eb0700b03b09c3c98b39f2bf98f3bf90c881853ae243&reaction=dislike'>๐</a>
##########
superset/versioning/changes/listener.py:
##########
@@ -395,7 +396,51 @@ def _persist_buffered_records(
incr_capture_error("bulk_insert")
-def register_change_record_listener() -> None: # noqa: C901
+def finalize_change_records(session: Session) -> None:
+ """Build and persist the transaction's change records at commit time.
+
+ Module-level (rather than a closure inside the registration function)
+ so the capture write path can be exercised directly by unit tests
+ against an isolated session; it depends only on the session and the
+ module helpers, never on the registered entity classes.
+ """
+ if session.in_nested_transaction() or session.info.get(_FINALIZING_KEY):
+ return
+
+ session.info[_FINALIZING_KEY] = True
+ # Measures the FINALIZE stage only: the timer starts after the flush,
+ # which excludes the transaction's own write cost but also excludes
+ # capture_initial_states' per-entity pre-state SELECTs โ and runs
+ # through every capture step and early return. Every commit on the
+ # session emits a sample, including commits touching no versioned
+ # entity, because the whole-listener overhead is exactly what the
+ # kill-switch removes; a flush that raises emits nothing.
+ start: float | None = None
+ try:
+ session.flush()
+ start = perf_counter()
+ initial_states: dict[tuple[str, int], tuple[Any, dict[str, Any]]] = (
+ session.info.get(_INITIAL_STATES_KEY, {})
+ )
+ buffer = _build_scalar_buffer(initial_states)
+
+ tx_id = _current_transaction_id(session)
Review Comment:
**Suggestion:** Exceptions from `_current_transaction_id` and later
finalization steps are uncaught, so a capture failure can propagate from the
`before_commit` listener and abort the user's save. [error handling]
**Assessment:** ๐ `Major` ยท ๐ `Occurrence: Sometimes`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=eb02af37135d4e5ab79d576c57e5ef0f&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=eb02af37135d4e5ab79d576c57e5ef0f&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/versioning/changes/listener.py
**Line:** 427:427
**Comment:**
*Error Handling: Exceptions from `_current_transaction_id` and later
finalization steps are uncaught, so a capture failure can propagate from the
`before_commit` listener and abort the user's save.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44009&comment_hash=7dd05093cf5ed6795339703327e073b1cafd8658366b1ac9f53d86a546367e7e&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44009&comment_hash=7dd05093cf5ed6795339703327e073b1cafd8658366b1ac9f53d86a546367e7e&reaction=dislike'>๐</a>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
