You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by jose-torres <gi...@git.apache.org> on 2018/02/01 23:40:48 UTC

[GitHub] spark pull request #20445: [SPARK-23092][SQL] Migrate MemoryStream to DataSo...

Github user jose-torres commented on a diff in the pull request:

    https://github.com/apache/spark/pull/20445#discussion_r165222931
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/MicroBatchExecution.scala ---
    @@ -270,16 +270,17 @@ class MicroBatchExecution(
                 }
               case s: MicroBatchReader =>
                 updateStatusMessage(s"Getting offsets from $s")
    -            reportTimeTaken("getOffset") {
    -            // Once v1 streaming source execution is gone, we can refactor this away.
    -            // For now, we set the range here to get the source to infer the available end offset,
    -            // get that offset, and then set the range again when we later execute.
    -            s.setOffsetRange(
    -              toJava(availableOffsets.get(s).map(off => s.deserializeOffset(off.json))),
    -              Optional.empty())
    -
    -              (s, Some(s.getEndOffset))
    +            reportTimeTaken("setOffsetRange") {
    --- End diff --
    
    I agree that the old metric names don't make much sense anymore, but I worry about changing external-facing behavior as part of an API migration.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org