stegololz opened a new pull request, #73288:
URL: https://github.com/apache/airflow/pull/73288

   `SFTPClientPool.get_sftp_client` yields the pooled client inside a bare
   `except Exception` and marks the connection faulty for any exception the 
caller's
   `async with` body raises, including SFTP status replies such as 
`SFTPNoSuchFile`
   or `SFTPPermissionDenied`.
   
   A status reply means the server answered over a live channel, so the 
connection is
   still healthy. Dropping it costs a reconnect and re-auth each time, and 
since the
   caller usually keeps iterating, one transient miss becomes self-sustaining.
   
   Observed on a production task iterating ~36,000 files over one pool:
   
   | phase | files | SSH connections |
   |---|---|---|
   | before first miss | 28,380 | 23 |
   | after first miss | 7,598 | 9,244, with 7,598 re-auths |
   
   The files were not missing; they read back fine afterwards. Each failure also
   logged a full traceback, producing 135-190 MB of task logs per attempt.
   
   Only `SFTPNoConnection`, `SFTPConnectionLost` and `SFTPBadMessage` now count 
as
   transport faults. Every other `SFTPError` returns the connection to the pool
   intact. Non-`SFTPError` exceptions and `CancelledError` keep the previous
   behaviour and still drop it. The exception always propagates unchanged.
   
   The rule sits in a module-level `_is_connection_faulty` rather than inline 
in the
   `except` block because it is a pure function of the exception type: one
   parametrised test covers the whole `SFTPError` taxonomy with no pool, event 
loop
   or mocked connection to stand up. It also keeps `get_sftp_client` reading as
   acquire, classify, log, release, and puts the rule beside the
   `_TRANSPORT_SFTP_ERRORS` tuple it consults.
   
   The pool was introduced in #64465.
   
   Tested with `pytest providers/sftp/tests/unit/sftp/pools/test_sftp.py`: 32 
passed,
   covering the classifier table, a miss returning the connection to the pool, 
five
   consecutive misses reusing the same client without reconnecting, and the 
transport
   and non-`SFTPError` cases still dropping it.
   
   ---
   
   ##### Was generative AI tooling used to co-author this PR?
   
   - [X] Yes (please specify the tool below)
   
   Generated-by: Claude Code (Opus 5) following [the 
guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)
   


-- 
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]

Reply via email to