From dev-return-40809-archive-asf-public=cust-asf.ponee.io@ignite.apache.org Mon Oct 22 15:08:28 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 E2DFA18064A for ; Mon, 22 Oct 2018 15:08:27 +0200 (CEST) Received: (qmail 12331 invoked by uid 500); 22 Oct 2018 13:08:27 -0000 Mailing-List: contact dev-help@ignite.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@ignite.apache.org Delivered-To: mailing list dev@ignite.apache.org Received: (qmail 12314 invoked by uid 99); 22 Oct 2018 13:08:26 -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, 22 Oct 2018 13:08:26 +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 DF503C012B for ; Mon, 22 Oct 2018 13:08:25 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamd4-us-west.apache.org X-Spam-Flag: NO X-Spam-Score: 2.889 X-Spam-Level: ** X-Spam-Status: No, score=2.889 tagged_above=-999 required=6.31 tests=[DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, FREEMAIL_REPLY=1, HTML_MESSAGE=2, RCVD_IN_DNSWL_NONE=-0.0001, SPF_PASS=-0.001, T_DKIMWL_WL_MED=-0.01] autolearn=disabled Authentication-Results: spamd4-us-west.apache.org (amavisd-new); dkim=pass (2048-bit key) header.d=gmail.com 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 CmnJiWl7u5hK for ; Mon, 22 Oct 2018 13:08:23 +0000 (UTC) Received: from mail-it1-f181.google.com (mail-it1-f181.google.com [209.85.166.181]) by mx1-lw-us.apache.org (ASF Mail Server at mx1-lw-us.apache.org) with ESMTPS id 5F1E85F1D3 for ; Mon, 22 Oct 2018 13:08:23 +0000 (UTC) Received: by mail-it1-f181.google.com with SMTP id c85-v6so12352536itd.1 for ; Mon, 22 Oct 2018 06:08:23 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to; bh=yphALUBA3I5/fJrRFohWsu0nSL+FK+Pyv+9I6MR3hRw=; b=aIFpq7TTPRfhyfCPnRIPYh8k9id7vFdfIEtukEu3dv+taYtcgh/lAVtNA6ewsuA1PH OZS7mJRSWkzoTytJeF3luUYjnrH1Bjz6xthwVJ129rBpQNpatwn4rQUl8EPvCVt7BghV 7AYaYba/smW/LNqstsahsYDkVME8gj1olm2qHQGSxgnxJKA4qT+OmnEGYpGx9G1dQEMS 2IQMw3ikpVa5EhjSVkxVDuYZRuJ6O2HjEj1iUXKOZKWqD9zrSyghXRwPLB08psWI3doX 84Fwbt97KG4XqV8m7cHILwHztQDNGTpJqtlyiPFEIxPe3BqbR/Y/DTSCyMhKp8Ci4Ndm AzSA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to; bh=yphALUBA3I5/fJrRFohWsu0nSL+FK+Pyv+9I6MR3hRw=; b=QDE1oV2YvkmWVMuGu0AW27xLf/LIkygda80F1MIbBm2NUJoE5wJvMpNJsJd+2pzTXm s/IZOq/KTmSPRI+ofUMw05G2Sabp8GKv/pDUMwzHft3t9ELNwQTw/uAEuobE0elAd/WZ t5+AgrMi2d3pnMOvzA7vsoWfIQXR5R6EHhDrBZlIuuWOqjI44FMZADefdogArWQvhEIJ 9C3p8qdrrdtTNflVxFNU29ef+Nz9jk8HhVkM+X8mF4crLkEBX0trXYJdfd1QrL5fz4hf WdNf+q1QsZiAriGgIDo2ZIsDDcMC07smBP9pfsk6nASppbWhNPPN6sUMUTq2P3bzyBXg 651A== X-Gm-Message-State: ABuFfojZyH8PdGcJBY1/4i2Zsf/YDf7Z6rpMjdFm4qRLeOrtp41OJH7U iS4jvpJCXINeRltMliuuUW3cqX9p9Zb6cPOAug2UqO2V X-Google-Smtp-Source: ACcGV60lXhBFX2oeRh0uGqro5nfzwFEI559cICQ3JqdwOdPgKWGcRPXt1U0VMtKxCvQ6Arqk/tuOzYkxxuxNgKMluUg= X-Received: by 2002:a24:61cc:: with SMTP id s195-v6mr9106128itc.140.1540213702359; Mon, 22 Oct 2018 06:08:22 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Andrey Kuznetsov Date: Mon, 22 Oct 2018 16:08:10 +0300 Message-ID: Subject: Re: Switching to real FailureHandler in tests To: dev Content-Type: multipart/alternative; boundary="0000000000009b7f940578d0f2cb" --0000000000009b7f940578d0f2cb Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi, Dmitrii! As for [1], I think your suggestion is good to complete the change. As for [2], I tend to disagree: in future tests default no-op handler can hide bugs of new Ignite functionality. If it is really needed, developer can explicitly mention that critical failure is a normal part of test operation. [1] https://issues.apache.org/jira/browse/IGNITE-8227 [2] https://issues.apache.org/jira/browse/IGNITE-9660 =D0=BF=D0=BD, 22 =D0=BE=D0=BA=D1=82. 2018 =D0=B3. =D0=B2 15:18, Dmitrii Rya= bov : > I tried to replace default no-op handler by handler stopping node and > failing the test. > > I've returned the no-op handler in many classes because critical > situations are expected behavior. But PR still have a lot of failed > tests and suites. In some tests, I can't understand a failure reason. > > I'm not finished to check failures, but after several RunAll runs, I > see new flaky tests (1 or 2 fails) appeared because of new handler. > > I think we should keep no-op handler as default, but add new handler > for a few classes, where critical situations aren't expected. > > =D0=BF=D1=82, 21 =D1=81=D0=B5=D0=BD=D1=82. 2018 =D0=B3. =D0=B2 17:03, And= rey Kuznetsov : > > > > Thanks to all for participating the discussion. > > > > I've updated [1]: now it requires new handler from [2] for completion. > > > > [1] https://issues.apache.org/jira/browse/IGNITE-9660 > > [2] https://issues.apache.org/jira/browse/IGNITE-8227 > > > > =D1=87=D1=82, 20 =D1=81=D0=B5=D0=BD=D1=82. 2018 =D0=B3. =D0=B2 21:56, V= ladimir Ozerov : > > > > > Stop node handler is not very good choice. Some test will continue > work as > > > usual even if some node failed. E.g. SQL queries with backups may > continue > > > function in some cases, especially if these are test with REPLICATED > cache. > > > > > > New test-scope handler looks like a better candidate to me. > > > > > > =D1=87=D1=82, 20 =D1=81=D0=B5=D0=BD=D1=82. 2018 =D0=B3. =D0=B2 21:22,= Andrey Kuznetsov : > > > > > > > I meant the first comment in [1]. We are to decide first whether > we'll do > > > > it or not. > > > > > > > > [1] https://issues.apache.org/jira/browse/IGNITE-8227 > > > > < > > > > > > > > https://issues.apache.org/jira/browse/IGNITE-8227?focusedCommentId=3D1643= 5298&page=3Dcom.atlassian.jira.plugin.system.issuetabpanels:comment-tabpane= l#comment-16435298 > > > > > > > > > > > > > =D1=87=D1=82, 20 =D1=81=D0=B5=D0=BD=D1=82. 2018 =D0=B3. =D0=B2 21:1= 8, Dmitriy Pavlov >: > > > > > > > > > Sorry, incomplete message. > > > > > > > > > > Why do you think there is no consensus? > > > > > > > > > > I have no clue what can be a reason for another approach. > > > > > By default failure handler should fail all test. > > > > > > > > > > Failure handlers test will be always a minority of tests, so fail > > > handler > > > > > call is something abnormal. > > > > > > > > > > =D1=87=D1=82, 20 =D1=81=D0=B5=D0=BD=D1=82. 2018 =D0=B3. =D0=B2 21= :15, Dmitriy Pavlov < > dpavlov.spb@gmail.com>: > > > > > > > > > > > Why do you think there is no consensus? > > > > > > > > > > > > I have no clue that by default failure handler should fail all > test. > > > > > > > > > > > > =D1=87=D1=82, 20 =D1=81=D0=B5=D0=BD=D1=82. 2018 =D0=B3. =D0=B2 = 21:10, Andrey Kuznetsov < > stkuzma@gmail.com>: > > > > > > > > > > > >> I've created [1] to address this. > > > > > >> > > > > > >> Dmitriy, I like your idea of creating special test-scope > handler. > > > But > > > > > >> there > > > > > >> is no consensus about it, so I don't want to rely on that > potential > > > > > >> handler > > > > > >> right now. We can switch to it later, of course. > > > > > >> > > > > > >> [1] https://issues.apache.org/jira/browse/IGNITE-9660 > > > > > >> > > > > > >> =D1=87=D1=82, 20 =D1=81=D0=B5=D0=BD=D1=82. 2018 =D0=B3. =D0=B2= 20:03, Maxim Muzafarov < > maxmuzaf@gmail.com>: > > > > > >> > > > > > >> > Andrey, > > > > > >> > > > > > > >> > I like your idea. > > > > > >> > > > > > > >> > After changing the default node failure handler to the new > one we > > > > > should > > > > > >> > carefully review the whole new test failures. For instance, > > > calling > > > > > this > > > > > >> > method in tests should not lead test to the node being > stopped: > > > > > >> > > > > > > >> > FOR TEST ONLY!!! > > > > > >> > TcpDiscoverySpi#simulateNodeFailure > > > > > >> > > > > > > >> > BTW, I would like to remove this method at all from producti= on > > > code. > > > > > >> > > > > > > >> > On Thu, 20 Sep 2018 at 19:43 Dmitriy Pavlov < > > > dpavlov.spb@gmail.com> > > > > > >> wrote: > > > > > >> > > > > > > >> > > But the totally ideal situation would be finding a way to > fail > > > the > > > > > >> test > > > > > >> > by > > > > > >> > > default, not only stopping a node. > > > > > >> > > > > > > > >> > > Some time ago I've created > > > > > >> > > https://issues.apache.org/jira/browse/IGNITE-8227 to > > > > > >> > > find out a way to do so. > > > > > >> > > > > > > > >> > > =D1=87=D1=82, 20 =D1=81=D0=B5=D0=BD=D1=82. 2018 =D0=B3. = =D0=B2 19:40, Dmitriy Pavlov < > > > > dpavlov.spb@gmail.com > > > > > >: > > > > > >> > > > > > > > >> > > > ++1 > > > > > >> > > > > > > > > >> > > > =D1=87=D1=82, 20 =D1=81=D0=B5=D0=BD=D1=82. 2018 =D0=B3. = =D0=B2 19:39, Andrey Kuznetsov < > > > > stkuzma@gmail.com > > > > > >: > > > > > >> > > > > > > > > >> > > >> Igniters, > > > > > >> > > >> > > > > > >> > > >> While running tests I see a lot of ignored critical > failures > > > > > >> caused by > > > > > >> > > >> {{NoOpFailureHandler}} that we use by default. In some > tests, > > > > of > > > > > >> > cource, > > > > > >> > > >> critical failures are the part of normal workflow, but > in the > > > > > >> majority > > > > > >> > > of > > > > > >> > > >> tests they indicate bugs. By using > {{NoOpFailureHandler}} we > > > > just > > > > > >> hide > > > > > >> > > >> these bugs from ourselves. > > > > > >> > > >> > > > > > >> > > >> What do you think about changing default handler to > > > > > >> > > >> {{StopNodeFailureHandler}} at {{GridAbstractTest}} leve= l? > > > This > > > > > >> could > > > > > >> > be > > > > > >> > > >> overridden in subclasses. > > > > > >> > > >> > > > > > >> > > >> -- > > > > > >> > > >> Best regards, > > > > > >> > > >> Andrey Kuznetsov. > > > > > >> > > >> > > > > > >> > > > > > > > > >> > > > > > > > >> > -- > > > > > >> > -- > > > > > >> > Maxim Muzafarov > > > > > >> > > > > > > >> > > > > > >> > > > > > >> -- > > > > > >> Best regards, > > > > > >> Andrey Kuznetsov. > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > -- > > > > Best regards, > > > > Andrey Kuznetsov. > > > > > > > > > > > > > -- > > Best regards, > > Andrey Kuznetsov. > --=20 Best regards, Andrey Kuznetsov. --0000000000009b7f940578d0f2cb--