Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.0.3-alpha
    • Component/s: None
    • Labels:
      None

      Description

      Add a basic client wrapper library to the AM RM protocol in order to prevent proliferation of code being duplicated everywhere. Provide helper functions to perform reverse mapping of container requests to RM allocation resource request table format.

      1. YARN-103.1.patch
        57 kB
        Bikas Saha
      2. YARN-103.2.patch
        58 kB
        Bikas Saha
      3. YARN-103.3.patch
        61 kB
        Bikas Saha
      4. YARN-103.4.patch
        69 kB
        Bikas Saha
      5. YARN-103.4.wrapper.patch
        73 kB
        Bikas Saha
      6. YARN-103.5.patch
        62 kB
        Bikas Saha
      7. YARN-103.6.patch
        62 kB
        Bikas Saha
      8. YARN-103.7.patch
        62 kB
        Bikas Saha
      9. YARN-103.8.patch
        62 kB
        Bikas Saha
      10. YARN-103.9.patch
        35 kB
        Bikas Saha

        Issue Links

          Activity

          Hide
          Vinod Kumar Vavilapalli added a comment -

          +1, this is really helpful if we can get the APIs right.

          Show
          Vinod Kumar Vavilapalli added a comment - +1, this is really helpful if we can get the APIs right.
          Hide
          Bikas Saha added a comment -

          AMRMClient has wrappers for all AMRM protocol methods. In addition it provides API to release containers, add and remove container requests.
          TestAMRMClient added that also ends up testing YarnClient.
          Changed ApplicationMaster in DistributedShell to use AMRMClientImpl.
          Changed memory setting of AM in DistributedShell because there were occasional failures with the old setting. AM container was sometimes being terminated at startup because its VM usage was going slightly above quota.

          Show
          Bikas Saha added a comment - AMRMClient has wrappers for all AMRM protocol methods. In addition it provides API to release containers, add and remove container requests. TestAMRMClient added that also ends up testing YarnClient. Changed ApplicationMaster in DistributedShell to use AMRMClientImpl. Changed memory setting of AM in DistributedShell because there were occasional failures with the old setting. AM container was sometimes being terminated at startup because its VM usage was going slightly above quota.
          Hide
          Bo Wang added a comment -

          +1, this one can take over a lot of burden of writing AMs.

          Show
          Bo Wang added a comment - +1, this one can take over a lot of burden of writing AMs.
          Hide
          Bikas Saha added a comment -

          Fix a bug with not clearing remoteRequestTable and add test for that.
          Fix synchronization. I had meant to guard against concurrent access to shared collections and had taken the lazy route of synchronizing the whole method. Fixed.

          Show
          Bikas Saha added a comment - Fix a bug with not clearing remoteRequestTable and add test for that. Fix synchronization. I had meant to guard against concurrent access to shared collections and had taken the lazy route of synchronizing the whole method. Fixed.
          Hide
          Bikas Saha added a comment -

          Cancelling patch since this needs more work.

          Show
          Bikas Saha added a comment - Cancelling patch since this needs more work.
          Hide
          Bikas Saha added a comment -

          Added exception handling on allocate. Added tests to verify sending 0 container requests when resources are not needed anymore.
          Added tests to verify behavior during exception on allocate.

          Show
          Bikas Saha added a comment - Added exception handling on allocate. Added tests to verify sending 0 container requests when resources are not needed anymore. Added tests to verify behavior during exception on allocate.
          Hide
          Siddharth Seth added a comment -

          This is looking good with the synchronization fixes and error handling. The MR-3902 branch has some similar changes in RMContainerRequester. I'll try plugging this in there.
          A few comments.

          • makeContainerRequest was called addContainerRequest in RMContainerRequestor. Why rename it. To me, addContainerRequest makes more sense since the AMRMClient maintains an aggregate of requests.
          • The javadoc for removeContainerRequest is a little misleading. "This will not remove previous requests after allocate has been called for those requests". removeContainerRequests just decrements the specific request from the main request table - and sends out whatever has changed in the next allocate call.
          • getClusterAvailableResources / getClusterNodeCount - Instead of documenting that these should be called after an 'allocate' call - should this be enforced, maybe via an exception ? (Independent of this jira, does it make sense to include additional information in the Register call - i.e. numClusterNodes, avaialbleResources / other parts of the allocate payload)
          • Similarly, instead of documenting that allocate cannot be called concurrently - this can be enforced.
          • In the unit test, would prefer avoiding MiniMRCluster where possible. This one should be possible using the already existing MockRM or by mocking the RM. (branch MR-3902:TestRMContainerRequestor already does this, and tests a failure scenario for this patch)
            Minor Stuff
          • Avoid RecordFactory usage. Replaced by Records.newRecord() or BuilderUtils.*
          • The modification to ask in case of a failure could be a simple ask.addAll instead of the exists check.
          Show
          Siddharth Seth added a comment - This is looking good with the synchronization fixes and error handling. The MR-3902 branch has some similar changes in RMContainerRequester. I'll try plugging this in there. A few comments. makeContainerRequest was called addContainerRequest in RMContainerRequestor. Why rename it. To me, addContainerRequest makes more sense since the AMRMClient maintains an aggregate of requests. The javadoc for removeContainerRequest is a little misleading. "This will not remove previous requests after allocate has been called for those requests". removeContainerRequests just decrements the specific request from the main request table - and sends out whatever has changed in the next allocate call. getClusterAvailableResources / getClusterNodeCount - Instead of documenting that these should be called after an 'allocate' call - should this be enforced, maybe via an exception ? (Independent of this jira, does it make sense to include additional information in the Register call - i.e. numClusterNodes, avaialbleResources / other parts of the allocate payload) Similarly, instead of documenting that allocate cannot be called concurrently - this can be enforced. In the unit test, would prefer avoiding MiniMRCluster where possible. This one should be possible using the already existing MockRM or by mocking the RM. (branch MR-3902:TestRMContainerRequestor already does this, and tests a failure scenario for this patch) Minor Stuff Avoid RecordFactory usage. Replaced by Records.newRecord() or BuilderUtils.* The modification to ask in case of a failure could be a simple ask.addAll instead of the exists check.
          Hide
          Bikas Saha added a comment -
          • Changed to addResourceRequest.
          • Changed javadoc.
          • This client is the barebones implementation and hence has no smarts. The checks can be implemented/may not be necessary in smarter versions of the client. Additionally, like you say, some information may be better exposed at different API's.
          • I actually want to test against the MiniMRCluster instead of an expected behavior via a mockRM. I want the client test to fail if the RM behavior changes. So yes this is more of a functional test than a unit test.
          • Used BuilderUtils where available
          • I prefer not to do addAll because the current code explains the logic better and is resilient to the way comparators for the set object work.
          Show
          Bikas Saha added a comment - Changed to addResourceRequest. Changed javadoc. This client is the barebones implementation and hence has no smarts. The checks can be implemented/may not be necessary in smarter versions of the client. Additionally, like you say, some information may be better exposed at different API's. I actually want to test against the MiniMRCluster instead of an expected behavior via a mockRM. I want the client test to fail if the RM behavior changes. So yes this is more of a functional test than a unit test. Used BuilderUtils where available I prefer not to do addAll because the current code explains the logic better and is resilient to the way comparators for the set object work.
          Hide
          Bikas Saha added a comment -

          Per your comments in MAPREDUCE-4671, I have attached another version of the 4th patch that uses a Wrapper class for ResourceRequest instead of not deleting objects. You can compare both and see which one seems more palatable. Based on that MAPREDUCE-4671 can be updated if needed.

          Show
          Bikas Saha added a comment - Per your comments in MAPREDUCE-4671 , I have attached another version of the 4th patch that uses a Wrapper class for ResourceRequest instead of not deleting objects. You can compare both and see which one seems more palatable. Based on that MAPREDUCE-4671 can be updated if needed.
          Hide
          Siddharth Seth added a comment -

          This client is the barebones implementation and hence has no smarts. The checks can be implemented/may not be necessary in smarter versions of the client. Additionally, like you say, some information may be better exposed at different API's.

          Why have an additional smarter client on top of this one in YARN ? Individual apps may choose to have their own - MR for RPC specific error handling, etc. We could even considering getting rid of getNumClusterNodes / getAvailableResources since they're available via AllocateResponse.
          Would like others to take a look at the entire API specification before committing this.

          I actually want to test against the MiniMRCluster instead of an expected behavior via a mockRM. I want the client test to fail if the RM behavior changes. So yes this is more of a functional test than a unit test.

          RM behaviour depends upon the underlying scheduler. For a simple case like this, the behaviour is likely to be the same across schedulers - but the test runs against the default - which is the CS right now. We don't differentiate between functional / unit tests at the moment - which makes the test runtimes reasonably high. (MiniClusetr tests take several seconds versus fractions of a second for mocks).
          In the current functional test - there's a sleep-poll cycle which could get stuck.

          Per your comments in MAPREDUCE-4671, I have attached another version of the 4th patch that uses a Wrapper class for ResourceRequest instead of not deleting objects. You can compare both and see which one seems more palatable. Based on that MAPREDUCE-4671 can be updated if needed.

          A custom comparator may be sufficient - o.a.h.y.s.r.Application already uses this. Alternately, using the wrapper only for the 'ask' table may be simpler ?

          Looks like the patches accidentally include some hdfs changes.

          Show
          Siddharth Seth added a comment - This client is the barebones implementation and hence has no smarts. The checks can be implemented/may not be necessary in smarter versions of the client. Additionally, like you say, some information may be better exposed at different API's. Why have an additional smarter client on top of this one in YARN ? Individual apps may choose to have their own - MR for RPC specific error handling, etc. We could even considering getting rid of getNumClusterNodes / getAvailableResources since they're available via AllocateResponse. Would like others to take a look at the entire API specification before committing this. I actually want to test against the MiniMRCluster instead of an expected behavior via a mockRM. I want the client test to fail if the RM behavior changes. So yes this is more of a functional test than a unit test. RM behaviour depends upon the underlying scheduler. For a simple case like this, the behaviour is likely to be the same across schedulers - but the test runs against the default - which is the CS right now. We don't differentiate between functional / unit tests at the moment - which makes the test runtimes reasonably high. (MiniClusetr tests take several seconds versus fractions of a second for mocks). In the current functional test - there's a sleep-poll cycle which could get stuck. Per your comments in MAPREDUCE-4671 , I have attached another version of the 4th patch that uses a Wrapper class for ResourceRequest instead of not deleting objects. You can compare both and see which one seems more palatable. Based on that MAPREDUCE-4671 can be updated if needed. A custom comparator may be sufficient - o.a.h.y.s.r.Application already uses this. Alternately, using the wrapper only for the 'ask' table may be simpler ? Looks like the patches accidentally include some hdfs changes.
          Hide
          Bikas Saha added a comment -

          This version of the client is for advanced apps like MR where it lets them avoid duplication of client boiler plate code + some basic request table inversion logic.
          I am open to removing getNumClusterNodes/getAvailableResource helper functions since the values are available via AllocateResponse. I had put them their because I didnt like that one of them came from AMResponse and the other from AllocateResponse.
          A smarter client will do things like make sure the protocol semantics are being automatically followed. E.g. in will automatically make sure that the conditions you mention above dont happen. Also, it will automatically heartbeat with the RM so that the AM does not have to worry about it. It can even have basic container to task assignment logic. I have a smart client waiting in the wings that builds on top of this one
          The test only expects the RM to schedule some containers to it and ack release containers. So that should be relatively scheduler agnostic. Once we separate functional tests from unit tests then we could start a single mini cluster for a set of tests and amortize the costs. I have added a junit test timeout to limit the downside of getting stuck due to a bug.
          A custom comparator for ask would have been enough. I have changed that. The previous patch is the kind of code that comes out of a stuffy nose and head

          Show
          Bikas Saha added a comment - This version of the client is for advanced apps like MR where it lets them avoid duplication of client boiler plate code + some basic request table inversion logic. I am open to removing getNumClusterNodes/getAvailableResource helper functions since the values are available via AllocateResponse. I had put them their because I didnt like that one of them came from AMResponse and the other from AllocateResponse. A smarter client will do things like make sure the protocol semantics are being automatically followed. E.g. in will automatically make sure that the conditions you mention above dont happen. Also, it will automatically heartbeat with the RM so that the AM does not have to worry about it. It can even have basic container to task assignment logic. I have a smart client waiting in the wings that builds on top of this one The test only expects the RM to schedule some containers to it and ack release containers. So that should be relatively scheduler agnostic. Once we separate functional tests from unit tests then we could start a single mini cluster for a set of tests and amortize the costs. I have added a junit test timeout to limit the downside of getting stuck due to a bug. A custom comparator for ask would have been enough. I have changed that. The previous patch is the kind of code that comes out of a stuffy nose and head
          Hide
          Bikas Saha added a comment -

          Ping

          Show
          Bikas Saha added a comment - Ping
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12548531/YARN-103.5.patch
          against trunk revision .

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 2 new or modified test files.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          -1 javadoc. The javadoc tool appears to have generated 1 warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          -1 findbugs. The patch appears to introduce 4 new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed unit tests in hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-applications-distributedshell hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-YARN-Build/185//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-YARN-Build/185//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-yarn-client.html
          Console output: https://builds.apache.org/job/PreCommit-YARN-Build/185//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12548531/YARN-103.5.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 2 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. -1 javadoc . The javadoc tool appears to have generated 1 warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. -1 findbugs . The patch appears to introduce 4 new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-applications-distributedshell hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-YARN-Build/185//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-YARN-Build/185//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-yarn-client.html Console output: https://builds.apache.org/job/PreCommit-YARN-Build/185//console This message is automatically generated.
          Hide
          Bikas Saha added a comment -

          Attaching patch that fixes warnings where they make sense.

          Show
          Bikas Saha added a comment - Attaching patch that fixes warnings where they make sense.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12556378/YARN-103.6.patch
          against trunk revision .

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 2 new or modified test files.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          -1 core tests. The patch failed these unit tests in hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-applications-distributedshell hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client:

          org.apache.hadoop.yarn.applications.distributedshell.TestDistributedShell

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-YARN-Build/209//testReport/
          Console output: https://builds.apache.org/job/PreCommit-YARN-Build/209//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12556378/YARN-103.6.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 2 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. -1 core tests . The patch failed these unit tests in hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-applications-distributedshell hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client: org.apache.hadoop.yarn.applications.distributedshell.TestDistributedShell +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-YARN-Build/209//testReport/ Console output: https://builds.apache.org/job/PreCommit-YARN-Build/209//console This message is automatically generated.
          Hide
          Bikas Saha added a comment -

          Attaching patch that fixes the test failure.

          Show
          Bikas Saha added a comment - Attaching patch that fixes the test failure.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12559906/YARN-103.7.patch
          against trunk revision .

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 2 new or modified test files.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed unit tests in hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-applications-distributedshell hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-YARN-Build/210//testReport/
          Console output: https://builds.apache.org/job/PreCommit-YARN-Build/210//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12559906/YARN-103.7.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 2 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-applications-distributedshell hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-YARN-Build/210//testReport/ Console output: https://builds.apache.org/job/PreCommit-YARN-Build/210//console This message is automatically generated.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12559906/YARN-103.7.patch
          against trunk revision .

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 2 new or modified test files.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed unit tests in hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-applications-distributedshell hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-YARN-Build/231//testReport/
          Console output: https://builds.apache.org/job/PreCommit-YARN-Build/231//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12559906/YARN-103.7.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 2 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-applications-distributedshell hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-YARN-Build/231//testReport/ Console output: https://builds.apache.org/job/PreCommit-YARN-Build/231//console This message is automatically generated.
          Hide
          Siddharth Seth added a comment -

          Apologies for taking ages to look at this. Some minor stuff pending.

          • AMRMClient JavaDoc
            1) The javadoc for the ContainerRequest class has some typos and needs to be punctuated (instead of newlines)
            2) allocate javadoc - makes a reference to makeContainerRequest which is now called addContainerRequest. Also it'll be useful to mention the reboot flag which may be sent as part of the response.
          • AMRMCLientImpl unregisterApplicationMaster - setAppAttemptId doesn't need to be in a synchronized block
          • AMRMClientImpl - add/decContainerRequest rack null checks need fixing (host instead of rack)
          • AMRMClientImpl.addResourceRequestToAsk - am not sure why this method is needed. A simple synchronized asks.add should be sufficient

          Also, would prefer the DistributedShell changes in a separate jira - just to keep this patch clean. Breaking that out of the current patch should be simple enough.

          Show
          Siddharth Seth added a comment - Apologies for taking ages to look at this. Some minor stuff pending. AMRMClient JavaDoc 1) The javadoc for the ContainerRequest class has some typos and needs to be punctuated (instead of newlines) 2) allocate javadoc - makes a reference to makeContainerRequest which is now called addContainerRequest. Also it'll be useful to mention the reboot flag which may be sent as part of the response. AMRMCLientImpl unregisterApplicationMaster - setAppAttemptId doesn't need to be in a synchronized block AMRMClientImpl - add/decContainerRequest rack null checks need fixing (host instead of rack) AMRMClientImpl.addResourceRequestToAsk - am not sure why this method is needed. A simple synchronized asks.add should be sufficient Also, would prefer the DistributedShell changes in a separate jira - just to keep this patch clean. Breaking that out of the current patch should be simple enough.
          Hide
          Tom White added a comment -

          > +1, this is really helpful if we can get the APIs right.

          I agree this would be very useful. Regarding getting the APIs right, making AMRMClient a class rather than an interface would permit us to add methods later on, which is not possible for interfaces. This is a helper class, so there shouldn't be a need for alternative implementations.

          Show
          Tom White added a comment - > +1, this is really helpful if we can get the APIs right. I agree this would be very useful. Regarding getting the APIs right, making AMRMClient a class rather than an interface would permit us to add methods later on, which is not possible for interfaces. This is a helper class, so there shouldn't be a need for alternative implementations.
          Hide
          Bikas Saha added a comment -

          I went with the interface + impl approach because that seems to be the pattern in the YARN code. YarnClient and YarnClientImpl among many others.

          Show
          Bikas Saha added a comment - I went with the interface + impl approach because that seems to be the pattern in the YARN code. YarnClient and YarnClientImpl among many others.
          Hide
          Siddharth Seth added a comment -

          Forgot to mention, I think the interface stability should be set to Unstable for now. This will change once AMResponse, AllocateResponse are flattened - and the RM api changes.
          This being a class instead of an interface seems Ok to me.

          Show
          Siddharth Seth added a comment - Forgot to mention, I think the interface stability should be set to Unstable for now. This will change once AMResponse, AllocateResponse are flattened - and the RM api changes. This being a class instead of an interface seems Ok to me.
          Hide
          Alejandro Abdelnur added a comment - - edited

          IMO the interface/class approach is good if the following considerations are observed:

          • the interface defines the API contract
          • the class may implement default implementation of the contract
          • the class may implement helper methods

          This pattern is done over and over in the the JDK and in many extensions (ie Servlet/HttpServlet).

          A nice thing of this pattern is that if in the future you need a new method in API contract, you can add it to the interface, and add to the class the default implementation of this new method (this is a must) and all existing subclasses won't break and won't need to implement such new method if they are ok with the default behavior of the new method.

          I'd suggest (following JDK conventions) renaming the AMRMCLientImpl class to AbstractAMRMClient and making it an abstract class.

          Show
          Alejandro Abdelnur added a comment - - edited IMO the interface/class approach is good if the following considerations are observed: the interface defines the API contract the class may implement default implementation of the contract the class may implement helper methods This pattern is done over and over in the the JDK and in many extensions (ie Servlet/HttpServlet). A nice thing of this pattern is that if in the future you need a new method in API contract, you can add it to the interface, and add to the class the default implementation of this new method (this is a must) and all existing subclasses won't break and won't need to implement such new method if they are ok with the default behavior of the new method. I'd suggest (following JDK conventions) renaming the AMRMCLientImpl class to AbstractAMRMClient and making it an abstract class.
          Hide
          Bikas Saha added a comment -

          I think the patch follows the example you mention about interface and default implementation. What I dont understand is why the default implementation should be made abstract? Noone will be able to use a fully functional implementation if its made abstract.

          Show
          Bikas Saha added a comment - I think the patch follows the example you mention about interface and default implementation. What I dont understand is why the default implementation should be made abstract? Noone will be able to use a fully functional implementation if its made abstract.
          Hide
          Alejandro Abdelnur added a comment -

          you would extend it to implement a concrete client.

          Show
          Alejandro Abdelnur added a comment - you would extend it to implement a concrete client.
          Hide
          Bikas Saha added a comment -

          That would mean creating an empty concrete client that extends an abstract AMRMClientImpl because AMRMClientImpl has a complete default implementation of the interface. I am not sure I understand why that needs to be done.

          Show
          Bikas Saha added a comment - That would mean creating an empty concrete client that extends an abstract AMRMClientImpl because AMRMClientImpl has a complete default implementation of the interface. I am not sure I understand why that needs to be done.
          Hide
          Alejandro Abdelnur added a comment - - edited

          My mistake then, I've misunderstood how you would use it, this means that is, as Tom indicated, a helper class. Then no need for an interface/implementation unless you have factory that hides the creation and the implementation returning an interface.

          Show
          Alejandro Abdelnur added a comment - - edited My mistake then, I've misunderstood how you would use it, this means that is, as Tom indicated, a helper class. Then no need for an interface/implementation unless you have factory that hides the creation and the implementation returning an interface.
          Hide
          Siddharth Seth added a comment -

          After discussing this offline with Vinod - couple of reasons this was done for the YarnClient and possibly elsewhere.

          • Often, we end up making private methods public for testing (with an annotation of-course). Having an interface allows this to be done in the implementing class only.
          • Also mocking can be a little cleaner with an interface.

          The interface shouldn't really be implemented by anyone outside of YARN - it exists primarily for cleaner code. Given this, I think it's Ok to have an interface, and it doesn't really limit adding methods. Alternately an abstract class to keep things clean. (The two should also stay in the same module - there's no requirement for the interface to sit in an API module and the impl elsewhere.)

          Show
          Siddharth Seth added a comment - After discussing this offline with Vinod - couple of reasons this was done for the YarnClient and possibly elsewhere. Often, we end up making private methods public for testing (with an annotation of-course). Having an interface allows this to be done in the implementing class only. Also mocking can be a little cleaner with an interface. The interface shouldn't really be implemented by anyone outside of YARN - it exists primarily for cleaner code. Given this, I think it's Ok to have an interface, and it doesn't really limit adding methods. Alternately an abstract class to keep things clean. (The two should also stay in the same module - there's no requirement for the interface to sit in an API module and the impl elsewhere.)
          Hide
          Alejandro Abdelnur added a comment -

          If developers (of AMs) suppose to use the interface only, then they should not see the implementing class. They should get an interface instance via a factory. The implementing class should not be in the same package or it should be package private. Using annotations in public API on methods to state they are for testing only is calling for trouble as developers will use those methods.

          As I assume this pattern will hold true for more than one interface/impl set, we could have a generic factory for this:

            public <T> T create(Class<T> klass, Configuration conf);
          

          Internally we can use our good old ReflectionUtils.newInstance() and drive the implementation from a configuration property.

          Show
          Alejandro Abdelnur added a comment - If developers (of AMs) suppose to use the interface only, then they should not see the implementing class. They should get an interface instance via a factory. The implementing class should not be in the same package or it should be package private. Using annotations in public API on methods to state they are for testing only is calling for trouble as developers will use those methods. As I assume this pattern will hold true for more than one interface/impl set, we could have a generic factory for this: public <T> T create( Class <T> klass, Configuration conf); Internally we can use our good old ReflectionUtils.newInstance() and drive the implementation from a configuration property.
          Hide
          Tom White added a comment -

          > The interface shouldn't really be implemented by anyone outside of YARN

          This is the heart of the problem. We don't have a way to say (via the audience annotations) that an interface is for read-only use only - and not for users to implement. An interface may be @Public @Stable from the point of view of a user who wants to call it, but that doesn't mean that folks should implement it themselves, since for interfaces like the one we are discussing we might want to add a new method, say (note that such a change is compatible with @Stable). Adding a new method is fine for the first type of user, but not for the second, since their implementation breaks.

          In this case, I think it's likely we'll add more methods. For example, it would be useful to add a waitForState method to YarnClient (which is also an interface), which waits for a given application to reach a particular YarnApplicationState. If YarnClient were a class then this would be a compatible change, but if it's an interface then it is not.

          I think we should do one of the following:
          1. Change YarnClient and AMRMClient to be concrete implementations.
          2. Leave the interface/implementation distinction and make the interfaces @Public @Unstable.

          I prefer 1. since these classes are helper classes - they are not a tightly-defined interface.

          Show
          Tom White added a comment - > The interface shouldn't really be implemented by anyone outside of YARN This is the heart of the problem. We don't have a way to say (via the audience annotations) that an interface is for read-only use only - and not for users to implement. An interface may be @Public @Stable from the point of view of a user who wants to call it, but that doesn't mean that folks should implement it themselves, since for interfaces like the one we are discussing we might want to add a new method, say (note that such a change is compatible with @Stable). Adding a new method is fine for the first type of user, but not for the second, since their implementation breaks. In this case, I think it's likely we'll add more methods. For example, it would be useful to add a waitForState method to YarnClient (which is also an interface), which waits for a given application to reach a particular YarnApplicationState. If YarnClient were a class then this would be a compatible change, but if it's an interface then it is not. I think we should do one of the following: 1. Change YarnClient and AMRMClient to be concrete implementations. 2. Leave the interface/implementation distinction and make the interfaces @Public @Unstable. I prefer 1. since these classes are helper classes - they are not a tightly-defined interface.
          Hide
          Alejandro Abdelnur added a comment -

          If we go that route I'd also prefer #1 and any method required for testing should be package private.

          Show
          Alejandro Abdelnur added a comment - If we go that route I'd also prefer #1 and any method required for testing should be package private.
          Hide
          Tom White added a comment -

          Some feedback on the current patch:

          • Why not use the existing ResourceRequest rather than creating a new type ContainerRequest? Having two equivalent types seems confusing.
          • Is the ANY constant meant to be used by users? It looks like you specify a ContainerRequest with null hosts and racks in this case. If so, then it would be useful to add a constructor that doesn't take hosts or racks for that case, although given my previous point it would be easier to use ResourceRequest.
          • assertTrue(containersRequestedRack == 2); appears on two successive lines. I think the assertion should be about containersRequestedAny.
          • "// do a few iterations to ensure RM is not going send new containers" - wait in the loop to allow NMs to heartbeat?
          Show
          Tom White added a comment - Some feedback on the current patch: Why not use the existing ResourceRequest rather than creating a new type ContainerRequest? Having two equivalent types seems confusing. Is the ANY constant meant to be used by users? It looks like you specify a ContainerRequest with null hosts and racks in this case. If so, then it would be useful to add a constructor that doesn't take hosts or racks for that case, although given my previous point it would be easier to use ResourceRequest. assertTrue(containersRequestedRack == 2); appears on two successive lines. I think the assertion should be about containersRequestedAny. "// do a few iterations to ensure RM is not going send new containers" - wait in the loop to allow NMs to heartbeat?
          Hide
          Bikas Saha added a comment -

          Looks like there are different views on interface usage. How about the following proposal?
          The next step after this gets committed is to write a version of the AMRMClient that is more advanced and handles things like auto-heartbeat and task->container matching. This will be useful for simple applications that dont need fine grained control over scheduling. After I am done writing that client then it will be clear if the interface API's apply or if its better to write that class with a different API. Based on that we can continue to keep the interface and add a factory or remove the interface. Does this sound like a way forward?

          Show
          Bikas Saha added a comment - Looks like there are different views on interface usage. How about the following proposal? The next step after this gets committed is to write a version of the AMRMClient that is more advanced and handles things like auto-heartbeat and task->container matching. This will be useful for simple applications that dont need fine grained control over scheduling. After I am done writing that client then it will be clear if the interface API's apply or if its better to write that class with a different API. Based on that we can continue to keep the interface and add a factory or remove the interface. Does this sound like a way forward?
          Hide
          Bikas Saha added a comment -

          Why not use the existing ResourceRequest...

          I am trying to create ContainerRequest akin to ContainerRequest in the MRAppMaster. This object will be used to map a given request for containers to a requestor object stored a reference (like the TaskAttemptId). This will enable the AMRMClient to hand back containers to specific tasks (ie assign containers to requestors) and do the necessary book-keeping. As compared to the current impl, in which the users are expected to do this assignment on their own after analyzing the results of the allocated request. This is logically different from a ResourceRequest which has no provenance info. Sorry this is not clear in the current code because the requestor reference is not present. It was giving warnings since its not used in this version of the client.

          The other comments have been fixed. ANY is no longer public and so users cannot use it. Ideally, we should have a YARN ANY constant so that everyone does not have to redefine it. Perhaps another jira?

          AMRMClientImpl.addResourceRequestToAsk - am not sure why this method is needed. A simple synchronized asks.add should be sufficient

          The change of the ResourceRequest comparator to be agnostic of container count necessitates the check being done in addResourceRequestToAsk. If ask already contains request FOO for 5 containers then it will not add a request FOO for 6 containers.

          Does this comment along with my previous one for interface vs class resolve the concerns?

          Show
          Bikas Saha added a comment - Why not use the existing ResourceRequest... I am trying to create ContainerRequest akin to ContainerRequest in the MRAppMaster. This object will be used to map a given request for containers to a requestor object stored a reference (like the TaskAttemptId). This will enable the AMRMClient to hand back containers to specific tasks (ie assign containers to requestors) and do the necessary book-keeping. As compared to the current impl, in which the users are expected to do this assignment on their own after analyzing the results of the allocated request. This is logically different from a ResourceRequest which has no provenance info. Sorry this is not clear in the current code because the requestor reference is not present. It was giving warnings since its not used in this version of the client. The other comments have been fixed. ANY is no longer public and so users cannot use it. Ideally, we should have a YARN ANY constant so that everyone does not have to redefine it. Perhaps another jira? AMRMClientImpl.addResourceRequestToAsk - am not sure why this method is needed. A simple synchronized asks.add should be sufficient The change of the ResourceRequest comparator to be agnostic of container count necessitates the check being done in addResourceRequestToAsk. If ask already contains request FOO for 5 containers then it will not add a request FOO for 6 containers. Does this comment along with my previous one for interface vs class resolve the concerns?
          Hide
          Bikas Saha added a comment -

          Attaching patch with review comments fixed. If this looks ok then I will break it up into 1) addition of AMRMClient 2) use of AMRMClient in DistributedShell as requested by Sid.

          Show
          Bikas Saha added a comment - Attaching patch with review comments fixed. If this looks ok then I will break it up into 1) addition of AMRMClient 2) use of AMRMClient in DistributedShell as requested by Sid.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12562368/YARN-103.8.patch
          against trunk revision .

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 2 new or modified test files.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed unit tests in hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-applications-distributedshell hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-YARN-Build/259//testReport/
          Console output: https://builds.apache.org/job/PreCommit-YARN-Build/259//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12562368/YARN-103.8.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 2 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-applications-distributedshell hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-YARN-Build/259//testReport/ Console output: https://builds.apache.org/job/PreCommit-YARN-Build/259//console This message is automatically generated.
          Hide
          Siddharth Seth added a comment -

          This is the heart of the problem. We don't have a way to say (via the audience annotations) that an interface is for read-only use only - and not for users to implement.

          Completely agree with this. Do you think it makes sense to add such an annotation ?
          I don't know the details - but I believe JobControl being a class in the 0.20 line was problematic - and it had to be changed to an interface. Curious as to why that was required.
          Marking this as unstable seems to be correct for now - irrespective of whether it's a class / interface. I'd be happier with an interface + factory. Like Bikas said, this could be resolved at a later point.

          The change of the ResourceRequest comparator to be agnostic of container count necessitates the check being done in addResourceRequestToAsk. If ask already contains request FOO for 5 containers then it will not add a request FOO for 6 containers.

          I still don't get why this is needed... maybe I'm missing something. The same resourceRequest being removed and added back doesn't seem very useful.

          Other than this one change, I'm +1 for committing this. Alejandro, Tom - if you don't have concerns - I'll commit this in the next couple of days.

          Show
          Siddharth Seth added a comment - This is the heart of the problem. We don't have a way to say (via the audience annotations) that an interface is for read-only use only - and not for users to implement. Completely agree with this. Do you think it makes sense to add such an annotation ? I don't know the details - but I believe JobControl being a class in the 0.20 line was problematic - and it had to be changed to an interface. Curious as to why that was required. Marking this as unstable seems to be correct for now - irrespective of whether it's a class / interface. I'd be happier with an interface + factory. Like Bikas said, this could be resolved at a later point. The change of the ResourceRequest comparator to be agnostic of container count necessitates the check being done in addResourceRequestToAsk. If ask already contains request FOO for 5 containers then it will not add a request FOO for 6 containers. I still don't get why this is needed... maybe I'm missing something. The same resourceRequest being removed and added back doesn't seem very useful. Other than this one change, I'm +1 for committing this. Alejandro, Tom - if you don't have concerns - I'll commit this in the next couple of days.
          Hide
          Bikas Saha added a comment -

          Clarified Sids question offline. Added a comment for that code that explains the logic. Removed DistributedShell changes. Attaching patch.

          Show
          Bikas Saha added a comment - Clarified Sids question offline. Added a comment for that code that explains the logic. Removed DistributedShell changes. Attaching patch.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12562540/YARN-103.9.patch
          against trunk revision .

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 1 new or modified test files.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed unit tests in hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-YARN-Build/264//testReport/
          Console output: https://builds.apache.org/job/PreCommit-YARN-Build/264//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12562540/YARN-103.9.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-YARN-Build/264//testReport/ Console output: https://builds.apache.org/job/PreCommit-YARN-Build/264//console This message is automatically generated.
          Hide
          Siddharth Seth added a comment -

          +1. Committing this. Thanks Bikas, also for the patience on this Jira - has taken a long time to go in.

          Show
          Siddharth Seth added a comment - +1. Committing this. Thanks Bikas, also for the patience on this Jira - has taken a long time to go in.
          Hide
          Siddharth Seth added a comment -

          Committed to trunk and branch-2.

          Show
          Siddharth Seth added a comment - Committed to trunk and branch-2.
          Hide
          Hudson added a comment -

          Integrated in Hadoop-trunk-Commit #3171 (See https://builds.apache.org/job/Hadoop-trunk-Commit/3171/)
          YARN-103. Add a yarn AM-RM client module. Contributed by Bikas Saha. (Revision 1428554)

          Result = SUCCESS
          sseth : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1428554
          Files :

          • /hadoop/common/trunk/hadoop-yarn-project/CHANGES.txt
          • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/pom.xml
          • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/AMRMClient.java
          • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/AMRMClientImpl.java
          • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/test/java/org/apache/hadoop/yarn/client/TestAMRMClient.java
          Show
          Hudson added a comment - Integrated in Hadoop-trunk-Commit #3171 (See https://builds.apache.org/job/Hadoop-trunk-Commit/3171/ ) YARN-103 . Add a yarn AM-RM client module. Contributed by Bikas Saha. (Revision 1428554) Result = SUCCESS sseth : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1428554 Files : /hadoop/common/trunk/hadoop-yarn-project/CHANGES.txt /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/pom.xml /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/AMRMClient.java /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/AMRMClientImpl.java /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/test/java/org/apache/hadoop/yarn/client/TestAMRMClient.java
          Hide
          Bikas Saha added a comment -

          Thanks guys! Appreciate the feedback. Will post a smarter client version soon.

          Show
          Bikas Saha added a comment - Thanks guys! Appreciate the feedback. Will post a smarter client version soon.
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Yarn-trunk #86 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/86/)
          YARN-103. Add a yarn AM-RM client module. Contributed by Bikas Saha. (Revision 1428554)

          Result = FAILURE
          sseth : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1428554
          Files :

          • /hadoop/common/trunk/hadoop-yarn-project/CHANGES.txt
          • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/pom.xml
          • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/AMRMClient.java
          • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/AMRMClientImpl.java
          • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/test/java/org/apache/hadoop/yarn/client/TestAMRMClient.java
          Show
          Hudson added a comment - Integrated in Hadoop-Yarn-trunk #86 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/86/ ) YARN-103 . Add a yarn AM-RM client module. Contributed by Bikas Saha. (Revision 1428554) Result = FAILURE sseth : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1428554 Files : /hadoop/common/trunk/hadoop-yarn-project/CHANGES.txt /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/pom.xml /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/AMRMClient.java /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/AMRMClientImpl.java /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/test/java/org/apache/hadoop/yarn/client/TestAMRMClient.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk #1275 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1275/)
          YARN-103. Add a yarn AM-RM client module. Contributed by Bikas Saha. (Revision 1428554)

          Result = FAILURE
          sseth : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1428554
          Files :

          • /hadoop/common/trunk/hadoop-yarn-project/CHANGES.txt
          • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/pom.xml
          • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/AMRMClient.java
          • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/AMRMClientImpl.java
          • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/test/java/org/apache/hadoop/yarn/client/TestAMRMClient.java
          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #1275 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1275/ ) YARN-103 . Add a yarn AM-RM client module. Contributed by Bikas Saha. (Revision 1428554) Result = FAILURE sseth : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1428554 Files : /hadoop/common/trunk/hadoop-yarn-project/CHANGES.txt /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/pom.xml /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/AMRMClient.java /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/AMRMClientImpl.java /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/test/java/org/apache/hadoop/yarn/client/TestAMRMClient.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-trunk #1305 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1305/)
          YARN-103. Add a yarn AM-RM client module. Contributed by Bikas Saha. (Revision 1428554)

          Result = SUCCESS
          sseth : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1428554
          Files :

          • /hadoop/common/trunk/hadoop-yarn-project/CHANGES.txt
          • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/pom.xml
          • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/AMRMClient.java
          • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/AMRMClientImpl.java
          • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/test/java/org/apache/hadoop/yarn/client/TestAMRMClient.java
          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #1305 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1305/ ) YARN-103 . Add a yarn AM-RM client module. Contributed by Bikas Saha. (Revision 1428554) Result = SUCCESS sseth : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1428554 Files : /hadoop/common/trunk/hadoop-yarn-project/CHANGES.txt /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/pom.xml /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/AMRMClient.java /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/AMRMClientImpl.java /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/test/java/org/apache/hadoop/yarn/client/TestAMRMClient.java

            People

            • Assignee:
              Bikas Saha
              Reporter:
              Bikas Saha
            • Votes:
              0 Vote for this issue
              Watchers:
              14 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development