----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47040/#review132942 -----------------------------------------------------------
common/src/java/org/apache/hive/common/util/HiveStringUtils.java (line 323) <https://reviews.apache.org/r/47040/#comment197204> Guava has Strings.isNullOrEmpty(). IF there is already a dependency on guava, just use that. ql/src/java/org/apache/hadoop/hive/ql/session/YarnFairScheduling.java (line 58) <https://reviews.apache.org/r/47040/#comment197207> It's unclear what validation is actually happening here. ql/src/java/org/apache/hadoop/hive/ql/session/YarnFairScheduling.java (line 65) <https://reviews.apache.org/r/47040/#comment197208> Would we ever expect a user to hit this case? If not, it should be converted to an invarient / precondition check. service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java (line 126) <https://reviews.apache.org/r/47040/#comment197212> We should continue to use the flag hive.server2.map.fair.scheduler.queue to determine whether or not to enable the mapping functionality shims/common/src/main/java/org/apache/hadoop/fs/FileSystemWatcher.java (line 42) <https://reviews.apache.org/r/47040/#comment197215> You might consider simplifying this a bit since we: a) don't care about watching multiple files. this should only support watching a single file. Can always extend later. b) don't care about multiple callbacks. Can always extend later. c) don't care about specific events For reference, a simplified version is in Impala. You might want to consider a similar approach: I think this can be simplified a bit. Impala did something similar, you might consider using a common approach: https://github.com/cloudera/Impala/blob/cdh5-trunk/fe/src/main/java/com/cloudera/impala/util/FileWatchService.java shims/scheduler/src/main/java/org/apache/hadoop/hive/schshim/FairSchedulerShim.java (line 135) <https://reviews.apache.org/r/47040/#comment197218> I don't think we need to support the case where the config file location has changed. Hive doesn't dynamically refresh the configs so I'm not sure we would see this. For now lets' keep this scoped to only detecting changes to the underlying file and using the same path for the course of the operation/ - Lenni Kuff On May 12, 2016, 1:16 p.m., Reuben Kuhnert wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/47040/ > ----------------------------------------------------------- > > (Updated May 12, 2016, 1:16 p.m.) > > > Review request for hive, Lenni Kuff, Mohit Sabharwal, and Sergio Pena. > > > Bugs: HIVE-13696 > https://issues.apache.org/jira/browse/HIVE-13696 > > > Repository: hive-git > > > Description > ------- > > Ensure that jobs sent to YARN with impersonation off are correctly routed to > the proper queue based on fair-scheduler.xml. Monitor this file for changes > and validate that jobs can only be sent to queues authorized for the user. > > > Diffs > ----- > > common/src/java/org/apache/hive/common/util/HiveStringUtils.java > 6d28396893532302fbbd66eace53ae32b71848c3 > ql/src/java/org/apache/hadoop/hive/ql/Driver.java > 3fecc5c4ca2a06a031c0c4a711fb49e757c49062 > ql/src/java/org/apache/hadoop/hive/ql/session/YarnFairScheduling.java > PRE-CREATION > service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java > a0015ebc655931f241b28c53fbb94cfe172841b1 > shims/common/src/main/java/org/apache/hadoop/fs/FileSystemWatcher.java > PRE-CREATION > shims/common/src/main/java/org/apache/hadoop/hive/shims/SchedulerShim.java > 63803b8b0752745bd2fedaccc5d100befd97093b > shims/scheduler/pom.xml b36c12325c588cdb609c6200b1edef73a2f79552 > > shims/scheduler/src/main/java/org/apache/hadoop/hive/schshim/FairSchedulerQueueAllocator.java > PRE-CREATION > > shims/scheduler/src/main/java/org/apache/hadoop/hive/schshim/FairSchedulerShim.java > 372244dc3c989d2a3ae2eb2bfb8cd0a235705e18 > > shims/scheduler/src/main/java/org/apache/hadoop/hive/schshim/QueueAllocator.java > PRE-CREATION > > shims/scheduler/src/test/java/org/apache/hadoop/hive/schshim/TestFairScheduler.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/47040/diff/ > > > Testing > ------- > > > Thanks, > > Reuben Kuhnert > >
