You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@gluten.apache.org by "exmy (via GitHub)" <gi...@apache.org> on 2024/07/31 08:28:18 UTC
[PR] [GLUTEN-6656][CELEBORN] Fix CelebornColumnarShuffleWriter assertion failed [incubator-gluten]
exmy opened a new pull request, #6657:
URL: https://github.com/apache/incubator-gluten/pull/6657
## What changes were proposed in this pull request?
After https://github.com/apache/incubator-gluten/pull/6432, if all of the ColumnarBatch have empty rows in `CHCelebornColumnarShuffleWriter` or `VeloxCelebornColumnarShuffleWriter`, the `nativeShuffleWriter` won't be initialized and causes `assertion failed`.
(Fixes: \#6656)
## How was this patch tested?
PASS 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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-6656][CELEBORN] Fix CelebornColumnarShuffleWriter assertion failed [incubator-gluten]
Posted by "SteNicholas (via GitHub)" <gi...@apache.org>.
SteNicholas commented on code in PR #6657:
URL: https://github.com/apache/incubator-gluten/pull/6657#discussion_r1698444933
##########
gluten-celeborn/clickhouse/src/main/scala/org/apache/spark/shuffle/CHCelebornColumnarShuffleWriter.scala:
##########
@@ -71,7 +71,12 @@ class CHCelebornColumnarShuffleWriter[K, V](
}
}
- assert(nativeShuffleWriter != -1L)
+ // If all of the ColumnarBatch have empty rows, the nativeShuffleWriter still equals -1
Review Comment:
When `!records.hasNext=true`, does this also need to invoke `handleEmptyIterator`?
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-6656][CELEBORN] Fix CelebornColumnarShuffleWriter assertion failed [incubator-gluten]
Posted by "SteNicholas (via GitHub)" <gi...@apache.org>.
SteNicholas commented on PR #6657:
URL: https://github.com/apache/incubator-gluten/pull/6657#issuecomment-2260428974
@exmy, without #6432, `VeloxCelebornColumnarShuffleWriter` has the same problem, right?
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-6656][CELEBORN] Fix CelebornColumnarShuffleWriter assertion failed [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #6657:
URL: https://github.com/apache/incubator-gluten/pull/6657#issuecomment-2260048311
https://github.com/apache/incubator-gluten/issues/6656
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-6656][CELEBORN] Fix CelebornColumnarShuffleWriter assertion failed [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #6657:
URL: https://github.com/apache/incubator-gluten/pull/6657#issuecomment-2260048764
Run Gluten Clickhouse 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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-6656][CELEBORN] Fix CelebornColumnarShuffleWriter assertion failed [incubator-gluten]
Posted by "taiyang-li (via GitHub)" <gi...@apache.org>.
taiyang-li merged PR #6657:
URL: https://github.com/apache/incubator-gluten/pull/6657
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-6656][CELEBORN] Fix CelebornColumnarShuffleWriter assertion failed [incubator-gluten]
Posted by "exmy (via GitHub)" <gi...@apache.org>.
exmy commented on code in PR #6657:
URL: https://github.com/apache/incubator-gluten/pull/6657#discussion_r1699283944
##########
gluten-celeborn/clickhouse/src/main/scala/org/apache/spark/shuffle/CHCelebornColumnarShuffleWriter.scala:
##########
@@ -71,7 +71,12 @@ class CHCelebornColumnarShuffleWriter[K, V](
}
}
- assert(nativeShuffleWriter != -1L)
+ // If all of the ColumnarBatch have empty rows, the nativeShuffleWriter still equals -1
Review Comment:
Yes，the logic of invoking `handleEmptyIterator` when `!records.hasNext=true` is in the `wtite` method of super class `CelebornColumnarShuffleWriter`.
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-6656][CELEBORN] Fix CelebornColumnarShuffleWriter assertion failed [incubator-gluten]
Posted by "exmy (via GitHub)" <gi...@apache.org>.
exmy commented on code in PR #6657:
URL: https://github.com/apache/incubator-gluten/pull/6657#discussion_r1699283944
##########
gluten-celeborn/clickhouse/src/main/scala/org/apache/spark/shuffle/CHCelebornColumnarShuffleWriter.scala:
##########
@@ -71,7 +71,12 @@ class CHCelebornColumnarShuffleWriter[K, V](
}
}
- assert(nativeShuffleWriter != -1L)
+ // If all of the ColumnarBatch have empty rows, the nativeShuffleWriter still equals -1
Review Comment:
Yes，the logic of invoking `handleEmptyIterator` when `!records.hasNext=true` is in the `write` method of super class `CelebornColumnarShuffleWriter`.
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-6656][CELEBORN] Fix CelebornColumnarShuffleWriter assertion failed [incubator-gluten]
Posted by "exmy (via GitHub)" <gi...@apache.org>.
exmy commented on PR #6657:
URL: https://github.com/apache/incubator-gluten/pull/6657#issuecomment-2261811541
> @exmy, `VeloxCelebornColumnarShuffleWriter` has the same problem without #6432, right? If yes, please update the description of this pull request.
Done.
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org