Repository navigation
Fix SSH commands failing when the task holds over 1024 file descriptors - #74211
Conversation
select.select() cannot watch a descriptor numbered 1024 or higher, so SSHHook.exec_ssh_client_command failed with "filedescriptor out of range in select()" once the task process had that many descriptors open, whatever its open-file limit. Task SDK workers reach that count far more easily than Airflow 2 task processes did. closes: apache#74205
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
The regression test generates the server's key itself, so the client can trust exactly that key and keep paramiko's default rejection of unknown host keys rather than accepting whatever key it is offered.
|
@firasbouzazi I think the issue with RSA is not so much speed, like the fact it's considered outdated. I think ECDSA is fine since paramiko can't generate Ed25519, or better yet: MLKEM via cryptography I don't think it matters much for a test, unless we're talking a multi-minute penalty. So just go with the fastest algo. |
RSA is considered outdated, and ECDSA keys are also the fastest paramiko can generate.
Done :) @Dev-iL |
potiuk
left a comment
potiuk
left a comment
There was a problem hiding this comment.
Approving. Moving from select() to selectors.DefaultSelector fixes the FD_SETSIZE limit without changing the read and shutdown logic, and the in-process server test shows it with real descriptors above 1024.
One follow-up: providers/ssh/tunnel.py (_serve_forever) has the same select() call. There the ValueError is caught and the forwarding thread just exits, so with more than 1024 descriptors open, SSHHook.get_tunnel() leaves a local port that accepts connections but never forwards them. Since channels come and go in that loop, it needs register/unregister calls rather than a drop-in swap. Would you be up for a follow-up PR?
Drafted-by: Claude Code (Opus 5.5); reviewed by @potiuk before posting
|
Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions. |
The port-forwarding loop in SSHTunnel._serve_forever watched its sockets and channels with select.select(), which cannot handle a file descriptor numbered >= FD_SETSIZE (1024) and raises 'filedescriptor out of range in select()'. That ValueError was caught and the forwarding thread exited, leaving a local port that accepts connections but never forwards them. Task SDK workers reach a high descriptor count easily. Replace select.select() with selectors.DefaultSelector (epoll/poll), which has no FD_SETSIZE ceiling. The watched set is rebuilt each loop iteration (mirroring the previous select() behaviour) because forwarded channels come and go between iterations. Add regression tests: a single high-fd forwarding check and an end-to-end concurrent/sequential forwarding test over a real SSH server under a high fd count. Both fail on the old select() implementation. Follow-up to apache#74211 (same select() ceiling in SSHHook).
Sure :) |
4 participants
select.select() cannot watch a descriptor numbered 1024 or higher, so SSHHook.exec_ssh_client_command failed with "filedescriptor out of range in select()" once the task process had that many descriptors open, whatever its open-file limit. Task SDK workers reach that count far more easily than Airflow 2 task processes did.
closes: #74205
Was generative AI tooling used to co-author this PR?
{pr_number}.significant.rst, in airflow-core/newsfragments. You can add this file in a follow-up commit after the PR is created so you know the PR number.