Repository navigation
Conversation
A storage-service read publishes a data access, and the monitor service turns that into an expiration extension for the monitor that produces the type. The cluster topology monitor reads the topology it produces on every tick, so it renewed its own 15-minute expiration indefinitely and could never be reaped as idle. StorageService.get gains a keyword-only register_data_access flag, default True so every consumer is unaffected, and the monitor's own read passes False. Keyword-only so a positional argument cannot silently become this flag. The expiration is still not reachable on its own: ClusterTopologyMonitorImpl reports can_dispose only once it has already stopped, so remove_expired_if never fires for a live monitor. That is tracked separately; the two defects mask each other, and fixing either alone changes nothing observable.
karenc-bq
added this pull request to stack #1298
October 20, 2026 07:15
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
The async topology monitor held the provider that built it and called back into it, so every provider ran its own background task, topology cache and monitoring connection for the same cluster. This puts the async path on the shared storage service and monitor service, as the synchronous provider already is.
StorageService.getgains a keyword-onlyregister_data_accessflag, defaultTrue, and the monitor's own read passesFalse. The expiration is still unreachable on its own becausecan_disposeisTrueonly after the monitor stops; that is tracked separately.(cluster_id, loop_token)key. The loop token is needed because entries hold mutableHostInfoobjects written in place, and a monitor owns a task bound to one loop, so a bare cluster id would hand a fresh loop a dead loop's entry. Readers get copies.MonitorService.run_if_absentunder the same key the topology is stored at.MonitorServicehas no liveness check and extends expiration on every cache hit, so a monitor on a closed loop would be handed out until the stuck sweep removed it while every blocking refresh waited for an update that could not arrive. The provider rechecksis_runningand replaces a dead monitor once, removing it by identity through the newstop_and_remove_value, because a cleanup pass may already have put a replacement at the key.cluster_idproperty, default"1", matching the synchronous provider and the property's own documentation. Connections sharing an id share a topology entry and a monitor, so an application using more than one cluster has to set it per cluster.Behaviour changes
refresh()no longer applies its own freshness window; it returns the stored entry and the monitor keeps it fresh.topology_refresh_msstill sets the monitor's tick interval.cluster_instance_host_patternis rejected when the provider is built, during connect, rather than on the first topology query. That is where the synchronous provider validates it.MonitorResetEventon the async side before, so a switchover left the old host list readable until it expired.Deliberate divergences from sync
stop()is non-blocking and schedules teardown on the monitor's own loop, where sync joins its thread for 30 seconds.MonitorStopEventis delivered on the publisher's thread and async blue/green publishes from a coroutine, so blocking would wedge that loop. An awaitableaclose()stays registered as a shutdown hook, sincerelease_resourcesclears the monitor cache and would drop the last reference first.can_disposereports whether a refresh is in flight. Sync reportsself._stop.is_set(), false for every live monitor, which makes its expiration dead code. Filed separately.process_eventends by removing the stored topology, which sync does not do. Filed separately.Other Fixes
ExpirationTrackingCache.detach_valuereported success whether or not it removed anything.tests/unit/conftest.pygains a monitor-service reset, since monitors are keyed partly by loop and a recycled loop identity let one test's monitor reach the next.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.