marin-ma commented on code in PR #11083:
URL:
https://github.com/apache/incubator-gluten/pull/11083#discussion_r2537229964
##########
backends-velox/src/main/scala/org/apache/spark/shuffle/ColumnarShuffleWriter.scala:
##########
@@ -203,7 +205,7 @@ class ColumnarShuffleWriter[K, V](
val columnarBatchHandle =
ColumnarBatches.getNativeHandle(BackendsApiManager.getBackendName,
cb)
val startTime = System.nanoTime()
- shuffleWriterJniWrapper.write(
+ val curBytesWritten = shuffleWriterJniWrapper.write(
Review Comment:
https://github.com/apache/incubator-gluten/blob/main/cpp/core/jni/JniWrapper.cc#L1000-L1002
Looks like the returned value from each call is the size of the input batch,
but looks like here it's treated as an accumulated value. I wonder if this
should be
```
val bytesWritten = shuffleWriterJniWrapper.write(...)
writeMetrics.incBytesWritten(bytesWritten)
```
##########
cpp/core/shuffle/PartitionWriter.h:
##########
@@ -48,7 +48,12 @@ class PartitionWriter : public Reclaimable {
~PartitionWriter() override = default;
- virtual arrow::Status stop(ShuffleWriterMetrics* metrics) = 0;
+ void acceptMetrics(ShuffleWriterMetrics* metrics) {
Review Comment:
Why do we need this change?
--
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]
