----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37026/#review136733 -----------------------------------------------------------
Fix it, then Ship it! LGTM. We are also trying to test it internally in LinkedIn as well. If you can update w/ the fixes soon, we can verify the patch in parallel and merge it quickly. Thanks a lot! samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMaster.scala (line 114) <https://reviews.apache.org/r/37026/#comment201831> Just confirmed w/ Jagadish who had observed the application's behavior with final application status before, we actually need to cleanup even in FinalApplicationStatus.FAILED case as well. The only case that we should leave it is the "UNDEFINED" state. Any defined FinalApplicationStatus means that YARN won't try to restart the AM. Hence, we should cleanup. I have included that logic in patch-7 earlier (https://issues.apache.org/jira/secure/attachment/12808018/SAMZA-727.7.patch). samza-yarn/src/main/java/org/apache/samza/config/YarnConfig.java (line 110) <https://reviews.apache.org/r/37026/#comment201832> nit: prefer to fix the typo and make the constant string to be YARN_KERBEROS_PRINCIPAL and YARN_KERBEROS_KEYTAB samza-yarn/src/main/java/org/apache/samza/config/YarnConfig.java (line 197) <https://reviews.apache.org/r/37026/#comment201833> Same here. Should be getYarnKerberosPrincipal() and getYarnKerberosKeytab() - Yi Pan (Data Infrastructure) On June 8, 2016, 7:10 p.m., Chen Song wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/37026/ > ----------------------------------------------------------- > > (Updated June 8, 2016, 7:10 p.m.) > > > Review request for samza. > > > Repository: samza > > > Description > ------- > > Basic support for kerberization based on keytab authentication. > > > Diffs > ----- > > build.gradle 16facbb > samza-core/src/main/java/org/apache/samza/container/SecurityManager.java > PRE-CREATION > > samza-core/src/main/java/org/apache/samza/container/SecurityManagerFactory.java > PRE-CREATION > samza-core/src/main/scala/org/apache/samza/config/JobConfig.scala 4d5ca4d > samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala > 951d479 > samza-yarn/src/main/java/org/apache/samza/config/YarnConfig.java c556d83 > samza-yarn/src/main/scala/org/apache/samza/job/yarn/ClientHelper.scala > 74a0676 > samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMaster.scala > 7bd8131 > > samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMasterSecurityManager.scala > PRE-CREATION > > samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMasterService.scala > 3adf86f > > samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaContainerSecurityManager.scala > PRE-CREATION > > samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaYarnSecurityManagerFactory.scala > PRE-CREATION > samza-yarn/src/main/scala/org/apache/samza/job/yarn/YarnJob.scala 62ddb26 > samza-yarn/src/main/scala/org/apache/samza/job/yarn/YarnJobUtil.scala > PRE-CREATION > samza-yarn/src/test/scala/org/apache/samza/job/yarn/TestClientHelper.scala > PRE-CREATION > > samza-yarn/src/test/scala/org/apache/samza/job/yarn/TestSamzaAppMasterService.scala > fc0091f > > Diff: https://reviews.apache.org/r/37026/diff/ > > > Testing > ------- > > > Thanks, > > Chen Song > >
