You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-dev@hadoop.apache.org by xiaoyuyao <gi...@git.apache.org> on 2016/03/26 05:23:41 UTC

[GitHub] hadoop pull request: HADOOP-12916

GitHub user xiaoyuyao opened a pull request:

    https://github.com/apache/hadoop/pull/86

    HADOOP-12916

    Create a github PR for code review of v04 patch on the JIRA. 

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/xiaoyuyao/hadoop HADOOP-12916

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/hadoop/pull/86.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #86
    
----
commit 93cf7377980a69f8b5b22453419a95b58a19c2aa
Author: Xiaoyu Yao <xy...@apache.org>
Date:   2016-03-26T02:45:48Z

    HADOOP-12916.patch.04

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] hadoop pull request: HADOOP-12916

Posted by xiaoyuyao <gi...@git.apache.org>.
Github user xiaoyuyao commented on a diff in the pull request:

    https://github.com/apache/hadoop/pull/86#discussion_r57619429
  
    --- Diff: hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java ---
    @@ -1636,6 +1640,15 @@ public long getTimeDuration(String name, long defaultValue, TimeUnit unit) {
         return unit.convert(Long.parseLong(vStr), vUnit.unit());
       }
     
    +  public long[] getTimeDurations(String name, TimeUnit unit) {
    --- End diff --
    
    Good point. I will address that in the next patch.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] hadoop pull request: HADOOP-12916

Posted by arp7 <gi...@git.apache.org>.
Github user arp7 commented on a diff in the pull request:

    https://github.com/apache/hadoop/pull/86#discussion_r57606818
  
    --- Diff: hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/CallQueueManager.java ---
    @@ -49,25 +57,68 @@
       private final AtomicReference<BlockingQueue<E>> putRef;
       private final AtomicReference<BlockingQueue<E>> takeRef;
     
    +  private RpcScheduler scheduler;
    +
       public CallQueueManager(Class<? extends BlockingQueue<E>> backingClass,
    +                          Class<? extends RpcScheduler> schedulerClass,
           boolean clientBackOffEnabled, int maxQueueSize, String namespace,
           Configuration conf) {
    +    int priorityLevels = parseNumLevels(namespace, conf);
    +    this.scheduler = createScheduler(schedulerClass, priorityLevels,
    +        namespace, conf);
         BlockingQueue<E> bq = createCallQueueInstance(backingClass,
    -      maxQueueSize, namespace, conf);
    +        priorityLevels, maxQueueSize, namespace, conf);
         this.clientBackOffEnabled = clientBackOffEnabled;
         this.putRef = new AtomicReference<BlockingQueue<E>>(bq);
         this.takeRef = new AtomicReference<BlockingQueue<E>>(bq);
         LOG.info("Using callQueue " + backingClass);
       }
     
    +  private static <T extends RpcScheduler> T createScheduler(
    +      Class<T> theClass, int priorityLevels, String ns, Configuration conf) {
    +    // Used for custom, configurable scheduler
    +    try {
    +      Constructor<T> ctor = theClass.getDeclaredConstructor(int.class,
    +          String.class, Configuration.class);
    +      return ctor.newInstance(priorityLevels, ns, conf);
    +    } catch (RuntimeException e) {
    --- End diff --
    
    See HDFS-9478 which is fixing exception handling when constructing callqueue instances. We could use a similar fix for createScheduler.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] hadoop pull request: HADOOP-12916

Posted by arp7 <gi...@git.apache.org>.
Github user arp7 commented on a diff in the pull request:

    https://github.com/apache/hadoop/pull/86#discussion_r57606373
  
    --- Diff: hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java ---
    @@ -1636,6 +1640,15 @@ public long getTimeDuration(String name, long defaultValue, TimeUnit unit) {
         return unit.convert(Long.parseLong(vStr), vUnit.unit());
       }
     
    +  public long[] getTimeDurations(String name, TimeUnit unit) {
    --- End diff --
    
    Consider returning a List.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---