Repository navigation
fix(kernel): forward telemetry_flush_interval_ms to the kernel - #1026
Conversation
The kernel binding accepts telemetry_flush_interval_ms, but the connector never forwarded it, so kernel connections always used the kernel's 5s default timer even when the caller asked for a longer interval. Signed-off-by: Vu Anh Phung <vu.phung@databricks.com> Co-authored-by: Isaac <no-reply@databricks.com>
GitHub Actions
Error
GitHub Actions
Inactive
![peco-review-bot[bot]](https://avatars.githubusercontent.com/in/3801177?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict: 1 Low
Looks good. The fix is small and correctly scoped. The new kwarg goes through the existing _kernel_session_accepts_kwarg gate, so older kernel wheels are unaffected. An unset value is omitted so the kernel keeps its default, and the session threading, the forwarding, the unset case and the fixed-signature filtering all have tests. One low concern: the "must be positive" contract is documented but not validated in the connector.
PyO3 accepts True as 1ms and reports negative or non-int values with a conversion error that does not name the option. Signed-off-by: Vu Anh Phung <vu.phung@databricks.com> Co-authored-by: Isaac <no-reply@databricks.com>
GitHub Actions
Inactive
GitHub Actions
Inactive
GitHub Actions
Error
GitHub Actions
Inactive
There was a problem hiding this comment.
Verdict: 1 Low
Looks good. The fix is small and well tested, with one low concern: on older kernel wheels an explicitly set flush interval is still dropped without any error or warning. Validation, the unset → omitted behavior, and the threading through session.py all have tests.
GitHub Actions
Error
On the kernel path,
connect(telemetry_flush_interval_ms=...)is silently dropped._kernel_telemetry_kwargsforwardsenable_telemetry,telemetry_batch_size, and the circuit-breaker flag, but not the flush interval, even though the kernelSessionacceptstelemetry_flush_interval_ms. Every kernel connection therefore flushes on the kernel's 5s default timer.This made databricks-driver-test TELEMETRY-015 flaky. That test asks for a 300s interval so that only session closes export telemetry. In nightly run 43146507303, a slow first
SELECT(12.4s) let the 5s timer fire exactly 5.0s after the host telemetry client was created. The upload carried the test's twoCREATE_SESSIONevents before any close.The connector now forwards an explicit
telemetry_flush_interval_msto the kernel. When the option is unset, nothing is sent and the kernel keeps its default. Thrift is unchanged; its exporter still flushes every 300s. The option is documented inCONNECTION_PARAMETERS.md.Validation:
-m "not realkernel"). Real-kernel tests passed against kernel6f4b6d6(the currentKERNEL_REV).black --check srcandmypy srcare clean.telemetry_flush_interval_ms=300_000, then a 12s stall before any close. Onmain, a telemetry upload landed 5.0s after the first CreateSession. With this branch there were no uploads until close.test_telemetry.pymodule in kernel mode passed 3/3 runs with this branch.test_feature_flags.py(kernel, non-OAuth cases) also passed.This pull request and its description were written by Isaac.