andygrove opened a new pull request, #2234:
URL: https://github.com/apache/datafusion-ballista/pull/2234

   # Which issue does this PR close?
   
   Backport of #2069 to `branch-54`. The issue it fixes is #2068.
   
   # Rationale for this change
   
   `client_ttl` defaults to `0`, which disables the executor's Ballista client 
pool entirely. With no pool, the shuffle-read path opens and discards a new TCP 
connection per fetch, so a single shuffle-heavy query exhausts the host's 
ephemeral port range and fails with `AddrNotAvailable`, as does every query 
after it, until those sockets leave `TIME_WAIT`.
   
   A single executor never shows this: shuffle reads are served from local 
files, so there are no remote fetches. It appears as soon as a second executor 
exists, which is every real deployment.
   
   Measured in the original PR on the SF1 TPC-DS correctness gate, 1 scheduler 
+ 2 executors (`-c 4` each), `--partitions 16`, everything else default:
   
   | | before | after |
   |---|---:|---:|
   | peak `TIME_WAIT` | 16343 | 12 |
   | full gate | 11 OK / 77 FAILED | 88 OK / 0 FAILED |
   
   The test host's ephemeral range is 16384 ports, so a single query consumed 
all of it.
   
   `branch-54` carries the same default, so every multi-executor deployment on 
the release branch is exposed unless the operator happens to know to pass 
`--client-ttl`.
   
   # What changes are included in this PR?
   
   A clean cherry-pick of 66af2392a, unmodified.
   
   - Default `client_ttl` to 30 seconds (was 0) so the pool is on unless 
explicitly disabled. `--client-ttl 0` still opts out.
   - Update the flag help and the `ExecutorProcessConfig::client_ttl` doc to 
say what `0` actually costs.
   - Add a test pinning the default to a pooling value, so it cannot regress to 
the disabled state unnoticed.
   
   # Are there any user-facing changes?
   
   The default value of `--client-ttl` changes from `0` to `30` seconds. No 
public API, config key, or proto/wire format changes, so this is not a breaking 
change. Deployments that already pass `--client-ttl` explicitly are unaffected.
   
   ---
   
   Verified locally on the `branch-54` base: `cargo fmt --all -- --check` is 
clean, and `cargo check --workspace --all-targets --locked` completes with no 
warnings on a combined stack of the six backports being proposed together. Test 
execution is left to CI.
   


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

Reply via email to