samza-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jake Maes <jacob.m...@gmail.com>
Subject Re: Review Request 44920: Remove tight coupling of Samza with Yarn. Define APIs for resource manager integration
Date Wed, 16 Mar 2016 19:27:26 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44920/#review123904
-----------------------------------------------------------



I had a few minutes, so here's some low-hanging fruit.


samza-core/src/main/java/org/apache/samza/clustermanager/AbstractContainerAllocator.java (line
203)
<https://reviews.apache.org/r/44920/#comment186196>

    yarn -> cluster manager



samza-core/src/main/java/org/apache/samza/clustermanager/AbstractContainerAllocator.java (line
230)
<https://reviews.apache.org/r/44920/#comment186200>

    terminology: container==resource?
    
    multiple occurrences from here to the end of the file



samza-core/src/main/java/org/apache/samza/clustermanager/ClusterBasedJobCoordinator.java (line
133)
<https://reviews.apache.org/r/44920/#comment186203>

    Multiple occurrences in this file: bracket placement is inconsistent with the rest of
the codebase



samza-core/src/main/java/org/apache/samza/clustermanager/ClusterBasedJobCoordinator.java (lines
193 - 194)
<https://reviews.apache.org/r/44920/#comment186204>

    Stack traces should not be printed to console. The log line should include the stack trace,
so while the printStackTrace() may be useful for early debugging, it should be removed before
committing.



samza-core/src/main/java/org/apache/samza/clustermanager/ClusterBasedJobCoordinator.java (lines
199 - 200)
<https://reviews.apache.org/r/44920/#comment186205>

    Same, no stacktrace



samza-core/src/main/java/org/apache/samza/clustermanager/ClusterBasedJobCoordinator.java (lines
225 - 236)
<https://reviews.apache.org/r/44920/#comment186206>

    No stacktraces



samza-core/src/main/java/org/apache/samza/clustermanager/ContainerAllocator.java (line 47)
<https://reviews.apache.org/r/44920/#comment186208>

    Terminology: container vs resource



samza-core/src/main/java/org/apache/samza/clustermanager/ContainerRequestState.java (line
152)
<https://reviews.apache.org/r/44920/#comment186211>

    Same terminology question


- Jake Maes


On March 16, 2016, 6:23 p.m., Jagadish Venkatraman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44920/
> -----------------------------------------------------------
> 
> (Updated March 16, 2016, 6:23 p.m.)
> 
> 
> Review request for samza, Boris Shkolnik, Chris Pettitt, Jake Maes, Yi Pan (Data Infrastructure),
Navina Ramesh, and Xinyu Liu.
> 
> 
> Repository: samza
> 
> 
> Description
> -------
> 
> Samza currently has tight coupling with Yarn. This makes it impossible to integrate with
other resource managers like Mesos, or to run standalone without any resource manager. This
RB is a step to implementing SAMZA-881.
> 
> Design Doc: https://issues.apache.org/jira/secure/attachment/12790540/SamzaJobCoordinatorRe-designProposal.pdf
> 
> 1.Proposed new APIs for a resource manager to integrate with Samza. (SAMZA-881)
>    - Defined the ContainerProcessManager abstraction, SamzaResource, SamzaResourceRequest.

>    - Re-wrote the SamzaAppMaster into a ClusterBasedJobCoordinator.
>    - Re-wrote yarn specific request logic by abstracting it into a YarnContainerManager.

> 2.Defined a ClusterManagerConfig to handle configurations independent of Yarn/Mesos.
> 3.Made Samza's cluster interaction independent of Yarn. This separates Samza specific
components into samza-core and Yarn components into samza-yarn.
> 4.Readability improvements to the existing code base.
>    -Added docs for most methods, member variables and classes (including on thread-safety)
>    - Made internal variables final to document intent, visibility across threads. (trivially
by adding modifiers, or by changing where they're initialized.)
> 5.Refactored JobCoordinator into a JobModelReader.
> 
> TODO: Can go into the upcoming release. (P0)
> 1.Refactor the UI state variables and tests. Port some method re-orgs from SAMZA-867
into here.
> 2.Revise packaging structure.
> 4.Document new configs.
> 5.Rename run-am.sh to run-coordinator.sh, Delete all files in the non-refactored namespace.
(For unit-testing, these files continue to exist)
> 
> TODO: (P1)
> 1.Build Mesos integration for Samza. Should be simpler to integrate with the newer APIs.
>   - I started on this, and I plan to refine and post an RB in one of the hack-days.
> 2.Refactor the SamzaAppState class to provide more accessors and eliminate public variables.
(This was
> a consequence of the already existing design which I've tried to be compatible with)
> 
> TODO: I plan to track these with JIRAs so that they can be done later. (P2)
> 1.Get rid of the HTTP Server in the JobCoordinator
> 2.Make YarnJobCoordinator implement the JobCoordinator API as SAMZA-881.
> 
> 
> Diffs
> -----
> 
>   samza-core/src/main/java/org/apache/samza/clustermanager/AbstractContainerAllocator.java
PRE-CREATION 
>   samza-core/src/main/java/org/apache/samza/clustermanager/ClusterBasedJobCoordinator.java
PRE-CREATION 
>   samza-core/src/main/java/org/apache/samza/clustermanager/ContainerAllocator.java PRE-CREATION

>   samza-core/src/main/java/org/apache/samza/clustermanager/ContainerManagerFactory.java
PRE-CREATION 
>   samza-core/src/main/java/org/apache/samza/clustermanager/ContainerProcessManager.java
PRE-CREATION 
>   samza-core/src/main/java/org/apache/samza/clustermanager/ContainerRequestState.java
PRE-CREATION 
>   samza-core/src/main/java/org/apache/samza/clustermanager/HostAwareContainerAllocator.java
PRE-CREATION 
>   samza-core/src/main/java/org/apache/samza/clustermanager/ResourceFailure.java PRE-CREATION

>   samza-core/src/main/java/org/apache/samza/clustermanager/SamzaAppState.java PRE-CREATION

>   samza-core/src/main/java/org/apache/samza/clustermanager/SamzaContainerLaunchException.java
PRE-CREATION 
>   samza-core/src/main/java/org/apache/samza/clustermanager/SamzaResource.java PRE-CREATION

>   samza-core/src/main/java/org/apache/samza/clustermanager/SamzaResourceRequest.java
PRE-CREATION 
>   samza-core/src/main/java/org/apache/samza/clustermanager/SamzaResourceStatus.java PRE-CREATION

>   samza-core/src/main/java/org/apache/samza/clustermanager/SamzaTaskManager.java PRE-CREATION

>   samza-core/src/main/java/org/apache/samza/config/ClusterManagerConfig.java PRE-CREATION

>   samza-core/src/main/scala/org/apache/samza/coordinator/JobModelReader.scala PRE-CREATION

>   samza-core/src/main/scala/org/apache/samza/metrics/SamzaAppMasterMetrics.scala PRE-CREATION

>   samza-shell/src/main/bash/run-am.sh 9545a96953baaff17ad14962e02bc12aadbb1101 
>   samza-yarn/src/main/java/org/apache/samza/job/yarn/refactor/YarnAppState.java PRE-CREATION

>   samza-yarn/src/main/java/org/apache/samza/job/yarn/refactor/YarnContainerManager.java
PRE-CREATION 
>   samza-yarn/src/main/java/org/apache/samza/job/yarn/refactor/YarnContainerManagerFactory.java
PRE-CREATION 
>   samza-yarn/src/main/java/org/apache/samza/job/yarn/refactor/YarnContainerRunner.java
PRE-CREATION 
>   samza-yarn/src/main/scala/org/apache/samza/job/yarn/YarnAppMasterListener.scala 6bf3046a1ae4ed8f57500acae763184084ad0e09

>   samza-yarn/src/main/scala/org/apache/samza/job/yarn/refactor/SamzaAppMasterService.scala
PRE-CREATION 
>   samza-yarn/src/main/scala/org/apache/samza/job/yarn/refactor/SamzaYarnAppMasterLifecycle.scala
PRE-CREATION 
>   samza-yarn/src/test/scala/org/apache/samza/job/yarn/TestSamzaAppMasterLifecycle.scala
30cf34fe1fd3f74537d16e8a51b467cd50835357 
>   samza-yarn/src/test/scala/org/apache/samza/job/yarn/TestSamzaAppMasterService.scala
7f5d9f4af088589d4287c26737bae25567c157d7 
> 
> Diff: https://reviews.apache.org/r/44920/diff/
> 
> 
> Testing
> -------
> 
> 1.Tested with running test jobs in Yarn clusters of varying sizes from 1 node to 36 nodes.
> 2.Tested for failures by killing containers, and ensuring they were brought up again.
> 
> TODO:
> 1.Refactor all unit tests, and ensure updates to state variables are consistent with
the current design.
> 2.More testing with test jobs on clusters.
> 
> 
> Thanks,
> 
> Jagadish Venkatraman
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message