From api-return-1088-archive-asf-public=cust-asf.ponee.io@directory.apache.org Tue Mar 23 13:24:53 2021 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mxout1-ec2-va.apache.org (mxout1-ec2-va.apache.org [3.227.148.255]) by mx-eu-01.ponee.io (Postfix) with ESMTPS id 195D51804BB for ; Tue, 23 Mar 2021 14:24:53 +0100 (CET) Received: from mail.apache.org (mailroute1-lw-us.apache.org [207.244.88.153]) by mxout1-ec2-va.apache.org (ASF Mail Server at mxout1-ec2-va.apache.org) with SMTP id 569CB42F6C for ; Tue, 23 Mar 2021 13:24:52 +0000 (UTC) Received: (qmail 22481 invoked by uid 500); 23 Mar 2021 13:24:52 -0000 Mailing-List: contact api-help@directory.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Delivered-To: mailing list api@directory.apache.org Received: (qmail 22465 invoked by uid 99); 23 Mar 2021 13:24:51 -0000 Received: from spamproc1-he-de.apache.org (HELO spamproc1-he-de.apache.org) (116.203.196.100) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 23 Mar 2021 13:24:51 +0000 Received: from localhost (localhost [127.0.0.1]) by spamproc1-he-de.apache.org (ASF Mail Server at spamproc1-he-de.apache.org) with ESMTP id 0A6F71FF471 for ; Tue, 23 Mar 2021 13:24:51 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamproc1-he-de.apache.org X-Spam-Flag: NO X-Spam-Score: 3.45 X-Spam-Level: *** X-Spam-Status: No, score=3.45 tagged_above=-999 required=6.31 tests=[ENA_SUBJ_ODD_CASE=3.2, HEADER_FROM_DIFFERENT_DOMAINS=0.249, RCVD_IN_MSPIKE_H3=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_PASS=-0.001] autolearn=disabled Received: from mx1-ec2-va.apache.org ([116.203.227.195]) by localhost (spamproc1-he-de.apache.org [116.203.196.100]) (amavisd-new, port 10024) with ESMTP id DgW9_GOdVAKf for ; Tue, 23 Mar 2021 13:24:50 +0000 (UTC) Received-SPF: Pass (mailfrom) identity=mailfrom; client-ip=209.85.210.47; helo=mail-ot1-f47.google.com; envelope-from=shawn.michael.mckinney@gmail.com; receiver= Received: from mail-ot1-f47.google.com (mail-ot1-f47.google.com [209.85.210.47]) by mx1-ec2-va.apache.org (ASF Mail Server at mx1-ec2-va.apache.org) with ESMTPS id 48CC6BD0E3 for ; Tue, 23 Mar 2021 13:24:50 +0000 (UTC) Received: by mail-ot1-f47.google.com with SMTP id w31-20020a9d36220000b02901f2cbfc9743so19130000otb.7 for ; Tue, 23 Mar 2021 06:24:50 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:content-transfer-encoding:mime-version :subject:date:references:to:in-reply-to:message-id; bh=26Er+Kwm8B1U29LPfP4EuAKMyA2q6XemjYmFZdRm92o=; b=nyJfIWBVk0aUvA2KkBunqnUV7Mx+LgofrYZCOiW1E5g2gUZPj33hDwTnzRlZZ1SI74 mmtFGUlzJ7e+r1+VkaBURSox7Gxo0rfxiFOi+enwkzNdryI0HxQwNfD/nuZjJEp/lHj/ jpwwfg3EAfUzw1Xlp3ePannf9+yzGN8Mflg3Lo2EEMEi4UW8ccvSvfIf5tWpodFQooei 3N8wILt3Vekhnn/l+zLDI35LA+Oz7qC6GkXYotFMWKz07qDMqZ1LI+54W/BFo3qq7To3 1oaP/G+B7P9yWgjjJTzGhAzFl7ja8IuHdNextZDD1a1Ho/3TdXIo6LOp5Izf7dSEtxM8 Q05g== X-Gm-Message-State: AOAM530rHeVMHM6lFZrTpsnJA+2dV2Yj3d2R7CcLb+2kTB2S+rG+EZHz ipjb+60+KObV7qYTgDLpeELFLNfUyjsM0Q== X-Google-Smtp-Source: ABdhPJxXR1A9W4C6/S6Hz/x3l6aI8ffaubvOAoJ5wmeRJEXLf3d4Bf6BSmyrwHZtL/wxyFJ1F8UBMQ== X-Received: by 2002:a05:6830:140e:: with SMTP id v14mr4452029otp.155.1616505884017; Tue, 23 Mar 2021 06:24:44 -0700 (PDT) Received: from shawns-mbp-9155.attlocal.net ([2600:1700:eca0:8550:f44f:9fee:adc5:57a6]) by smtp.gmail.com with ESMTPSA id v23sm4265956ots.63.2021.03.23.06.24.42 for (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Tue, 23 Mar 2021 06:24:43 -0700 (PDT) From: Shawn McKinney Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Mime-Version: 1.0 (Mac OS X Mail 11.5 \(3445.9.5\)) Subject: Re: LdapConnectionPool.getConnection doing extraneous search? Date: Tue, 23 Mar 2021 08:24:41 -0500 References: <1CDE07B8-5AAA-48DA-B364-D87F9678179C@apache.org> <6c683582-3f9d-2bb8-21ae-2723f08a3de4@gmail.com> <811583D9-F9A8-4F28-90DF-CCBBDC380861@apache.org> <41EE2818-F802-415D-B740-0D8EE699959C@apache.org> <67ade323-6b58-ac82-d6dc-fd6e37fa8de0@gmail.com> <7417C637-DFB5-4DE0-8BB8-DD3948466046@apache.org> To: api@directory.apache.org In-Reply-To: Message-Id: <516880BA-B126-4098-B1CF-E23C3E690515@apache.org> X-Mailer: Apple Mail (2.3445.9.5) > On Mar 23, 2021, at 3:00 AM, Emmanuel L=C3=A9charny = wrote: >=20 >=20 > On 22/03/2021 19:41, Shawn McKinney wrote: >>> On Mar 22, 2021, at 11:05 AM, Emmanuel L=C3=A9charny = wrote: >>>=20 >>> LDAP connections are quite stable. Again, this check is there to = respect the commons-pool API contract. If the connection is dead, then = doing this check will let the pool fetching another connection, which is = good. OTOH, if you don't want to check connections while fetching one, = then you are on your own (ie, deal with the consequences of a bad = connection). >> Again, I must disagree. Connections aren=E2=80=99t stable, subjected = to env conditions and we can never assume a connection is good.=20 >=20 > You *can* assume it's good, and react exceptionally if it's not. That = the fastest way to deal with potential bad connections, if you want to = avoid a check every time you pull a connection from the pool. (but see = later for another option) There are 150 locations in fortress core where an ldap connection is = being pulled from the pool. No way I want to revisit all of that code.=20= >=20 > Something in the API chain must do the job of testing and reconnect, = every time it=E2=80=99s pulled from the pool. >=20 > This is exactly what the testOnBorrow does ;-) But it's costly... (see = later for another option) >=20 >> Now, having said that, that=E2=80=99s exactly what I=E2=80=99m = observing currently, with the test on borrow flag turned off. >> Let me explain the scenario: >> 1. Start server >> 2. Start client >> This initializes a connection pool which creates and stores exactly 1 = connection (min 1, max1 ) >> 3. Set breakpoint in client on pool.getConnection method >> 4. Restart the server. >> 5. Client performs ldap op which triggers the breakpoint on = getConnection >> 6. Server at this point still hasn=E2=80=99t any connections with the = client. The client =E2=80=98thinks=E2=80=99 it has connections in the = pool, but these were broken on step 4. >> 7. Step over the getConnection which then triggers the commons pool = to execute: >> ``` = GenericObjectPool._factory.activateObject(latch.getPair().value) >> ``` >> 8. A connection is made with the server, along with bind >> ``` >> 6058e163 conn=3D1000 fd=3D14 ACCEPT from IP=3D127.0.0.1:35246 = (IP=3D127.0.0.1:389) >> [exec] 6058e163 conn=3D1000 op=3D0 BIND = dn=3D"cn=3Dmanager,dc=3Dexample,dc=3Dcom" method=3D128 >> [exec] 6058e163 conn=3D1000 op=3D0 BIND = dn=3D"cn=3Dmanager,dc=3Dexample,dc=3Dcom" mech=3DSIMPLE ssf=3D0 >> [exec] 6058e163 conn=3D1000 op=3D0 RESULT tag=3D97 err=3D0 = duration=3D1.667ms >> ``` >> 9. Client continues with its ldap op successfully and is never the = wiser that the connections were all forcibly closed on server restart. >> This is EXACTLY what I want to see all of which is done without the = test on borrow eliminating the extraneous search on every connection = retrieval. >=20 > So that would mean we have some kind of 'retry' on connection = operation: if it fails, then let's assume the connection is broken, and = redo it with a fresh connection. >=20 Yes, that=E2=80=99s what=E2=80=99s happening. =20 In the Commons pool lib, this is the block that gets executed: ``` this._factory.activateObject(latch.getPair().value); if (this._testOnBorrow && = !this._factory.validateObject(latch.getPair().value)) { throw new Exception("ValidateObject failed"); } ``` In the first line above, activateObject, this code gets called, from our = AbstractPoolableLdapConnectionFactory class: ``` public void activateObject(LdapConnection connection) throws = LdapException { LOG.debug("Activating {}", connection); if (!connection.isConnected() || !connection.isAuthenticated()) { LOG.debug("rebind due to connection dropped on {}", connection); this.connectionFactory.bindConnection(connection); } ``` The code performs a rebind which renews the connection. All of this with testOnBorrow being false!=20 So, I=E2=80=99m still scratching my head figuring why we need this = secondary level that is wasting a round trip to the server. > The problem is that the pool is passive: it does not react to any = connection event, like a connection closure. OTOH, when a connection is = properly closed by the server (ie an NoticeOfDisconnect - aka NoD - is = generated by the server), the connection should process it and die. Now = we have an issue: the connection is just lying in the pool, not being = used by any client, so there is a missing step: removing the connection = from the pool in this very case. That can be something we can add to the = current LDAP pool. >=20 > Note that if the server does not send a NoD, you are screwed. There is = no way to be sure that the connection is ok until you use it. OTOH, = leveraging the setTestWhileIdle() could be a solution to partially = workaround the issue. Here you=E2=80=99ve lost me. I=E2=80=99m running the server in debug = mode, inside a bash shell, running in the foreground. In my test I stop the server by =E2=80=98ctrl-c=E2=80=99 killing the = shell. The server is not reacting to this and sending a NoD. =E2=80=94 Shawn --------------------------------------------------------------------- To unsubscribe, e-mail: api-unsubscribe@directory.apache.org For additional commands, e-mail: api-help@directory.apache.org