From fdb81537a2f854cf5e2b9dd95c7e5542bb5cd420 Mon Sep 17 00:00:00 2001 From: Paul Dennis Date: Mon, 29 Jul 2024 16:43:02 -0400 Subject: [PATCH] Attempt to fix SFTP issue #636 Too many out-of-order packets. lftp supports making many SFTP DATA requests in parallel when downloading a file. Responses with data payloads from an sftp server can sometimes arrive in a different order from the original requests. The out of order data payload (packets) are added to the `ooo_chain` to be processed when the expected (but late) next requested packet arrives. A single packet being delayed while many parallel requests continue to be made can sometimes result in the `Too many out-of-order packets` message if the `ooo_chain` buffer of 64 entries is exceeded. This fix checks the remaining capacity of the `ooo_chain` and pending requests (`RespQueueSize()`) and will avoid sending new requests for more data until the `ooo_chain` is processed or `RespQueueSize()` shrinks. Adding this constraint does mean that `sftp:max-packets-in-flight` will be limited to the current `ooo_chain` length of 64. I was able to reliably reproduce the issue with a 4gb download and verify the fix. Conflict:NA Reference:https://github/lavv17/lftp/commit/fdb81537a2f854cf5e2b9dd95c7e5542bb5cd420 --- src/SFtp.cc | 8 ++++++-- src/SFtp.h | 1 + 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/SFtp.cc b/src/SFtp.cc index 6c818dfa1..5879a6314 100644 --- a/src/SFtp.cc +++ b/src/SFtp.cc @@ -344,6 +344,7 @@ void SFtp::Init() size_write=0x8000; use_full_path=false; flush_timer.Set(0,500); + max_out_of_order=64; } SFtp::SFtp() : SSH_Access("SFTP:") @@ -892,7 +893,7 @@ void SFtp::HandleExpect(Expect *e) { LogNote(9,"put a packet with id=%d on out-of-order chain (need_pos=%lld packet_pos=%lld)", reply->GetID(),(long long)(pos+file_buf->Size()),(long long)r->pos); - if(ooo_chain.count()>=64) + if(ooo_chain.count()>=max_out_of_order) { LogError(0,"Too many out-of-order packets"); Disconnect(); @@ -1192,7 +1193,10 @@ int SFtp::Read(Buffer *buf,int size) { // keep some packets in flight. int limit=(entity_size>=0?max_packets_in_flight:max_packets_in_flight_slow_start); - if(RespQueueSize()Eof()) + int ooo_queue_available=max_out_of_order-ooo_chain.count(); + int current_in_flight=RespQueueSize(); + if(RespQueueSize()Eof() + && current_in_flight < ooo_queue_available) { // but don't request much after possible EOF. if(entity_size<0 || request_pos