sunchao commented on PR #25428:
URL: https://github.com/apache/datafusion/pull/25428#issuecomment-5744209141

   Follow-up to [the 
review](https://github.com/apache/datafusion/pull/25428#pullrequestreview-5254853397):
 pushed 
[0ee7a4d06](https://github.com/apache/datafusion/commit/0ee7a4d06aad89bd699059d9e0cf8a613120a7dc)
 and replied to the inline threads.
   
   Widening now trades read-ahead for inputs within the original reservation, 
so it cannot take the additional pool capacity that the narrow selection left 
for peer replay. Case G again uses default fan-in. The deterministic old-head 
probe reproduces a peer's 4 KiB allocation failure; the revised selection 
preserves its budget. Conditional source/output budgeting also removes the 
full-input batch fragmentation: the 8 × 10 × 8,192-row case now produces 80 
full batches instead of 150. The short-input case produces 38 batches instead 
of 80. Retry errors retain the original cause, and the reuse/cleanup comments 
are addressed.
   
   The requested **exact `b0752560` versus revised-head** benchmark is now in 
the description, including reproduction commands and data generation:
   
   | Fair pool | Exact base mean | Revised mean | Change |
   | --- | ---: | ---: | ---: |
   | 8 MiB | 754.17 ms | 816.45 ms | +8.3% |
   | 16 MiB | 758.83 ms | 748.56 ms | −1.4% |
   
   These are six fixed samples per cell on one development VM, timed after the 
other checks finished. The 8 MiB case is slower despite reducing spill files 
from 19 to 18; the 16 MiB difference is small. This evidence does **not** 
establish a general speedup. Results were exactly 2,000,000 and tracked pool 
peaks were unchanged and within both limits.
   
   Validation passed: 199 focused sort/merge tests; all four new seeded 
concurrent-aggregate cases; 160 uncapped Case G repetitions; 11,874 extended 
Rust tests (8 ignored); all 521 SQL files; formatting, strict 
all-target/all-feature Clippy, and the complete lint/docs suite. New CI is 
running on the pushed commit.
   
   I also corrected the dependency text: #25383 has merged, while #25172 was 
closed without merging. This update remains on the review's pinned base; it has 
not been retargeted to `main`.
   


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