From yarn-issues-return-118240-apmail-hadoop-yarn-issues-archive=hadoop.apache.org@hadoop.apache.org Mon Jul 24 21:15:02 2017 Return-Path: X-Original-To: apmail-hadoop-yarn-issues-archive@minotaur.apache.org Delivered-To: apmail-hadoop-yarn-issues-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id D073D19D25 for ; Mon, 24 Jul 2017 21:15:02 +0000 (UTC) Received: (qmail 57314 invoked by uid 500); 24 Jul 2017 21:15:02 -0000 Delivered-To: apmail-hadoop-yarn-issues-archive@hadoop.apache.org Received: (qmail 57275 invoked by uid 500); 24 Jul 2017 21:15:02 -0000 Mailing-List: contact yarn-issues-help@hadoop.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Delivered-To: mailing list yarn-issues@hadoop.apache.org Received: (qmail 57263 invoked by uid 99); 24 Jul 2017 21:15:02 -0000 Received: from pnap-us-west-generic-nat.apache.org (HELO spamd4-us-west.apache.org) (209.188.14.142) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 24 Jul 2017 21:15:02 +0000 Received: from localhost (localhost [127.0.0.1]) by spamd4-us-west.apache.org (ASF Mail Server at spamd4-us-west.apache.org) with ESMTP id 2C4F2C006E for ; Mon, 24 Jul 2017 21:15:02 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamd4-us-west.apache.org X-Spam-Flag: NO X-Spam-Score: -100.002 X-Spam-Level: X-Spam-Status: No, score=-100.002 tagged_above=-999 required=6.31 tests=[RP_MATCHES_RCVD=-0.001, SPF_PASS=-0.001, USER_IN_WHITELIST=-100] autolearn=disabled Received: from mx1-lw-us.apache.org ([10.40.0.8]) by localhost (spamd4-us-west.apache.org [10.40.0.11]) (amavisd-new, port 10024) with ESMTP id FYjpyZmwiMjJ for ; Mon, 24 Jul 2017 21:15:01 +0000 (UTC) Received: from mailrelay1-us-west.apache.org (mailrelay1-us-west.apache.org [209.188.14.139]) by mx1-lw-us.apache.org (ASF Mail Server at mx1-lw-us.apache.org) with ESMTP id 345D85FBDF for ; Mon, 24 Jul 2017 21:15:01 +0000 (UTC) Received: from jira-lw-us.apache.org (unknown [207.244.88.139]) by mailrelay1-us-west.apache.org (ASF Mail Server at mailrelay1-us-west.apache.org) with ESMTP id BF3DDE0959 for ; Mon, 24 Jul 2017 21:15:00 +0000 (UTC) Received: from jira-lw-us.apache.org (localhost [127.0.0.1]) by jira-lw-us.apache.org (ASF Mail Server at jira-lw-us.apache.org) with ESMTP id 5A20C23F05 for ; Mon, 24 Jul 2017 21:15:00 +0000 (UTC) Date: Mon, 24 Jul 2017 21:15:00 +0000 (UTC) From: "Daniel Templeton (JIRA)" To: yarn-issues@hadoop.apache.org Message-ID: In-Reply-To: References: Subject: [jira] [Commented] (YARN-6307) Refactor FairShareComparator#compare MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-JIRA-FingerPrint: 30527f35849b9dde25b450d4833f0394 [ https://issues.apache.org/jira/browse/YARN-6307?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16099134#comment-16099134 ] Daniel Templeton commented on YARN-6307: ---------------------------------------- Nice patch, [~yufeigu]. Here are my comments: # Since we're unnesting all the _if_ blocks, let's take do that here, too: {code} if (res == 0) { // Apps are tied in fairness ratio. Break the tie by submit time and job // name to get a deterministic ordering, which is useful for unit tests. res = (int) Math.signum(s1.getStartTime() - s2.getStartTime()); if (res == 0) { res = s1.getName().compareTo(s2.getName()); } }{code} # Let's not stack the declarations: {code} double useToWeightRatio1, useToWeightRatio2;{code} Otherwise, looks good. > Refactor FairShareComparator#compare > ------------------------------------ > > Key: YARN-6307 > URL: https://issues.apache.org/jira/browse/YARN-6307 > Project: Hadoop YARN > Issue Type: Bug > Components: fairscheduler > Reporter: Yufei Gu > Assignee: Yufei Gu > Attachments: YARN-6307.001.patch, YARN-6307.002.patch > > > The method does three things: compare the min share usage, compare fair share usage by checking weight ratio, break tied by submit time and name. They are mixed with each other which is not easy to read and maintenance, poor style. Additionally, there are potential performance issues, like no need to check weight ratio if minShare usage comparison already indicate the order. It is worth to improve considering huge amount invokings in scheduler. -- This message was sent by Atlassian JIRA (v6.4.14#64029) --------------------------------------------------------------------- To unsubscribe, e-mail: yarn-issues-unsubscribe@hadoop.apache.org For additional commands, e-mail: yarn-issues-help@hadoop.apache.org