Skip to content

[BEAM-2074,BEAM-2073] Fixed SourceDStream's rate control usage.#2733

Merged
asfgit merged 1 commit intoapache:masterfrom
staslev:BEAM-2074-2073-fix-SourceDStream-rate-control
May 1, 2017
Merged

[BEAM-2074,BEAM-2073] Fixed SourceDStream's rate control usage.#2733
asfgit merged 1 commit intoapache:masterfrom
staslev:BEAM-2074-2073-fix-SourceDStream-rate-control

Conversation

@staslev
Copy link
Contributor

@staslev staslev commented Apr 27, 2017

Be sure to do all of the following to help us incorporate your contribution
quickly and easily:

  • Make sure the PR title is formatted like:
    [BEAM-<Jira issue #>] Description of pull request
  • Make sure tests pass via mvn clean verify. (Even better, enable
    Travis-CI on your fork and ensure the whole test matrix passes).
  • Replace <Jira issue #> in the title with the actual Jira issue
    number, if there is one.
  • If this contribution is large, please file an Apache
    Individual Contributor License Agreement.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.003%) to 69.83% when pulling c0e1d98 on staslev:BEAM-2074-2073-fix-SourceDStream-rate-control into b82cd24 on apache:master.

@staslev
Copy link
Contributor Author

staslev commented Apr 28, 2017

Run Spark ValidatesRunner

@staslev
Copy link
Contributor Author

staslev commented Apr 28, 2017

retest this please.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 69.674% when pulling c0e1d98 on staslev:BEAM-2074-2073-fix-SourceDStream-rate-control into b82cd24 on apache:master.

@staslev
Copy link
Contributor Author

staslev commented Apr 28, 2017

Run Spark ValidatesRunner

@staslev
Copy link
Contributor Author

staslev commented Apr 30, 2017

R: @aviemzur

Copy link
Member

@aviemzur aviemzur left a comment

Choose a reason for hiding this comment

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

LGTM. 1 question and 1 minor comment.

Question: Can we write a test for an unbounded pipeline with the max records defined?
If it is very complex perhaps this could be a separate ticket/PR.

final scala.Option<Long> rateControlledMax = rateControlledMaxRecords();
if (rateControlledMax.isDefined()) {
LOG.info("Max records per batch has been set to {}, as advised by the rate controller.",
rateControlledMax.get());
Copy link
Member

Choose a reason for hiding this comment

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

Could extract param as this is called twice. not crucial.

@staslev
Copy link
Contributor Author

staslev commented Apr 30, 2017

Prior to submitting this PR I briefly played around with trying to test it, but could not find an easy way to test things at the level of inspecting rates per individual batches and the likes.

I believe this deserves a separate JIRA so please feel free to file one.

@staslev
Copy link
Contributor Author

staslev commented May 1, 2017

Rebased against #2698.

@staslev
Copy link
Contributor Author

staslev commented May 1, 2017

Run Spark ValidatesRunner

@coveralls
Copy link

Coverage Status

Coverage remained the same at 70.147% when pulling 233668b on staslev:BEAM-2074-2073-fix-SourceDStream-rate-control into 9f2733a on apache:master.

@staslev
Copy link
Contributor Author

staslev commented May 1, 2017

Run Spark ValidatesRunner

@staslev
Copy link
Contributor Author

staslev commented May 1, 2017

Run Spark ValidatesRunner

@coveralls
Copy link

Coverage Status

Coverage increased (+0.007%) to 70.142% when pulling fcb61ae on staslev:BEAM-2074-2073-fix-SourceDStream-rate-control into 254470e on apache:master.

@asfgit asfgit merged commit fcb61ae into apache:master May 1, 2017
asfgit pushed a commit that referenced this pull request May 1, 2017
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.

4 participants