Skip to content

Conversation

@CodingCat
Copy link
Contributor

@CodingCat CodingCat commented Feb 7, 2017

What changes were proposed in this pull request?

addBatch method in Sink trait is supposed to be a synchronous method to coordinate with the fault-tolerance design in StreamingExecution (being different with the compute() method in DStream)

We need to add more notes in the comments of this method to remind the developers

How was this patch tested?

existing tests

* Otherwise, you may get a wrong result.
*
* Note 2: The method is supposed to be executed synchronously, i.e. the method should only return
* after data is added to sink successfully.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: data is added -> data is consumed. add may be not clear as you can also add DataFrame to a collection and do nothing.

@SparkQA
Copy link

SparkQA commented Feb 7, 2017

Test build #72527 has finished for PR 16840 at commit d9222d7.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Feb 7, 2017

Test build #72535 has finished for PR 16840 at commit a58e1c8.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@zsxwing
Copy link
Member

zsxwing commented Feb 8, 2017

LGTM. Thanks. Merging to master and 2.1.

asfgit pushed a commit that referenced this pull request Feb 8, 2017
## What changes were proposed in this pull request?

addBatch method in Sink trait is supposed to be a synchronous method to coordinate with the fault-tolerance design in StreamingExecution (being different with the compute() method in DStream)

We need to add more notes in the comments of this method to remind the developers

## How was this patch tested?

existing tests

Author: CodingCat <[email protected]>

Closes #16840 from CodingCat/SPARK-19499.

(cherry picked from commit d4cd975)
Signed-off-by: Shixiong Zhu <[email protected]>
@asfgit asfgit closed this in d4cd975 Feb 8, 2017
@CodingCat
Copy link
Contributor Author

thanks

cmonkey pushed a commit to cmonkey/spark that referenced this pull request Feb 15, 2017
## What changes were proposed in this pull request?

addBatch method in Sink trait is supposed to be a synchronous method to coordinate with the fault-tolerance design in StreamingExecution (being different with the compute() method in DStream)

We need to add more notes in the comments of this method to remind the developers

## How was this patch tested?

existing tests

Author: CodingCat <[email protected]>

Closes apache#16840 from CodingCat/SPARK-19499.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants