Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/8552 )
Change subject: KUDU-1454 [part 1]: update propagated timestamp on the driver ...................................................................... Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/8552/1/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala File java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala: http://gerrit.cloudera.org:8080/#/c/8552/1/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala@86 PS1, Line 86: syncClient.updateLastPropagatedTimestamp(timestampAccumulator.value) > it seems slightly odd that merge() has a side-effect here. Does this end up My understanding is that merge() is only called on the driver according to reading of the source code: https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/scheduler/DAGScheduler.scala#L1122. I agree it looks odd to update TS here. But on a second thought, I decide to put it here since it is central place to do the TS update, other that go into different scan/write paths to do the update. Does that make sense? http://gerrit.cloudera.org:8080/#/c/8552/1/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala@294 PS1, Line 294: timestampAccumulator.add(syncClient.getLastPropagatedTimestamp) > should this be finally{} so that, even if there is an error, you can see al I do not quite follow your second comment. My understanding is for each RDD, the kuduClient would be copied to the executors from the driver. And with the last-propagated timestamp set on driver, the next read/write on executors should also get this information? -- To view, visit http://gerrit.cloudera.org:8080/8552 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id0a078ae8ebaa6a859be75c822879291192c5842 Gerrit-Change-Number: 8552 Gerrit-PatchSet: 2 Gerrit-Owner: Hao Hao <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Hao Hao <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Comment-Date: Thu, 16 Nov 2017 00:13:44 +0000 Gerrit-HasComments: Yes
