----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34180/#review83810 -----------------------------------------------------------
agents-common/src/main/java/org/apache/ranger/plugin/util/TimedExecutor.java <https://reviews.apache.org/r/34180/#comment134877> We discussed about having the 'lookup-timeout' handled in ranger-admin (ServiceREST), without requireing/expecting each service/plugin to implement the timeout. Any reason to move this to plugins? agents-common/src/main/java/org/apache/ranger/plugin/util/TimedExecutor.java <https://reviews.apache.org/r/34180/#comment134872> is this block only for debug logging? If yes, perhaps this can be moved inside the following try {} block - for better readability. agents-common/src/main/java/org/apache/ranger/plugin/util/TimedExecutor.java <https://reviews.apache.org/r/34180/#comment134873> missing LOG.isDebugEnabled() here and the following LOG.debug() agents-common/src/main/java/org/apache/ranger/plugin/util/TimedExecutor.java <https://reviews.apache.org/r/34180/#comment134868> Printing the exception details in the log will be helpful in troubleshooting: LOG.warn(message, e); agents-common/src/main/java/org/apache/ranger/plugin/util/TimedExecutor.java <https://reviews.apache.org/r/34180/#comment134867> It will be good to call super.terminated() - even if the base class impl might be no-op for now. - Madhan Neethiraj On May 13, 2015, 9:56 p.m., Alok Lal wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/34180/ > ----------------------------------------------------------- > > (Updated May 13, 2015, 9:56 p.m.) > > > Review request for ranger, Abhay Kulkarni, Madhan Neethiraj, and Ramesh Mani. > > > Bugs: RANGER-265 > https://issues.apache.org/jira/browse/RANGER-265 > > > Repository: ranger > > > Description > ------- > > TimedUtils class has been fixed to actually run tasks async in a threadpool. > > > Diffs > ----- > > > agents-common/src/main/java/org/apache/ranger/plugin/service/ResourceLookupContext.java > 913f824 > > agents-common/src/main/java/org/apache/ranger/plugin/util/RangerObjectFactory.java > 4a570b6 > > agents-common/src/main/java/org/apache/ranger/plugin/util/TimedEventUtil.java > c6a7bbe > > agents-common/src/main/java/org/apache/ranger/plugin/util/TimedExecutor.java > PRE-CREATION > > agents-common/src/main/java/org/apache/ranger/plugin/util/TimedExecutorConfigurator.java > PRE-CREATION > > agents-common/src/test/java/org/apache/ranger/plugin/util/TestTimedExecutor.java > PRE-CREATION > > hbase-agent/src/main/java/org/apache/ranger/services/hbase/client/HBaseConnectionMgr.java > 5c1c73b > > hbase-agent/src/main/java/org/apache/ranger/services/hbase/client/HBaseResourceMgr.java > 4ce6a8d > > hdfs-agent/src/main/java/org/apache/ranger/services/hdfs/client/HdfsConnectionMgr.java > d62bb9c > > hdfs-agent/src/main/java/org/apache/ranger/services/hdfs/client/HdfsResourceMgr.java > 9161a5a > > hive-agent/src/main/java/org/apache/ranger/services/hive/client/HiveConnectionMgr.java > a5eda0b > > hive-agent/src/main/java/org/apache/ranger/services/hive/client/HiveResourceMgr.java > 98622b0 > > plugin-kafka/src/main/java/org/apache/ranger/services/kafka/client/ServiceKafkaClient.java > a62bd95 > > plugin-solr/src/main/java/org/apache/ranger/services/solr/client/ServiceSolrClient.java > 2e6d0ac > security-admin/pom.xml 9783d1f > security-admin/src/main/java/org/apache/ranger/biz/ServiceMgr.java 8498fbf > > security-admin/src/main/java/org/apache/ranger/common/LookupConfigurator.java > PRE-CREATION > security-admin/src/main/java/org/apache/ranger/common/PropertiesHelper.java > PRE-CREATION > security-admin/src/main/java/org/apache/ranger/common/TimedEventUtil.java > f833242 > security-admin/src/main/resources/conf.dist/ranger-admin-default-site.xml > 571d2a1 > security-admin/src/test/java/org/apache/ranger/biz/TestServiceMgr.java > PRE-CREATION > > security-admin/src/test/java/org/apache/ranger/common/TestLookupConfigurator.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/34180/diff/ > > > Testing > ------- > > New test added to assert the executor behaviors around: > - successful execution > - timeout during or waiting for execution > - rejection due to queue capacity limit > > > Thanks, > > Alok Lal > >
