mikebridge commented on PR #44009: URL: https://github.com/apache/superset/pull/44009#issuecomment-5588894961
@aminghadersohi — small observability PR in the versioning-capture area you've been reviewing, whenever you have a slot: the write-path latency metric that the `ENABLE_VERSIONING_CAPTURE` kill-switch decision has been missing (#42709's counters cover *loss*; this covers *slowdown*). Two commits: a structure-only hoist of `finalize_change_records` to module level (unit-testable seam; it never referenced the registration closure's state), then the metric — `superset.versioning.capture.finalize.latency` (ms, monotonic `perf_counter`), emitted through a fail-open sibling of `incr_capture_error`. One design point you may want to poke at: the timer starts **after** the finalizer's `session.flush()`, so the series measures capture-only overhead — charging the transaction's own flush (a cost that exists with versioning off) to capture could false-trigger the kill-switch on ordinary large saves; a flush that raises emits nothing. The series deliberately includes every commit (the whole-listener overhead is what the kill-switch removes), so alerting belongs on upper percentiles — the docstring says so. Tests drive the real finalizer on an isolated session: fires once per save-path commit, both guard arms emit nothing, flush-failure emits nothing, a raising stats backend can't break a save (with the swallow's log line pinned). 150/150 in `tests/unit_tests/versioning`. -- 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]
