You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@gluten.apache.org by "yuanhang-dev (via GitHub)" <gi...@apache.org> on 2025/07/01 06:59:07 UTC
[PR] [GLUTEN-10091][FLINK] Fix NexmarkTest to catch exceptions thrown by velox [incubator-gluten]
yuanhang-dev opened a new pull request, #10092:
URL: https://github.com/apache/incubator-gluten/pull/10092
## What changes were proposed in this pull request?
(Please fill in changes proposed in this fix)
(Fixes: \#ISSUE-ID)
## How was this patch tested?
(Please explain how this patch was tested. E.g. unit tests, integration tests, manual tests)
(If this patch involves UI changes, please attach a screenshot; otherwise, remove this)
--
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-10091][FLINK] Fix premature termination in NexmarkTest [incubator-gluten]
Posted by "PHILO-HE (via GitHub)" <gi...@apache.org>.
PHILO-HE merged PR #10092:
URL: https://github.com/apache/incubator-gluten/pull/10092
--
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-10091][FLINK] Fix NexmarkTest to catch exceptions thrown by velox [incubator-gluten]
Posted by "yuanhang-dev (via GitHub)" <gi...@apache.org>.
yuanhang-dev commented on code in PR #10092:
URL: https://github.com/apache/incubator-gluten/pull/10092#discussion_r2218416952
##########
gluten-flink/ut/src/test/java/org/apache/gluten/table/runtime/stream/custom/NexmarkTest.java:
##########
@@ -127,6 +130,19 @@ private void executeQuery(StreamTableEnvironment tEnv, String queryFileName) {
if (!insertQuery.isEmpty()) {
TableResult insertResult = tEnv.executeSql(insertQuery);
assertThat(insertResult.getJobClient().isPresent()).isTrue();
Review Comment:
Fixed
--
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-10091][FLINK] Fix NexmarkTest to catch exceptions thrown by velox [incubator-gluten]
Posted by "PHILO-HE (via GitHub)" <gi...@apache.org>.
PHILO-HE commented on code in PR #10092:
URL: https://github.com/apache/incubator-gluten/pull/10092#discussion_r2218291063
##########
gluten-flink/ut/src/test/java/org/apache/gluten/table/runtime/stream/custom/NexmarkTest.java:
##########
@@ -127,6 +130,19 @@ private void executeQuery(StreamTableEnvironment tEnv, String queryFileName) {
if (!insertQuery.isEmpty()) {
TableResult insertResult = tEnv.executeSql(insertQuery);
assertThat(insertResult.getJobClient().isPresent()).isTrue();
Review Comment:
In line 126, should `waitForJobCompletion` also be called to wait for the table creation.
##########
gluten-flink/ut/src/test/java/org/apache/gluten/table/runtime/stream/custom/NexmarkTest.java:
##########
@@ -127,6 +130,19 @@ private void executeQuery(StreamTableEnvironment tEnv, String queryFileName) {
if (!insertQuery.isEmpty()) {
TableResult insertResult = tEnv.executeSql(insertQuery);
assertThat(insertResult.getJobClient().isPresent()).isTrue();
Review Comment:
Move into waitForJobCompletion. And use assertTrue for simplicity.
##########
gluten-flink/ut/src/test/java/org/apache/gluten/table/runtime/stream/custom/NexmarkTest.java:
##########
@@ -127,6 +130,19 @@ private void executeQuery(StreamTableEnvironment tEnv, String queryFileName) {
if (!insertQuery.isEmpty()) {
TableResult insertResult = tEnv.executeSql(insertQuery);
assertThat(insertResult.getJobClient().isPresent()).isTrue();
+ try {
+ waitForJobCompletion(insertResult, 30000);
+ } catch (InterruptedException | ExecutionException | TimeoutException e) {
+ throw new RuntimeException("Query execution failed: " + queryFileName, e);
+ }
+ }
+ }
+
+ private void waitForJobCompletion(TableResult result, long timeoutMs)
+ throws InterruptedException, ExecutionException, TimeoutException {
+ if (result.getJobClient().isPresent()) {
Review Comment:
Looks this must true. Then, just add an assert check. And remove the if check.
--
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-10091][FLINK] Fix NexmarkTest to catch exceptions thrown by velox [incubator-gluten]
Posted by "yuanhang-dev (via GitHub)" <gi...@apache.org>.
yuanhang-dev commented on PR #10092:
URL: https://github.com/apache/incubator-gluten/pull/10092#issuecomment-3043907948
@PHILO-HE
--
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-10091][FLINK] Fix NexmarkTest to catch exceptions thrown by velox [incubator-gluten]
Posted by "yuanhang-dev (via GitHub)" <gi...@apache.org>.
yuanhang-dev commented on code in PR #10092:
URL: https://github.com/apache/incubator-gluten/pull/10092#discussion_r2218491874
##########
gluten-flink/ut/src/test/java/org/apache/gluten/table/runtime/stream/custom/NexmarkTest.java:
##########
@@ -127,6 +130,19 @@ private void executeQuery(StreamTableEnvironment tEnv, String queryFileName) {
if (!insertQuery.isEmpty()) {
TableResult insertResult = tEnv.executeSql(insertQuery);
assertThat(insertResult.getJobClient().isPresent()).isTrue();
Review Comment:
No need. First of all, the Create Table operation is very quick and doesn't require waiting. Just need to ensure the correctness of the insert process, avoiding situations where the insert hasn't finished but an exception is thrown and the test has already ended.
--
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-10091][FLINK] Fix NexmarkTest to catch exceptions thrown by velox [incubator-gluten]
Posted by "yuanhang-dev (via GitHub)" <gi...@apache.org>.
yuanhang-dev commented on PR #10092:
URL: https://github.com/apache/incubator-gluten/pull/10092#issuecomment-3095067235
I have fixed the [comments](https://github.com/apache/incubator-gluten/pull/10125#discussion_r2215629664) here. Perhaps we can merge this PR first. @PHILO-HE
--
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-10091][FLINK] Fix NexmarkTest to catch exceptions thrown by velox [incubator-gluten]
Posted by "PHILO-HE (via GitHub)" <gi...@apache.org>.
PHILO-HE commented on code in PR #10092:
URL: https://github.com/apache/incubator-gluten/pull/10092#discussion_r2218480448
##########
gluten-flink/ut/src/test/java/org/apache/gluten/table/runtime/stream/custom/NexmarkTest.java:
##########
@@ -127,6 +130,19 @@ private void executeQuery(StreamTableEnvironment tEnv, String queryFileName) {
if (!insertQuery.isEmpty()) {
TableResult insertResult = tEnv.executeSql(insertQuery);
assertThat(insertResult.getJobClient().isPresent()).isTrue();
Review Comment:
Should `waitForJobCompletion` also be called in the code below?
https://github.com/apache/incubator-gluten/blob/6bf6934ca3b490297989da6a2eb36b2b5b654a5a/gluten-flink/ut/src/test/java/org/apache/gluten/table/runtime/stream/custom/NexmarkTest.java#L128
--
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-10091][FLINK] Fix NexmarkTest to catch exceptions thrown by velox [incubator-gluten]
Posted by "PHILO-HE (via GitHub)" <gi...@apache.org>.
PHILO-HE commented on code in PR #10092:
URL: https://github.com/apache/incubator-gluten/pull/10092#discussion_r2218508372
##########
gluten-flink/ut/src/test/java/org/apache/gluten/table/runtime/stream/custom/NexmarkTest.java:
##########
@@ -127,6 +130,19 @@ private void executeQuery(StreamTableEnvironment tEnv, String queryFileName) {
if (!insertQuery.isEmpty()) {
TableResult insertResult = tEnv.executeSql(insertQuery);
assertThat(insertResult.getJobClient().isPresent()).isTrue();
Review Comment:
ok. Use assertFalse for that line.
--
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-10091][FLINK] Fix NexmarkTest to catch exceptions thrown by velox [incubator-gluten]
Posted by "yuanhang-dev (via GitHub)" <gi...@apache.org>.
yuanhang-dev commented on code in PR #10092:
URL: https://github.com/apache/incubator-gluten/pull/10092#discussion_r2218516146
##########
gluten-flink/ut/src/test/java/org/apache/gluten/table/runtime/stream/custom/NexmarkTest.java:
##########
@@ -127,6 +130,19 @@ private void executeQuery(StreamTableEnvironment tEnv, String queryFileName) {
if (!insertQuery.isEmpty()) {
TableResult insertResult = tEnv.executeSql(insertQuery);
assertThat(insertResult.getJobClient().isPresent()).isTrue();
Review Comment:
Fixed
--
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-10091][FLINK] Fix NexmarkTest to catch exceptions thrown by velox [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #10092:
URL: https://github.com/apache/incubator-gluten/pull/10092#issuecomment-3022155330
https://github.com/apache/incubator-gluten/issues/10091
--
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