-
Notifications
You must be signed in to change notification settings - Fork 331
SAMZA-1838: Make some minor improvements to ExecutionPlanner #623
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
SAMZA-1838: Make some minor improvements to ExecutionPlanner #623
Conversation
vjagadish1989
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple of questions to understand the change better, thanks Ahmed!
samza-core/src/main/java/org/apache/samza/execution/ExecutionPlanner.java
Show resolved
Hide resolved
samza-core/src/main/java/org/apache/samza/execution/ExecutionPlanner.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
approved, thanks for the contribution Ahmed! If need be, update SAMZA-1838 so that the JIRA and code are consistent
samza-core/src/main/java/org/apache/samza/execution/ExecutionPlanner.java
Show resolved
Hide resolved
samza-core/src/main/java/org/apache/samza/execution/ExecutionPlanner.java
Show resolved
Hide resolved
samza-core/src/main/java/org/apache/samza/execution/ExecutionPlanner.java
Show resolved
Hide resolved
samza-core/src/main/java/org/apache/samza/execution/ExecutionPlanner.java
Show resolved
Hide resolved
shanthoosh
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor comment, otherwise LGTM.
samza-core/src/main/java/org/apache/samza/execution/JobGraph.java
Outdated
Show resolved
Hide resolved
samza-core/src/main/java/org/apache/samza/execution/JobGraph.java
Outdated
Show resolved
Hide resolved
This commit includes the following changes:
- Fix case where ExecutionPlanner did not throw in response
to joining 2 input streams with different partition counts
- Improve some method names in ExecutionPlanner
- Improve some method/field names in JobGraph
- Make minor improvements to createJobGraph()
- Rewrite updateExistingPartitions() to make it a little easier to follow
- Use more constrained OperatorSpec types in the associations defined in
calculateJoinInputPartitions()
- Have calculateIntStreamPartitions() throw in response to bad config for
job.intermediate.stream.partitions
- Improve some error messages
@prateekm @vjagadish1989 whenever you get a chance. This is an initial set of mostly minor changes.