cameronlee314 opened a new pull request #1515:
URL: https://github.com/apache/samza/pull/1515


   Issues:
   1. `JobModelManager` has a public static method just for calculating 
`JobModel` (i.e. `readJobModel`) but it also has a factory method (i.e. 
`apply`) which creates the context necessary for calculating `JobModel`. Some 
components only use `readJobModel` while others use `apply`. This makes the 
class usage somewhat inconsistent and unclean.
   2. `JobModelManager.apply` is directly coupled with an `HttpServer` for 
communicating with workers. This means any other logic in 
`JobModelManager.apply` is tied to `HttpServer`, which may not be desirable in 
the future. There will be a follow-up PR which decouples `JobModel` creation 
with the communication layer.
   3. `JobModelManager` is written in scala, which is harder to maintain with 
the significant java usage in most of the rest of Samza.
   
   Changes:
   1. Extract `JobModelManager.readJobModel` into a separate class 
`JobModelCalculator` which is only responsible for calculating `JobModel`. 
Convert this logic from scala to java.
   2. Extract some of the logic of `JobModelManager.apply` (get grouper 
metadata, update task assignments) into a separate class `JobModelHelper` so 
that logic is not tied to `HttpServer` for communication. Convert this logic 
from scala to java.
   3. Fix a bug regarding deletion of standby task assignments. Assignments 
should get cleared when the old set of standby tasks is not the same as the new 
set of standby tasks, but that was not happening because the condition was 
checking for equality and it wasn't comparing the same type of objects.
   4. Add more tests around the job model logic.
   
   Tests:
   1. Wrote several more unit tests for the logic that got refactored. I was 
able to run the tests against the old logic as well to make sure they passed 
before and after the refactoring (the tests involving standbys did fail with 
the old logic due to the bug described in Change 3 above, and they pass with 
the new logic).
   
   API changes and upgrade/usage instructions: N/A


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to