Skip to content

fix(aio): async monitoring fixes (#1284, #1276, #1287, #1288) - #1290

Open
AhmadMasry wants to merge 5 commits into
aws:mainfrom
AhmadMasry:fix/aio-monitoring-fixes
Open

AhmadMasry wants to merge 5 commits into
aws:mainfrom
AhmadMasry:fix/aio-monitoring-fixes

Conversation

@AhmadMasry

Copy link
Copy Markdown
Contributor

Description

Fixes four async monitoring bugs in one PR, one commit per issue. They touch the same area (topology monitor, connection tracker, host monitoring), and #1288's fix relies on #1284's: it gives host_monitoring_v2 a topology provider, which without #1284 would add one topology monitor per connection.

Fixes #1284
Fixes #1276
Fixes #1287
Fixes #1288

Supersedes #1285, #1286 and #1289.

#1284: share the async topology monitor per cluster (4c05386)

Every async connect builds its own host list provider, and each provider started its own AsyncClusterTopologyMonitor with its own monitoring connection, so a pool of N connections held 2N server connections. Sync shares one monitor per cluster id through monitor_service.run_if_absent.

  • aio/host_list_provider.py: module-level registry keyed by (provider class, cluster id, event loop); _get_or_create_monitor() reuses the running monitor and drops entries whose task has ended. The loop is in the key because an asyncio task only runs on its own loop; the provider class keeps Aurora, Multi-AZ and Global monitors apart.
  • aio/cluster_topology_monitor.py: the monitor publishes topology to a weak set of subscribed providers.
  • Lifecycle matches sync: closing a connection doesn't stop the cluster's monitor; stop() stops it (like stop_monitor), and release_resources_async() stops it at shutdown.

#1276: aurora_connection_tracker closed cluster-endpoint connections on their first statement (4b0ce7e)

The connect-time live-probe pin used current_host_info, which for a cluster-endpoint connection is the cluster endpoint. The first execute compared it with the topology writer (an instance), saw a writer change, and invalidate_all closed the connection about to execute. Sync always takes the writer from topology, so it compares instance with instance.

  • The plugin remembers the instance a cluster-endpoint connection landed on (already identified by _fill_instance_alias) and pins that.
  • _invalidate_departed_host resolves a cluster-endpoint pre_failover_host to the same instance, so idle connections to a demoted writer are still closed when topology lags after a failover.

#1287: host_monitoring_v2 monitors per event loop (c612461)

_cleanup_idle_monitors() ran on every monitored statement and stopped any monitor that wasn't is_usable(), which was False whenever the monitor ran on a different loop from the caller's. With one event loop per thread, loops stopped each other's monitors on every statement, recreating monitors and monitoring connections and leaving in-flight statements unmonitored.

  • The registry is keyed by (monitor key, event loop); is_usable() is False only when the monitor is stopped, its task has ended, or its loop is closed; the registry is guarded by a lock, like sync's monitor service.
  • An app with N event loops holds N monitoring connections per monitored host. One per loop is required because a monitor aborts connections that belong to its loop.

#1288: host_monitoring_v2 without a topology plugin on a cluster endpoint (26fa22c)

_build_host_list_provider only chose a topology provider for failover-type plugins. With host_monitoring_v2 alone on an Aurora cluster endpoint, identify_connection returned None and the first statement on every connection raised AwsWrapperError; the plugin had already cached the cluster endpoint, so later statements monitored the cluster DNS name.

  • aio/wrapper.py: host_monitoring and host_monitoring_v2 get a topology provider when the database dialect is a TopologyAwareDatabaseDialect (Aurora, Multi-AZ, Global), as sync does for those dialects. Plain PostgreSQL/MySQL/RDS keep the static provider and pay nothing extra.
  • aio/host_monitoring_plugin.py: _get_monitoring_host_info only caches the monitoring host once it is known. Sync already behaves this way (it overwrites the cached value with identify_connection's None).

Testing

Separate gap, not changed here: async blue/green doesn't stop the topology monitor on switchover, while sync does (blue_green_plugin.py:908).

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Every AsyncAwsWrapperConnection.connect() builds its own host list
provider, and each provider created its own AsyncClusterTopologyMonitor
with its own dedicated monitoring connection. A pool of N connections
to one cluster held 2N server connections. Sync shares one monitor per
cluster id through monitor_service.run_if_absent.

Keep async topology monitors in a module-level registry keyed by
(provider class, cluster id, event loop). Providers for the same cluster
reuse the running monitor, and the monitor publishes each topology into
every provider that uses it. Lifecycle matches sync: closing a
connection doesn't stop the monitor; stop() stops it for the cluster,
like RdsHostListProvider.stop_monitor, and release_resources_async()
stops it at shutdown.

Fixes aws#1284
…_connection_tracker

The connect-time live-probe pin used current_host_info, which for a
cluster-endpoint connection is the cluster endpoint. The first execute
compared it with the topology writer (an instance endpoint), saw a writer
change, and invalidate_all closed the connection about to run its
statement.

Remember the instance a cluster-endpoint connection landed on and pin
that, as sync does by always taking the writer from topology. The
departed-host failover path resolves a cluster endpoint to the same
instance, so idle connections to a demoted writer are still closed when
topology lags.

Fixes aws#1276
Every monitored statement ran _cleanup_idle_monitors, which stopped any
monitor that wasn't is_usable(), and is_usable() was False whenever the
monitor ran on a different loop from the caller's. With one event loop
per thread, each loop stopped the other loops' monitors on every
statement: monitors and their monitoring connections were recreated
continuously, and in-flight statements on the other loop lost failure
detection.

Key the registry by monitor key and event loop, so each loop reuses its
own monitor, and treat a monitor as unusable only when it is stopped, its
task has finished, or its loop is closed. Guard the registry with a lock,
as sync's monitor service does.

Fixes aws#1287
…dialects

host_monitoring(_v2) identifies the instance behind a cluster endpoint
through the topology, but _build_host_list_provider only chose a topology
provider for failover-type plugins. With host_monitoring_v2 alone on an
Aurora cluster endpoint, identify_connection returned None and the first
statement on every connection raised AwsWrapperError. The plugin had
already cached the cluster endpoint, so later statements monitored the
cluster DNS name instead of the instance.

Choose a topology provider for host_monitoring(_v2) when the database
dialect is topology-aware (Aurora, Multi-AZ, Global), as sync does for
those dialects; plain PostgreSQL/MySQL/RDS keep the static provider. Only
cache the monitoring host once it is known, as sync does.

Fixes aws#1288
Keep only comments that explain non-obvious choices, drop issue references and history, and rename _TOPOLOGY_ON_TOPOLOGY_DIALECT_PLUGINS to say what it holds: plugins that use the topology when the dialect has one.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment