[SPARK-3154][STREAMING] Make FlumePollingInputDStream shutdown cleaner. #2065

harishreedharan · 2014-08-20T19:35:14Z

Currently lot of errors get thrown from Avro IPC layer when the dstream
or sink is shutdown. This PR cleans it up. Some refactoring is done in the
receiver code to put all of the RPC code into a single Try and just recover
from that. The sink code has also been cleaned up.

Currently lot of errors get thrown from Avro IPC layer when the dstream or sink is shutdown. This PR cleans it up. Some refactoring is done in the receiver code to put all of the RPC code into a single Try and just recover from that. The sink code has also been cleaned up.

SparkQA · 2014-08-20T19:40:54Z

QA tests have started for PR 2065 at commit ed608c8.

This patch merges cleanly.

SparkQA · 2014-08-20T20:33:17Z

QA tests have finished for PR 2065 at commit ed608c8.

This patch passes unit tests.
This patch merges cleanly.
This patch adds no public classes.

vanzin · 2014-08-20T22:55:23Z

...ume-sink/src/main/scala/org/apache/spark/streaming/flume/sink/SparkAvroCallbackHandler.scala

nit: space after if

SparkQA · 2014-08-21T00:06:37Z

QA tests have started for PR 2065 at commit 598efa7.

This patch merges cleanly.

SparkQA · 2014-08-21T01:02:01Z

QA tests have finished for PR 2065 at commit 598efa7.

This patch passes unit tests.
This patch merges cleanly.
This patch adds no public classes.

harishreedharan · 2014-08-21T03:05:24Z

Some errors (like the ChannelClosedException) getting logged on the sink side are inevitable. This is because Avro throws that when the other side closes the connection on the sink.

vanzin · 2014-08-21T16:13:03Z

...ume-sink/src/main/scala/org/apache/spark/streaming/flume/sink/SparkAvroCallbackHandler.scala

Do you mean to return here? If so you need an explicit return or an "else" block.

vanzin · 2014-08-21T16:17:09Z

Mostly nits left; looks OK after you look at the EventBatch issue I pointed out.

SparkQA · 2014-08-21T18:30:31Z

QA tests have started for PR 2065 at commit e7b8d82.

This patch merges cleanly.

SparkQA · 2014-08-21T19:00:31Z

QA tests have started for PR 2065 at commit 9001d26.

This patch merges cleanly.

SparkQA · 2014-08-21T19:25:15Z

QA tests have finished for PR 2065 at commit e7b8d82.

This patch passes unit tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2014-08-21T19:55:13Z

QA tests have finished for PR 2065 at commit 9001d26.

This patch passes unit tests.
This patch merges cleanly.
This patch adds no public classes.

tdas · 2014-08-25T21:39:58Z

...ume-sink/src/main/scala/org/apache/spark/streaming/flume/sink/SparkAvroCallbackHandler.scala

For better code understanding, it is better to call this sequenceNumberToProcessor (this style is often used in Spark, see DAGScheduler) and the processorsToShutdown to be called activeProcessors.

tdas · 2014-08-27T03:56:44Z

external/flume/src/main/scala/org/apache/spark/streaming/flume/FlumeBatchFetcher.scala

propogated --> propagated

SparkQA · 2014-08-27T04:00:58Z

QA tests have started for PR 2065 at commit a0a8852.

This patch merges cleanly.

SparkQA · 2014-08-27T04:11:14Z

QA tests have started for PR 2065 at commit d7427cc.

This patch merges cleanly.

SparkQA · 2014-08-27T04:54:41Z

QA tests have finished for PR 2065 at commit a0a8852.

This patch passes unit tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2014-08-27T05:07:09Z

QA tests have finished for PR 2065 at commit d7427cc.

This patch passes unit tests.
This patch merges cleanly.
This patch adds no public classes.

tdas · 2014-08-27T05:55:40Z

...ume-sink/src/main/scala/org/apache/spark/streaming/flume/sink/SparkAvroCallbackHandler.scala

nit: this line break can be better.

val processor = new TransactionProcessor( channel, seq, n, transactionTimeout, backOffInterval, this)

tdas · 2014-08-27T06:04:37Z

This now looks quite good to me. There were a few more formatting issues, should take 5 minutes to solve :)

SparkQA · 2014-08-27T07:01:02Z

QA tests have started for PR 2065 at commit f93a07c.

This patch merges cleanly.

SparkQA · 2014-08-27T07:53:59Z

QA tests have finished for PR 2065 at commit f93a07c.

This patch passes unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- rem In this case, leave out the main class (org.apache.spark.deploy.SparkSubmit) and use our own.

tdas · 2014-08-27T08:54:17Z

This looks real good now. Thanks @harishreedharan for all changes and the wonderful refactoring. I am going to quickly test this in my local flume set up for double confirmation. If it works out, will merge this in.

tdas · 2014-08-27T09:30:26Z

Alright, tested this. Merging it.

Currently lot of errors get thrown from Avro IPC layer when the dstream or sink is shutdown. This PR cleans it up. Some refactoring is done in the receiver code to put all of the RPC code into a single Try and just recover from that. The sink code has also been cleaned up. Author: Hari Shreedharan <hshreedharan@apache.org> Closes #2065 from harishreedharan/clean-flume-shutdown and squashes the following commits: f93a07c [Hari Shreedharan] Formatting fixes. d7427cc [Hari Shreedharan] More fixes! a0a8852 [Hari Shreedharan] Fix race condition, hopefully! Minor other changes. 4c9ed02 [Hari Shreedharan] Remove unneeded list in Callback handler. Other misc changes. 8fee36f [Hari Shreedharan] Scala-library is required, else maven build fails. Also catch InterruptedException in TxnProcessor. 445e700 [Hari Shreedharan] Merge remote-tracking branch 'asf/master' into clean-flume-shutdown 87232e0 [Hari Shreedharan] Refactor Flume Input Stream. Clean up code, better error handling. 9001d26 [Hari Shreedharan] Change log level to debug in TransactionProcessor#shutdown method e7b8d82 [Hari Shreedharan] Incorporate review feedback 598efa7 [Hari Shreedharan] Clean up some exception handling code e1027c6 [Hari Shreedharan] Merge remote-tracking branch 'asf/master' into clean-flume-shutdown ed608c8 [Hari Shreedharan] [SPARK-3154][STREAMING] Make FlumePollingInputDStream shutdown cleaner. (cherry picked from commit 6f671d0) Signed-off-by: Tathagata Das <tathagata.das1565@gmail.com>

Currently lot of errors get thrown from Avro IPC layer when the dstream or sink is shutdown. This PR cleans it up. Some refactoring is done in the receiver code to put all of the RPC code into a single Try and just recover from that. The sink code has also been cleaned up. Author: Hari Shreedharan <hshreedharan@apache.org> Closes apache#2065 from harishreedharan/clean-flume-shutdown and squashes the following commits: f93a07c [Hari Shreedharan] Formatting fixes. d7427cc [Hari Shreedharan] More fixes! a0a8852 [Hari Shreedharan] Fix race condition, hopefully! Minor other changes. 4c9ed02 [Hari Shreedharan] Remove unneeded list in Callback handler. Other misc changes. 8fee36f [Hari Shreedharan] Scala-library is required, else maven build fails. Also catch InterruptedException in TxnProcessor. 445e700 [Hari Shreedharan] Merge remote-tracking branch 'asf/master' into clean-flume-shutdown 87232e0 [Hari Shreedharan] Refactor Flume Input Stream. Clean up code, better error handling. 9001d26 [Hari Shreedharan] Change log level to debug in TransactionProcessor#shutdown method e7b8d82 [Hari Shreedharan] Incorporate review feedback 598efa7 [Hari Shreedharan] Clean up some exception handling code e1027c6 [Hari Shreedharan] Merge remote-tracking branch 'asf/master' into clean-flume-shutdown ed608c8 [Hari Shreedharan] [SPARK-3154][STREAMING] Make FlumePollingInputDStream shutdown cleaner.

Merge remote-tracking branch 'asf/master' into clean-flume-shutdown

e1027c6

vanzin reviewed Aug 20, 2014
View reviewed changes

...ume-sink/src/main/scala/org/apache/spark/streaming/flume/sink/SparkAvroCallbackHandler.scala Outdated

Copy link

Contributor

vanzin Aug 20, 2014

Choose a reason for hiding this comment

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

nit: space after if

Clean up some exception handling code

598efa7

vanzin reviewed Aug 21, 2014
View reviewed changes

Incorporate review feedback

e7b8d82

Change log level to debug in TransactionProcessor#shutdown method

9001d26

tdas reviewed Aug 25, 2014
View reviewed changes

tdas reviewed Aug 27, 2014
View reviewed changes

external/flume/src/main/scala/org/apache/spark/streaming/flume/FlumeBatchFetcher.scala Outdated

Copy link

Contributor

tdas Aug 27, 2014

Choose a reason for hiding this comment

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

propogated --> propagated

More fixes!

d7427cc

tdas reviewed Aug 27, 2014
View reviewed changes

Formatting fixes.

f93a07c

asfgit closed this in 6f671d0 Aug 27, 2014

[SPARK-3154][STREAMING] Make FlumePollingInputDStream shutdown cleaner. #2065

[SPARK-3154][STREAMING] Make FlumePollingInputDStream shutdown cleaner. #2065

Uh oh!

Conversation

harishreedharan commented Aug 20, 2014

Uh oh!

SparkQA commented Aug 20, 2014

Uh oh!

SparkQA commented Aug 20, 2014

Uh oh!

vanzin Aug 20, 2014

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Aug 21, 2014

Uh oh!

SparkQA commented Aug 21, 2014

Uh oh!

harishreedharan commented Aug 21, 2014

Uh oh!

vanzin Aug 21, 2014

Choose a reason for hiding this comment

Uh oh!

vanzin commented Aug 21, 2014

Uh oh!

SparkQA commented Aug 21, 2014

Uh oh!

SparkQA commented Aug 21, 2014

Uh oh!

SparkQA commented Aug 21, 2014

Uh oh!

SparkQA commented Aug 21, 2014

Uh oh!

tdas Aug 25, 2014

Choose a reason for hiding this comment

Uh oh!

tdas Aug 27, 2014

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Aug 27, 2014

Uh oh!

SparkQA commented Aug 27, 2014

Uh oh!

SparkQA commented Aug 27, 2014

Uh oh!

SparkQA commented Aug 27, 2014

Uh oh!

tdas Aug 27, 2014

Choose a reason for hiding this comment

Uh oh!

tdas commented Aug 27, 2014

Uh oh!

SparkQA commented Aug 27, 2014

Uh oh!

SparkQA commented Aug 27, 2014

Uh oh!

tdas commented Aug 27, 2014

Uh oh!

tdas commented Aug 27, 2014

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants