Return-Path: X-Original-To: archive-asf-public-internal@cust-asf2.ponee.io Delivered-To: archive-asf-public-internal@cust-asf2.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by cust-asf2.ponee.io (Postfix) with ESMTP id 76271200BE8 for ; Fri, 23 Dec 2016 14:21:00 +0100 (CET) Received: by cust-asf.ponee.io (Postfix) id 74DC5160B1E; Fri, 23 Dec 2016 13:21:00 +0000 (UTC) Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by cust-asf.ponee.io (Postfix) with SMTP id C112C160B1D for ; Fri, 23 Dec 2016 14:20:59 +0100 (CET) Received: (qmail 23651 invoked by uid 500); 23 Dec 2016 13:20:59 -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 23636 invoked by uid 99); 23 Dec 2016 13:20:58 -0000 Received: from arcas.apache.org (HELO arcas) (140.211.11.28) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 23 Dec 2016 13:20:58 +0000 Received: from arcas.apache.org (localhost [127.0.0.1]) by arcas (Postfix) with ESMTP id 89E7E2C2A67 for ; Fri, 23 Dec 2016 13:20:58 +0000 (UTC) Date: Fri, 23 Dec 2016 13:20:58 +0000 (UTC) From: "Naganarasimha G R (JIRA)" To: yarn-issues@hadoop.apache.org Message-ID: In-Reply-To: References: Subject: [jira] [Commented] (YARN-6025) Few issues in synchronization in CapacityScheduler & AbstractYarnScheduler MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-JIRA-FingerPrint: 30527f35849b9dde25b450d4833f0394 archived-at: Fri, 23 Dec 2016 13:21:00 -0000 [ https://issues.apache.org/jira/browse/YARN-6025?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15772885#comment-15772885 ] Naganarasimha G R commented on YARN-6025: ----------------------------------------- Thanks [~wangda], bq. Should not remove synchronized lock of recoverContainersOnNode / nodeUpdate in AbstractYarnScheduler, as they are used by FifoScheduler IMHO FifoScheduler should extend and get it synchronized if required (nodeUpdate is already done in that way) because other schedulers are not dependent on it and better to avoid holding both kind of locks (reentrant lock and object's lock) for other schedulers. bq. Should keep readlock inside CS#nodeUpdate, since it is possible that collection of applications / queues / nodes could be updated during the call. The readlock is added to protect this happens. May be i dint get it properly but based on my understanding either we need to hold write lock in CS and invoke {{super.nodeUpdate}} or ensure inside the AbstractYarnScheduler sufficient locking is there so that downstream classes need not worry. And further updates to applications, & queues happens with holding locks internally, so was not clear whether its required. > Few issues in synchronization in CapacityScheduler & AbstractYarnScheduler > -------------------------------------------------------------------------- > > Key: YARN-6025 > URL: https://issues.apache.org/jira/browse/YARN-6025 > Project: Hadoop YARN > Issue Type: Sub-task > Components: capacity scheduler, scheduler > Reporter: Naganarasimha G R > Assignee: Naganarasimha G R > Attachments: YARN-6025.01.patch > > > YARN-3139 does optimization on the locks by introducing ReentrantReadWriteLock to remove synchronized but seems to have some issues. > # CapacityScheduler > #* nodeUpdate(RMNode) need not be synchronized, as its the only one to be in the class > #* setLastNodeUpdateTime in nodeUpdate needs to be updated with readLock ? then getLastNodeUpdateTime is done without any lock and more over its volatile. > #* getUserGroupMappingPlacementRule need not be public as its held called within and not used in test and further is called from initScheduler and reinitialize where both are holding write locks so i presume getting read locks are of no use. > # AbstractYarnScheduler > #* recoverContainersOnNode is synchronized as well as holds write lock on the complete method so i presume we do not require synchronized here. > #* nodeUpdate method too is synchronized but if i see the updates done inside i do not see any place where node update from two different nodes will have any issues (except for schedulerHealth which is taken care internally with concurrentHashMap), And even if require we could better use write lock. (also depends on the decision of next point) > #* readLock is only used in containerLaunchedOnNode which i am not completely sure whether its required to have a read lock here, suppose we do not require then whether there is any use of read write locks in AbstractYarnScheduler as in general there is performance overhead in using readwrite lock over synchronized blocks on frequently accessed code path. -- This message was sent by Atlassian JIRA (v6.3.4#6332) --------------------------------------------------------------------- To unsubscribe, e-mail: yarn-issues-unsubscribe@hadoop.apache.org For additional commands, e-mail: yarn-issues-help@hadoop.apache.org