You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "caiconghui (Code Review)" <ge...@cloudera.org> on 2017/08/21 07:31:38 UTC
[kudu-CR] add the wrapper for the Iterator[Row] of KuduRDD by using Spark InterruptibleIterator[Row]
caiconghui has uploaded a new change for review.
http://gerrit.cloudera.org:8080/7753
Change subject: add the wrapper for the Iterator[Row] of KuduRDD by using Spark InterruptibleIterator[Row]
......................................................................
add the wrapper for the Iterator[Row] of KuduRDD by using Spark InterruptibleIterator[Row]
once we the submit a spark job to spark cluster, we cannot cancel or stop the kudu-spark
job immediately before using the InterruptibleIterator[Row]. Aftering using InterruptibleIterator[Row],
we can kill the job immediately by clicking the "kill" link on the spark Web UI.
Change-Id: I0b4284f2c0a40cd7ba8cf2b76e0403592552c814
---
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduRDD.scala
1 file changed, 2 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/53/7753/1
--
To view, visit http://gerrit.cloudera.org:8080/7753
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0b4284f2c0a40cd7ba8cf2b76e0403592552c814
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: caiconghui <ca...@xiaomi.com>
[kudu-CR] feat: add the wrapper for the Iterator by using InterruptibleIterator
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: feat: add the wrapper for the Iterator by using InterruptibleIterator
......................................................................
Patch Set 2:
That's a good point about killTaskIfInterrupted being private to spark. I think there's a way to get the best of both worlds here:
TaskContext.isInterrupted appears to be stable, and we can use it once per row batch from Kudu to check and throw. The only downside is we don't get the 'reason' for the task interruption, but hopefully that will be logged elsewhere.
Also, please include a test for task cancellation so we can make sure this doesn't regress.
--
To view, visit http://gerrit.cloudera.org:8080/7753
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0b4284f2c0a40cd7ba8cf2b76e0403592552c814
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: caiconghui <ca...@xiaomi.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: caiconghui <ca...@xiaomi.com>
Gerrit-HasComments: No
[kudu-CR] feat: add task interruption checking for RowIterator
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: feat: add task interruption checking for RowIterator
......................................................................
Patch Set 4: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/7753
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0b4284f2c0a40cd7ba8cf2b76e0403592552c814
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: caiconghui <ca...@xiaomi.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: caiconghui <ca...@xiaomi.com>
Gerrit-HasComments: No
[kudu-CR] add the wrapper for the Iterator[Row] of KuduRDD by using Spark InterruptibleIterator[Row]
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: add the wrapper for the Iterator[Row] of KuduRDD by using Spark InterruptibleIterator[Row]
......................................................................
Patch Set 1:
The CI failure looks like a flaky test - I've retriggered it just to make sure.
--
To view, visit http://gerrit.cloudera.org:8080/7753
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0b4284f2c0a40cd7ba8cf2b76e0403592552c814
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: caiconghui <ca...@xiaomi.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: caiconghui <ca...@xiaomi.com>
Gerrit-HasComments: No
[kudu-CR] feat: add the wrapper for the Iterator by using InterruptibleIterator
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: feat: add the wrapper for the Iterator by using InterruptibleIterator
......................................................................
Patch Set 2:
What I had in mind was something like this:
override def hasNext: Boolean = {
while ((currentIterator != null && !currentIterator.hasNext && scanner.hasMoreRows) ||
(scanner.hasMoreRows && currentIterator == null)) {
if (TaskContext.get().isInterrupted()) {
throw new RuntimeException("KuduRDD task interrupted")
}
currentIterator = scanner.nextRows()
}
currentIterator.hasNext
}
--
To view, visit http://gerrit.cloudera.org:8080/7753
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0b4284f2c0a40cd7ba8cf2b76e0403592552c814
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: caiconghui <ca...@xiaomi.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: caiconghui <ca...@xiaomi.com>
Gerrit-HasComments: No
[kudu-CR] feat: add the wrapper for the Iterator by using InterruptibleIterator
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: feat: add the wrapper for the Iterator by using InterruptibleIterator
......................................................................
Patch Set 2:
Maybe we should discuss this on the spark-dev mailing list? I don't feel great about depending on a private API of theirs without first at least asking their permission/recommendation. Maybe they should be opening this API to be public?
--
To view, visit http://gerrit.cloudera.org:8080/7753
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0b4284f2c0a40cd7ba8cf2b76e0403592552c814
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: caiconghui <ca...@xiaomi.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: caiconghui <ca...@xiaomi.com>
Gerrit-HasComments: No
[kudu-CR] feat: add task interruption checking for RowIterator
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: feat: add task interruption checking for RowIterator
......................................................................
Patch Set 5:
Yep, looks like it was just a flaky test. Thanks caiconghui!
--
To view, visit http://gerrit.cloudera.org:8080/7753
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0b4284f2c0a40cd7ba8cf2b76e0403592552c814
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: caiconghui <ca...@xiaomi.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: caiconghui <ca...@xiaomi.com>
Gerrit-HasComments: No
[kudu-CR] feat: add task interruption checking for RowIterator
Posted by "caiconghui (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7753
to look at the new patch set (#4).
Change subject: feat: add task interruption checking for RowIterator
......................................................................
feat: add task interruption checking for RowIterator
Once we the submit a spark job to spark cluster, we cannot cancel or
stop the kudu-spark job immediately before checking task interruption,
we can kill the job immediately by clicking the "kill" link on
the spark Web UI.
Change-Id: I0b4284f2c0a40cd7ba8cf2b76e0403592552c814
---
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduRDD.scala
1 file changed, 3 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/53/7753/4
--
To view, visit http://gerrit.cloudera.org:8080/7753
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0b4284f2c0a40cd7ba8cf2b76e0403592552c814
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: caiconghui <ca...@xiaomi.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: caiconghui <ca...@xiaomi.com>
[kudu-CR] feat: add the wrapper for the Iterator by using InterruptibleIterator
Posted by "caiconghui (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7753
to look at the new patch set (#2).
Change subject: feat: add the wrapper for the Iterator by using InterruptibleIterator
......................................................................
feat: add the wrapper for the Iterator by using InterruptibleIterator
Once we the submit a spark job to spark cluster, we cannot cancel or
stop the kudu-spark job immediately before using the
InterruptibleIterator[Row].With using InterruptibleIterator[Row],
we can kill the job immediately by clicking the "kill" link on
the spark Web UI.
Change-Id: I0b4284f2c0a40cd7ba8cf2b76e0403592552c814
---
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduRDD.scala
1 file changed, 2 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/53/7753/2
--
To view, visit http://gerrit.cloudera.org:8080/7753
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0b4284f2c0a40cd7ba8cf2b76e0403592552c814
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: caiconghui <ca...@xiaomi.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: caiconghui <ca...@xiaomi.com>
[kudu-CR] feat: add the wrapper for the Iterator by using InterruptibleIterator
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: feat: add the wrapper for the Iterator by using InterruptibleIterator
......................................................................
Patch Set 2:
My only hesitation with TaskKilledException is that it's a developer API, so it's subject to change. I also can't find an API to actually do cancellation, so skipping the test is fine.
--
To view, visit http://gerrit.cloudera.org:8080/7753
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0b4284f2c0a40cd7ba8cf2b76e0403592552c814
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: caiconghui <ca...@xiaomi.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: caiconghui <ca...@xiaomi.com>
Gerrit-HasComments: No
[kudu-CR] feat: add task interruption checking for RowIterator
Posted by "caiconghui (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7753
to look at the new patch set (#3).
Change subject: feat: add task interruption checking for RowIterator
......................................................................
feat: add task interruption checking for RowIterator
Once we the submit a spark job to spark cluster, we cannot cancel or
stop the kudu-spark job immediately before checking task interruption,
we can kill the job immediately by clicking the "kill" link on
the spark Web UI.
Change-Id: I0b4284f2c0a40cd7ba8cf2b76e0403592552c814
---
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduRDD.scala
1 file changed, 5 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/53/7753/3
--
To view, visit http://gerrit.cloudera.org:8080/7753
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0b4284f2c0a40cd7ba8cf2b76e0403592552c814
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: caiconghui <ca...@xiaomi.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: caiconghui <ca...@xiaomi.com>
[kudu-CR] add the wrapper for the Iterator[Row] of KuduRDD by using Spark InterruptibleIterator[Row]
Posted by "caiconghui (Code Review)" <ge...@cloudera.org>.
caiconghui has posted comments on this change.
Change subject: add the wrapper for the Iterator[Row] of KuduRDD by using Spark InterruptibleIterator[Row]
......................................................................
Patch Set 1:
@Kudu Jenkins what't wrong with the new feature. it seems that the exception below is not caused by the new feature, this feature is just a wrapper of Row[Iterator]
/home/jenkins-slave/workspace/kudu-master/3/src/kudu/integration-tests/alter_table-randomized-test.cc:604
Failed
Bad status: Invalid argument: Some columns are not present in the current schema: c395, c817, c224
--
To view, visit http://gerrit.cloudera.org:8080/7753
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0b4284f2c0a40cd7ba8cf2b76e0403592552c814
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: caiconghui <ca...@xiaomi.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: caiconghui <ca...@xiaomi.com>
Gerrit-HasComments: No
[kudu-CR] feat: add the wrapper for the Iterator by using InterruptibleIterator
Posted by "caiconghui (Code Review)" <ge...@cloudera.org>.
caiconghui has posted comments on this change.
Change subject: feat: add the wrapper for the Iterator by using InterruptibleIterator
......................................................................
Patch Set 2:
@Dan Burkert Yeah, I think throw TaskKilledException("KuduRDD task interrupted") is more accurate, that is also what InterruptibleIterator throw in the latest spark version, and you said writing a test, but I cannot find a public api to kill task, so I haven't push the new patch yet
--
To view, visit http://gerrit.cloudera.org:8080/7753
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0b4284f2c0a40cd7ba8cf2b76e0403592552c814
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: caiconghui <ca...@xiaomi.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: caiconghui <ca...@xiaomi.com>
Gerrit-HasComments: No
[kudu-CR] feat: add task interruption checking for RowIterator
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has submitted this change and it was merged.
Change subject: feat: add task interruption checking for RowIterator
......................................................................
feat: add task interruption checking for RowIterator
Once we the submit a spark job to spark cluster, we cannot cancel or
stop the kudu-spark job immediately before checking task interruption,
we can kill the job immediately by clicking the "kill" link on
the spark Web UI.
Change-Id: I0b4284f2c0a40cd7ba8cf2b76e0403592552c814
Reviewed-on: http://gerrit.cloudera.org:8080/7753
Tested-by: Kudu Jenkins
Reviewed-by: Dan Burkert <da...@apache.org>
---
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduRDD.scala
1 file changed, 3 insertions(+), 0 deletions(-)
Approvals:
Dan Burkert: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/7753
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I0b4284f2c0a40cd7ba8cf2b76e0403592552c814
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: caiconghui <ca...@xiaomi.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: caiconghui <ca...@xiaomi.com>
[kudu-CR] feat: add the wrapper for the Iterator by using InterruptibleIterator
Posted by "caiconghui (Code Review)" <ge...@cloudera.org>.
caiconghui has posted comments on this change.
Change subject: feat: add the wrapper for the Iterator by using InterruptibleIterator
......................................................................
Patch Set 2:
@Kudu Jenkins in my practice, the InterruptibleIterator dosen't reduce a lot of efficiency. context.killTaskIfInterrupted() is private api in package(org.apache.spark).In order to use the public spark api, I finally decide to place it in the original place
--
To view, visit http://gerrit.cloudera.org:8080/7753
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0b4284f2c0a40cd7ba8cf2b76e0403592552c814
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: caiconghui <ca...@xiaomi.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: caiconghui <ca...@xiaomi.com>
Gerrit-HasComments: No
[kudu-CR] feat: add task interruption checking for RowIterator
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: feat: add task interruption checking for RowIterator
......................................................................
Patch Set 3:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/7753/3/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduRDD.scala
File java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduRDD.scala:
Line 23: import org.apache.spark.{InterruptibleIterator, Partition, SparkContext, TaskContext}
This is no longer necessary, right?
Line 64: new InterruptibleIterator[Row](taskContext, new RowIterator(scanner))
likewise
--
To view, visit http://gerrit.cloudera.org:8080/7753
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0b4284f2c0a40cd7ba8cf2b76e0403592552c814
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: caiconghui <ca...@xiaomi.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: caiconghui <ca...@xiaomi.com>
Gerrit-HasComments: Yes
[kudu-CR] feat: add the wrapper for the Iterator by using InterruptibleIterator
Posted by "caiconghui (Code Review)" <ge...@cloudera.org>.
caiconghui has posted comments on this change.
Change subject: feat: add the wrapper for the Iterator by using InterruptibleIterator
......................................................................
Patch Set 2:
@Dan Burkert I cannot find the appropriate api for canceling task or killing task, to simulate killing job for webui such as
test("kill task") {
insertRows(100)
val rdd = kuduContext.kuduRDD(sc, tableName, List("key"))
try {
rdd.foreachPartition(iterator =>
while (iterator.hasNext) {
//ToDo
// task kill api
iterator.next()
}
)
} catch {
case _: TaskKilledException => // good
case NonFatal(_) =>
fail("kill job should throw TaskKilledOperationException")
}
}
--
To view, visit http://gerrit.cloudera.org:8080/7753
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0b4284f2c0a40cd7ba8cf2b76e0403592552c814
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: caiconghui <ca...@xiaomi.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: caiconghui <ca...@xiaomi.com>
Gerrit-HasComments: No
[kudu-CR] feat: add task interruption checking for RowIterator
Posted by "caiconghui (Code Review)" <ge...@cloudera.org>.
caiconghui has posted comments on this change.
Change subject: feat: add task interruption checking for RowIterator
......................................................................
Patch Set 4:
@Kudu Jenkins, The failure seems that the exception below is not caused by the new feature?
--
To view, visit http://gerrit.cloudera.org:8080/7753
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0b4284f2c0a40cd7ba8cf2b76e0403592552c814
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: caiconghui <ca...@xiaomi.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: caiconghui <ca...@xiaomi.com>
Gerrit-HasComments: No
[kudu-CR] add the wrapper for the Iterator[Row] of KuduRDD by using Spark InterruptibleIterator[Row]
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: add the wrapper for the Iterator[Row] of KuduRDD by using Spark InterruptibleIterator[Row]
......................................................................
Patch Set 1:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/7753/1//COMMIT_MSG
Commit Message:
Line 9: once we the submit a spark job to spark cluster, we cannot cancel or stop the kudu-spark
nit: Don't indent message body, and uppercase 'Once'. Wrap at 72 characters.
PS1, Line 10: Aftering using
nit: s/Aftering using/With
http://gerrit.cloudera.org:8080/#/c/7753/1/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduRDD.scala
File java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduRDD.scala:
Line 21: import org.apache.spark.InterruptibleIterator
Add this import to the braced block on line 24.
Line 65: new InterruptibleIterator[Row](taskContext, new RowIterator(scanner))
InterruptibleIterator checks the status for every single row, perhaps it would be more efficient to only check when a new scan batch is needed from Kudu? The check is pretty simple: https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/InterruptibleIterator.scala#L36. That check could slot in right before the 'scanner.nextRows()' call on line 91.
--
To view, visit http://gerrit.cloudera.org:8080/7753
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0b4284f2c0a40cd7ba8cf2b76e0403592552c814
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: caiconghui <ca...@xiaomi.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: caiconghui <ca...@xiaomi.com>
Gerrit-HasComments: Yes