pjfanning opened a new pull request, #955: URL: https://github.com/apache/pekko-management/pull/955
### Motivation Follow-up to #907, which added `CoordinatedShutdown` client-close tasks to the **deprecated** `pekko-discovery-aws-api` module but left `pekko-discovery-aws-api-async` — the module the deprecation warning tells users to migrate to — untouched. Both `AsyncEcsServiceDiscovery` and `AsyncEcsTaskSetDiscovery` build an `EcsAsyncClient` and never close it. The leak is bigger here than the one that was fixed. `AsyncEcsServiceDiscovery` does: ```scala val httpClient = NettyNioAsyncHttpClient.create() EcsAsyncClient.builder().overrideConfiguration(conf).httpClient(httpClient).build() ``` In AWS SDK v2, a client passed to `.httpClient(...)` is treated as caller-owned and wrapped in `NonManagedSdkAsyncHttpClient`, whose `close()` is a no-op. So the Netty event loop threads would survive even if the ECS client *were* closed. `.httpClientBuilder(...)` is the form that hands ownership to the SDK. ### Modification - Both classes get an `AtomicReference` populated only **after** the client is built, plus a `PhaseServiceUnbind` task that closes whatever client it finds there and returns `Done` when it is null — so shutdown never forces, or re-attempts, the lazy initialisation. (Same shape as #954, which fixes that ordering bug in the v1 module.) - The close runs on `pekko.actor.default-blocking-io-dispatcher`, since closing a Netty-backed client blocks until its event loops have shut down. - `AsyncEcsServiceDiscovery` switches to `.httpClientBuilder(NettyNioAsyncHttpClient.builder())` so the SDK owns the Netty client and shuts its event loops down on `close()`. - Client construction moves behind `private[ecs] def createEcsClient()`, so the spec can supply a stub without AWS credentials, a region, or network access. Both classes are `@ApiMayChange` and non-final; MiMa is clean. - Docs note that the clients are created lazily and closed at `service-unbind`. ### Result Terminating the `ActorSystem` releases the ECS clients and their Netty event loop threads. Discovery that is configured but never used still creates no client. ### Tests - `sbt "discovery-aws-api-async/test"` — 4 succeeded, 0 failed. New `AsyncEcsClientShutdownSpec`; this module had no test sources at all before this PR. - Directionality: the two "close the ECS client on coordinated shutdown" cases assert `close()` was called, which nothing on `main` does — there is no shutdown task there. I could not run them verbatim against `main` because the `createEcsClient` seam they stub does not exist there. - `sbt "discovery-aws-api-async/mimaReportBinaryIssues"` — success. - `sbt "discovery-aws-api-async/scalafmt" "discovery-aws-api-async/Test/scalafmt"` and `sbt headerCreateAll` — clean. - Not exercised against live AWS — no account available. The spec stubs `EcsAsyncClient` so no credentials, region or network access are needed. ### References Refs #907, Refs #954 -- 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]
