----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52168/ -----------------------------------------------------------
(Updated Feb. 16, 2017, 6:26 a.m.) Review request for samza. Changes ------- Changes that are related to JobCoordinator.scala: a) This change moves the logic implemented in the initializeJobModel method to recompute & write the new changeLogPartitionMapping to coordinator stream to apply method (there by making the initializeJobModel(purely readonly) and it can be exposed via a resource). This will make initializeJobModel just a reader of jobModel [JobCoordinator.scala Lines 120-134 has the change]. b) Removes refreshJobModel method in JobCoordinator and merges it with initializeJobModel. After the current change both the refreshJobModel & initializeJobModel each contains portions of code that reads the JobModel from coordinator stream, hence the reason to merge them into a single method. Also, there are no other uses of refreshJobModel method except this, hence the felt the need to merge them into one [JobCoordinator.scala lines 198-220]. c) Moves the changlogParititonManager.start(), localityManager.start() to apply method, since both the objects are created there. I think it’s better to do it over there, more like bootstrap/initializing them properly. I can’t wrap my head to understand why the existing implementation had it scattered across different methods, there by making it harder to understand the code and the flow [JobCoordinator.scala Lines 95-99]. d) Extracted out the logic to build systemAdmins into a method getSystemAdmins(config), so that it can be used from the SamzaTaskProxy resource while invoking initializeJobModel to build the jobModel. This extraction might seem unnecessary, but it removes code duplication and promotes reusability in samza-rest. [JobCoordinator.scala lines 274-288]. e) Stop the coordinatorStreamProducer and coordinatorStreamConsumer in apply method in finally block. This is done since both the producer and consumer will no longer be required for the reading/updating of jobModel at the end of apply method.[JobCoordinator.scala lines 139-145]. f) Moved reWriteConfig from JobRunner.scala to Util.scala, since it's required in samza-rest to apply custom rewriters to rewrite the job config before fetching from coordinatorStream[JobRunner.scala Lines 39-61]. Apart from these, the other changes in JobCoordinator.scala are minor name changes and extracting variable that should've been constants, fixing the imports(These were done with an intent to improve readability) JobCoordinator.scala Lines (33-39, 42-51, 63, 91, 188-189). This refactoring was done with a goal that the jobModel.read in JobCoordinator.scala is not reimplemented entirely in samza-rest. SamzaTaskProxy #getJobModel contains the bootstrap of CoordinatorStreamSystemConsumer, CoordinatorStreamSystemProducer to read the job model from coordinator stream. Tested the changes with a sample samza job and with samza-rest monitor. Repository: samza Description ------- This patch contains the following changes * Http get api to list the complete details of all the tasks that belongs to a job. * Refactored some methods in coordinator stream, to reuse the existing functionality of getting jobConfig from the coordinator stream. Diffs (updated) ----- docs/learn/documentation/versioned/rest/resource-directory.md 79746d1e2eb3491e4bd26c3c7cf6c7efd150d8ef docs/learn/documentation/versioned/rest/resources/tasks.md PRE-CREATION samza-core/src/main/scala/org/apache/samza/coordinator/JobCoordinator.scala df63b97e9d598ecd1840111ba490a723e410d089 samza-core/src/main/scala/org/apache/samza/job/JobRunner.scala 022b480856483059cb9f837a08f97a718bc04c31 samza-core/src/main/scala/org/apache/samza/util/Util.scala c4836f202f7eda1d4e71eac94fd48e46207b0316 samza-rest/src/main/config/samza-rest.properties 7be0b47d1466d2199ae278247e8d81522fb6a91c samza-rest/src/main/java/org/apache/samza/rest/model/Partition.java PRE-CREATION samza-rest/src/main/java/org/apache/samza/rest/model/Task.java PRE-CREATION samza-rest/src/main/java/org/apache/samza/rest/proxy/job/AbstractJobProxy.java 4d8647f3e1e650632e38b47ba5a8a2dac004f545 samza-rest/src/main/java/org/apache/samza/rest/proxy/job/JobProxyFactory.java 067711a74e5b0d7277a9c8b2d2517b56e9cfbcca samza-rest/src/main/java/org/apache/samza/rest/proxy/job/SimpleYarnJobProxy.java a935c98730f85f448c688a6baf2e8ddffdbb2cb4 samza-rest/src/main/java/org/apache/samza/rest/proxy/job/SimpleYarnJobProxyFactory.java 11d93d4608d23a4e3fb3bfc50dfac35ab6dbdf3c samza-rest/src/main/java/org/apache/samza/rest/proxy/task/SamzaTaskProxy.java PRE-CREATION samza-rest/src/main/java/org/apache/samza/rest/proxy/task/SamzaTaskProxyFactory.java PRE-CREATION samza-rest/src/main/java/org/apache/samza/rest/proxy/task/TaskProxy.java PRE-CREATION samza-rest/src/main/java/org/apache/samza/rest/proxy/task/TaskProxyFactory.java PRE-CREATION samza-rest/src/main/java/org/apache/samza/rest/proxy/task/TaskResourceConfig.java PRE-CREATION samza-rest/src/main/java/org/apache/samza/rest/resources/BaseResourceConfig.java PRE-CREATION samza-rest/src/main/java/org/apache/samza/rest/resources/DefaultResourceFactory.java e0224c6bcf4aeaa336e5786ac472482507fcd382 samza-rest/src/main/java/org/apache/samza/rest/resources/JobsResource.java a566db598c284d69ea61af88fdc0851483d5a089 samza-rest/src/main/java/org/apache/samza/rest/resources/JobsResourceConfig.java 527482d2ee55747e7b3f9c54c8a3b1afe7ad8797 samza-rest/src/main/java/org/apache/samza/rest/resources/Responses.java PRE-CREATION samza-rest/src/main/java/org/apache/samza/rest/resources/TasksResource.java PRE-CREATION samza-rest/src/test/java/org/apache/samza/rest/resources/TestJobsResource.java 7db437b348ecd286185898b8f8ab0220d59da71a samza-rest/src/test/java/org/apache/samza/rest/resources/TestTasksResource.java PRE-CREATION samza-rest/src/test/java/org/apache/samza/rest/resources/mock/MockInstallationFinder.java PRE-CREATION samza-rest/src/test/java/org/apache/samza/rest/resources/mock/MockResourceFactory.java PRE-CREATION samza-rest/src/test/java/org/apache/samza/rest/resources/mock/MockTaskProxy.java PRE-CREATION samza-rest/src/test/java/org/apache/samza/rest/resources/mock/MockTaskProxyFactory.java PRE-CREATION Diff: https://reviews.apache.org/r/52168/diff/ Testing ------- Manual and unit testing has been done to verify the apis. Thanks, Shanthoosh Venkataraman
