Skip to content

Conversation

@xinyuiscool
Copy link
Contributor

More improvements for SamzaRunner:

  • Life cycle methods for the pipeline runtime
  • Hook up Samza ExternalContext for LinkedIin use cases
  • Support metrics reporters in pipeline options
  • Some bug fixes for the state key in Samza

Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

  • Choose reviewer(s) and mention them in a comment (R: @username).
  • Format the pull request title like [BEAM-XXX] Fixes bug in ApproximateQuantiles, where you replace BEAM-XXX with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
  • If this contribution is large, please file an Apache Individual Contributor License Agreement.

Post-Commit Tests Status (on master branch)

Lang SDK Apex Dataflow Flink Gearpump Samza Spark
Go Build Status --- --- --- --- --- ---
Java Build Status Build Status Build Status Build Status
Build Status
Build Status
Build Status Build Status Build Status
Python Build Status
Build Status
--- Build Status
Build Status
Build Status --- --- ---

Pre-Commit Tests Status (on master branch)

--- Java Python Go Website
Non-portable Build Status Build Status Build Status Build Status
Portable --- Build Status --- ---

See .test-infra/jenkins/README for trigger phrase, status and link of all Jenkins jobs.


final StateInfo stateInfo = getStateInfo();

if (listener != null && (stateInfo.state == State.DONE || stateInfo.state == State.FAILED)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm trying to understand when would we hit this while state is neither Done nor FAILED. What is being protected by having this check?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, the status can be RUNNING, since it can be a timed waifForFinish().

private final SystemReduceFn<K, InputT, ?, OutputT, BoundedWindow> reduceFn;
private final String stepName;
private final String stepId;
private final PCollection.IsBounded isBounded;
Copy link
Contributor

Choose a reason for hiding this comment

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

import PCollection.IsBounded directly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

configBuilder.put("job.host-affinity.enabled", "true");
}

switch (options.getSamzaExecutionEnvironment()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

not sure whether we have logged the entire pipeline option somewhere. Otherwise it would be good to have a log entry here about which execution environment we're running

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense. Added more logs.

Copy link
Contributor

@lhaiesp lhaiesp left a comment

Choose a reason for hiding this comment

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

minor comments

Copy link
Contributor

@lhaiesp lhaiesp left a comment

Choose a reason for hiding this comment

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

LGTM

@xinyuiscool
Copy link
Contributor Author

Run JavaPortabilityApi PreCommit

@xinyuiscool
Copy link
Contributor Author

Run Python PreCommit

@xinyuiscool
Copy link
Contributor Author

Run Website PreCommit

1 similar comment
@xinyuiscool
Copy link
Contributor Author

Run Website PreCommit

@xinyuiscool xinyuiscool merged commit 37556f2 into apache:master Apr 9, 2019

repositories {
maven {
url "https://artifactory.corp.linkedin.com:8083/artifactory/DDS/"
Copy link
Contributor

Choose a reason for hiding this comment

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

Out of curiosity: What is this used for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch! That was for internal linkedin testing of Samza builds. Let me get rid of it. Thanks!

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