From dev-return-76004-archive-asf-public=cust-asf.ponee.io@zookeeper.apache.org Wed Nov 21 00:42:05 2018 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by mx-eu-01.ponee.io (Postfix) with SMTP id 5DC2318064E for ; Wed, 21 Nov 2018 00:42:04 +0100 (CET) Received: (qmail 89008 invoked by uid 500); 20 Nov 2018 23:42:03 -0000 Mailing-List: contact dev-help@zookeeper.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@zookeeper.apache.org Delivered-To: mailing list dev@zookeeper.apache.org Received: (qmail 88997 invoked by uid 99); 20 Nov 2018 23:42:03 -0000 Received: from pnap-us-west-generic-nat.apache.org (HELO spamd1-us-west.apache.org) (209.188.14.142) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 20 Nov 2018 23:42:03 +0000 Received: from localhost (localhost [127.0.0.1]) by spamd1-us-west.apache.org (ASF Mail Server at spamd1-us-west.apache.org) with ESMTP id 0E84ED1414 for ; Tue, 20 Nov 2018 23:42:03 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamd1-us-west.apache.org X-Spam-Flag: NO X-Spam-Score: -109.501 X-Spam-Level: X-Spam-Status: No, score=-109.501 tagged_above=-999 required=6.31 tests=[ENV_AND_HDR_SPF_MATCH=-0.5, KAM_ASCII_DIVIDERS=0.8, RCVD_IN_DNSWL_MED=-2.3, SPF_PASS=-0.001, USER_IN_DEF_SPF_WL=-7.5, USER_IN_WHITELIST=-100] autolearn=disabled Received: from mx1-lw-us.apache.org ([10.40.0.8]) by localhost (spamd1-us-west.apache.org [10.40.0.7]) (amavisd-new, port 10024) with ESMTP id ofqflxPt7uE0 for ; Tue, 20 Nov 2018 23:42: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 602E661399 for ; Tue, 20 Nov 2018 23:42: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 D3C5CE0E5D for ; Tue, 20 Nov 2018 23:42: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 75DCB20FEB for ; Tue, 20 Nov 2018 23:42:00 +0000 (UTC) Date: Tue, 20 Nov 2018 23:42:00 +0000 (UTC) From: "Michael K. Edwards (JIRA)" To: dev@zookeeper.apache.org Message-ID: In-Reply-To: References: Subject: [jira] [Comment Edited] (ZOOKEEPER-2778) Potential server deadlock between follower sync with leader and follower receiving external connection requests. MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-JIRA-FingerPrint: 30527f35849b9dde25b450d4833f0394 [ https://issues.apache.org/jira/browse/ZOOKEEPER-2778?page=3Dcom.atlas= sian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=3D= 16693898#comment-16693898 ]=20 Michael K. Edwards edited comment on ZOOKEEPER-2778 at 11/20/18 11:41 PM: -------------------------------------------------------------------------- From what I'm seeing, it would be a crashing bug for `getQuorumAddress()` (= which cannot be marked `protected`, because it's called by has-a holders of= a QuorumPeer reference rather than by is-a subclasses of QuorumPeer, but c= an and should be package-private) to be called before the addresses are set= . The only call to `getClientAddress()` (which should be `private`) is in = `processReconfig()`, and it's appropriate for it to return `null` if called= early. This leaves `getElectionAddress()`, which again is pseudo-protecte= d and would produce a crash if called before the addresses are set. So the actual problem here is that, if the election address is not yet know= n, there's no safe return value from `getElectionAddress()` in the race sce= nario cited in the bug description. This "fix" =E2=80=93 hanm's or mine = =E2=80=93 will turn it into an NPE instead of a deadlock. This might be addressable by ensuring that code that needs the `QV_LOCK` fo= r a `QuorumPeer` associated with a `QuorumCnxManager` (to protect the macro= scopic critical sections in `QuorumCnxManager.connectOne()`, `QuorumPeer.se= tLastSeenQuorumVerifier()`, and `QuorumPeer.setQuorumVerifier()`) always ta= kes the lock on the `QuorumCnxManager` instance first. Looking into that. was (Author: mkedwards): From what I'm seeing, it would be a crashing bug for `getQuorumAddress()` (= which cannot be marked `protected`, because it's called by has-a holders of= a QuorumPeer reference rather than by is-a subclasses of QuorumPeer, but c= an and should be package-private) to be called before the addresses are set= .=C2=A0 The only call to `getClientAddress()` (which should be `private`) i= s in `processReconfig()`, and it's appropriate for it to return `null` if c= alled early.=C2=A0 This leaves `getElectionAddress()`, which again is pseud= o-protected and would produce a crash if called before the addresses are se= t. So the actual problem here is that, if the election address is not yet know= n, there's no safe return value from `getElectionAddress()` in the race sce= nario cited in the bug description.=C2=A0 This "fix" =E2=80=93 hanm's or mi= ne =E2=80=93 will turn it into an NPE instead of a deadlock. This might be addressable by ensuring that=C2=A0code that needs the `QV_LOC= K` for a `QuorumPeer` associated with a `QuorumCnxManager`=C2=A0(to protect= the macroscopic critical sections in `QuorumCnxManager.connectOne()`, `Quo= rumPeer.setLastSeenQuorumVerifier()`, and `QuorumPeer.setQuorumVerifier()`)= always takes the lock on the `QuorumCnxManager` instance first.=C2=A0 Look= ing into that. > Potential server deadlock between follower sync with leader and follower = receiving external connection requests. > -------------------------------------------------------------------------= --------------------------------------- > > Key: ZOOKEEPER-2778 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2778 > Project: ZooKeeper > Issue Type: Bug > Components: quorum > Affects Versions: 3.5.3 > Reporter: Michael Han > Priority: Blocker > Labels: pull-request-available > Fix For: 3.6.0, 3.5.5 > > Time Spent: 1h 20m > Remaining Estimate: 0h > > It's possible to have a deadlock during recovery phase.=20 > Found this issue by analyzing thread dumps of "flaky" ReconfigRecoveryTes= t [1]. . Here is a sample thread dump that illustrates the state of the exe= cution: > {noformat} > [junit] java.lang.Thread.State: BLOCKED > [junit] at org.apache.zookeeper.server.quorum.QuorumPeer.get= ElectionAddress(QuorumPeer.java:686) > [junit] at org.apache.zookeeper.server.quorum.QuorumCnxManag= er.initiateConnection(QuorumCnxManager.java:265) > [junit] at org.apache.zookeeper.server.quorum.QuorumCnxManag= er.connectOne(QuorumCnxManager.java:445) > [junit] at org.apache.zookeeper.server.quorum.QuorumCnxManag= er.receiveConnection(QuorumCnxManager.java:369) > [junit] at org.apache.zookeeper.server.quorum.QuorumCnxManag= er$Listener.run(QuorumCnxManager.java:642) > [junit]=20 > [junit] java.lang.Thread.State: BLOCKED > [junit] at org.apache.zookeeper.server.quorum.QuorumCnxManag= er.connectOne(QuorumCnxManager.java:472) > [junit] at org.apache.zookeeper.server.quorum.QuorumPeer.con= nectNewPeers(QuorumPeer.java:1438) > [junit] at org.apache.zookeeper.server.quorum.QuorumPeer.set= LastSeenQuorumVerifier(QuorumPeer.java:1471) > [junit] at org.apache.zookeeper.server.quorum.Learner.syncWi= thLeader(Learner.java:520) > [junit] at org.apache.zookeeper.server.quorum.Follower.follo= wLeader(Follower.java:88) > [junit] at org.apache.zookeeper.server.quorum.QuorumPeer.run= (QuorumPeer.java:1133) > {noformat} > The dead lock happens between the quorum peer thread which running the fo= llower that doing sync with leader work, and the listener of the qcm of the= same quorum peer that doing the receiving connection work. Basically to fi= nish sync with leader, the follower needs to synchronize on both QV_LOCK an= d the qmc object it owns; while in the receiver thread to finish setup an i= ncoming connection the thread needs to synchronize on both the qcm object t= he quorum peer owns, and the same QV_LOCK. It's easy to see the problem her= e is the order of acquiring two locks are different, thus depends on timing= / actual execution order, two threads might end up acquiring one lock whil= e holding another. > [1] org.apache.zookeeper.server.quorum.ReconfigRecoveryTest.testCurrentSe= rversAreObserversInNextConfig -- This message was sent by Atlassian JIRA (v7.6.3#76005)