Simplify Exchange interfaces#13945
Conversation
arhimondr
force-pushed
the
remove-some-exchange-methods
branch
from
61d0a3a to
0e3b077
Compare
arhimondr
force-pushed
the
remove-some-exchange-methods
branch
from
0e3b077 to
2c5a5b5
Compare
There was a problem hiding this comment.
I fixed it to be called only once per handle. Caching is not ideal as it would use an extra memory. Ideally it shouldn't be called more than once.
There was a problem hiding this comment.
I am not convinced that we should drop this one. It make the SPI contract not obvious. What is the expected size of source handles to be returned by getSourceHandles now. What if exchange implementation can split the data into arbitrarly small chunks. Should it return 1M handles each serving 10 bytes of data or 1K each serving 10K?
If we drop split maybe we should pass a hint targetSourceHandleSizeInBytes to getSourceHandles?
losipiuk
left a comment
losipiuk
left a comment
There was a problem hiding this comment.
removal of statistics fine.
Regarding split lets talk.
…shed Added for testing, committed by mistake
In favor of ExchangeSourceHandle#getDataSizeInBytes The additional indirection is inconvenient as it requires the Exchange object to be available to simply get the size of data referenced by a certain ExchangeSourceHandle. In practice this information is either available in ExchangeSourceHandle internally or can easily be made available.
In favor of creating smaller ExchangeSourceHandle's. It is possible to create more than a single ExchangeSourceHandle per partition. Instead of splitting an existing ExchangeSourceHandle it is more natural not to create too large ExchangeSourceHandle in first place.
arhimondr
force-pushed
the
remove-some-exchange-methods
branch
from
2c5a5b5 to
30da438
Compare
|
@losipiuk Updated |
3 participants
Description
Remove
Exchange#getExchangeSourceStatisticsandExchange#split methodmethods. See commit messages for more details.Refactoring
Exchange
N/A
Related issues, pull requests, and links
Documentation
(x) No documentation is needed.
( ) Sufficient documentation is included in this PR.
( ) Documentation PR is available with #prnumber.
( ) Documentation issue #issuenumber is filed, and can be handled later.
Release notes
(x) No release notes entries required.
( ) Release notes entries required with the following suggested text: