-
Notifications
You must be signed in to change notification settings - Fork 447
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[VL] Decouple ShuffleWriter and PartitionWriter #3932
Conversation
Thanks for opening a pull request! Could you open an issue for this pull request on Github Issues? https://github.com/oap-project/gluten/issues Then could you also rename commit message and pull request title in the following format?
See also: |
eca7962
to
e7430aa
Compare
ready to review? |
@FelixYBW Yes. This PR is ready for review. |
/Benchmark Velox |
===== Performance report for TPCH SF2000 with Velox backend, for reference only ====
|
3c42026
to
a7c9e01
Compare
/Benchmark Velox |
===== Performance report for TPCH SF2000 with Velox backend, for reference only ====
|
/Benchmark Velox |
===== Performance report for TPCH SF2000 with Velox backend, for reference only ====
|
Resolved by #4099 |
In the previous implementation, The life cycle of
PartitionWriter
is managed byShuffleWriter
. However,PartitionWriter
also holds the pointer toShuffleWriter
and manipulates its internal state, which is considered as bad design. This PR removes the raw pointer ofShuffleWriter
inPartitionWrtier
.Detailed changes:
ShuffleWriter::Stop
, it will callPartitionWriter::Stop
to merge spilled files and flush cached payloads, and calls backShuffleWriter
to write in-memory split buffers. After this change,ShuffleWrtier
will transfer the ownership of split buffers toPartitionWriter
at the beginning ofShuffleWriter::Stop
, so the callback can be removed.PartitionWriter
will updateShuffleWriter
metrics. This PR addsShuffleWriterMetrics
andPartitionWriter::populateMetrics
to update metrics data only withShuffleWriterMetrics
instance.With above changes, the raw pointer of
ShuffleWriter
can be removed inPartitionWriter
.