aminghadersohi commented on code in PR #44009:
URL: https://github.com/apache/superset/pull/44009#discussion_r3970945594
##########
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
+ # The latency series measures CAPTURE overhead only: the timer starts
+ # after the transaction's own final flush — a cost that exists with
+ # versioning disabled and must not be charged to capture — and runs
Review Comment:
The excluded `session.flush()` also fires `before_flush` ->
`capture_initial_states`, whose per-entity `_read_pre_state` SELECT is
capture-only cost. Probe: 200ms of before_flush work emitted 0.0ms, so the
series is blind to the cost that scales with dirty entities.
```suggestion
# 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
```
--
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]
