verdier commented on PR #42629: URL: https://github.com/apache/superset/pull/42629#issuecomment-5250016474
Hi @goingforstudying-ctrl — heads up so you hear it from me rather than find it: I opened #43007 for #42622 without having seen this PR, which predates mine by ten days and reaches the same diagnosis. @rusackas asked us to compare notes before either lands, so I backported your approach onto a Superset 6.1 harness and measured both under identical conditions. Full write-up is in [this comment](https://github.com/apache/superset/pull/43007#issuecomment-5250013872); the short version: - both fix it: 10/10 concurrent `generate_chart` calls succeed, ten distinct sessions for ten tasks, against 2/10 and one shared session on master - both hit the same cliff at `pool_size + max_overflow`: 15/16 at concurrency 16 with a `QueuePool ... timeout 30.00`, wall clock within run-to-run variance of each other So the pool consequence is a property of isolating sessions at all, not of how — whichever lands needs a concurrency guard, and I will write one that applies to either. Two things I flagged as costs of your approach, in case they are useful rather than annoying: the install in `create_mcp_app` sits in a `try/except ImportError` that only logs at debug, so a serving path missing all three install points keeps the bug silently; and `("mcp_tool_call", id(token))` uses a reusable address as the registry key, so an entry that is not removed could be inherited by a later call. I also flagged, on my own PR, that your `_remove_session_safe()` on the request-context path closes a registry leak that mine has and yours does not — if mine is the one that lands, I would borrow that with credit. One caveat: backporting yours to 6.1 needed an import fix because 6.1 imports `has_app_context` inside `mcp_auth_hook` while master has it at module level. That is my backport, not your PR. No stake in which one lands — happy to close mine and bring the measurements and the guard here if the maintainers prefer yours. -- 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]
