From dev-return-92895-archive-asf-public=cust-asf.ponee.io@kafka.apache.org Mon Apr 2 17:02:10 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 A3946180627 for ; Mon, 2 Apr 2018 17:02:09 +0200 (CEST) Received: (qmail 42882 invoked by uid 500); 2 Apr 2018 15:02:08 -0000 Mailing-List: contact dev-help@kafka.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@kafka.apache.org Delivered-To: mailing list dev@kafka.apache.org Received: (qmail 42870 invoked by uid 99); 2 Apr 2018 15:02:07 -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; Mon, 02 Apr 2018 15:02:07 +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 18314C63D3 for ; Mon, 2 Apr 2018 15:02:07 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamd1-us-west.apache.org X-Spam-Flag: NO X-Spam-Score: 2.649 X-Spam-Level: ** X-Spam-Status: No, score=2.649 tagged_above=-999 required=6.31 tests=[DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, HTML_MESSAGE=2, KAM_INFOUSMEBIZ=0.75, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H2=-0.001] autolearn=disabled Authentication-Results: spamd1-us-west.apache.org (amavisd-new); dkim=pass (1024-bit key) header.d=frmr.me Received: from mx1-lw-eu.apache.org ([10.40.0.8]) by localhost (spamd1-us-west.apache.org [10.40.0.7]) (amavisd-new, port 10024) with ESMTP id sU0HFo9uP86q for ; Mon, 2 Apr 2018 15:02:05 +0000 (UTC) Received: from mail-yb0-f171.google.com (mail-yb0-f171.google.com [209.85.213.171]) by mx1-lw-eu.apache.org (ASF Mail Server at mx1-lw-eu.apache.org) with ESMTPS id 1C0125F522 for ; Mon, 2 Apr 2018 15:02:05 +0000 (UTC) Received: by mail-yb0-f171.google.com with SMTP id m185-v6so5097671ybm.11 for ; Mon, 02 Apr 2018 08:02:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=frmr.me; s=frmrme; h=mime-version:in-reply-to:references:from:date:message-id:subject:to; bh=xhWJC6RDd3UvzFuKye0DCoc52W0s239MayzGHEas3aE=; b=R2h1XAbe3VAkyrIRFjRb5hJPqIm8IvsPQtQmtNwqIQWAhkQKFJiXwGatuNJjJt1K+S /w2DLNs8J4wh3FNnRFwSMPathSM+3AmorwWu1MqkzYW3j1FcV+/LatorZtrjs9aOs2RV dnGZUrOoI1QemfyCdeYH1ZAd7NYvUcw3RSIvY= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to; bh=xhWJC6RDd3UvzFuKye0DCoc52W0s239MayzGHEas3aE=; b=nxgxUpGJ3SzuP37MZb+kOlmLvLZ6mOOytA+xsMUbSy2dAfL86yTvAAB9syI/QITh3U if0rmyRzIb0NB6xH3wILtvkkzoNIPTL0jzPcNfs+QCyRYQbXRoIOPIXdmqbprrkSW/fm KqAj0dDv5fmfT8tkn6plwKXZKlaP6MMt68mrkZsaNqW/WBpzYC3BO7IBFLGYTR7/SLxu n47Xdo2e0HolCNXomqThMhEoFnlRrLXtbsrG8YLYKzDaIZYLJYOW9fLa3ZPkCzeDcpKh 8YEIUGciorF6sSyStrPx/4dxMXHNgQv5MHc0F1DKMBVj7WaVvtEsfGU4YPcRvI4yfn3V 9oCg== X-Gm-Message-State: ALQs6tDMoS03kj3G59Zcm9XxWzmx0OisBxKz/Ytwc/P29WUvIMLRG189 EJwIciXfQLObwITavDB1yTA0HKqRbOPPnz4HMp45VNUS X-Google-Smtp-Source: AIpwx4/MJQ/ehVngjUbLn0MdJDF2R7H6Z2lNVPOBKylTviICaZ8UiybiN6kex3/DAEJx056AzdBoYhjbnndo6A2fI7w= X-Received: by 2002:a25:c1c7:: with SMTP id r190-v6mr5398989ybf.404.1522681320676; Mon, 02 Apr 2018 08:02:00 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a25:50cb:0:0:0:0:0 with HTTP; Mon, 2 Apr 2018 08:02:00 -0700 (PDT) In-Reply-To: References: From: Matt Farmer Date: Mon, 2 Apr 2018 11:02:00 -0400 Message-ID: Subject: Re: [DISCUSS] KIP-275 - Indicate "isClosing" in the SinkTaskContext To: dev@kafka.apache.org Content-Type: multipart/alternative; boundary="00000000000039b6ce0568dedfab" --00000000000039b6ce0568dedfab Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Ugh * I can update I need more caffeine... On Mon, Apr 2, 2018 at 11:01 AM, Matt Farmer wrote: > I'm can update the rejected alternatives section as you describe. > > Also, adding a paragraph to the preCommit javadoc also seems like a > Very Very Good Idea=E2=84=A2 so I'll make that update to the KIP as well. > > On Mon, Apr 2, 2018 at 10:48 AM, Randall Hauch wrote: > >> Thanks for the KIP proposal, Matt. >> >> You mention in the "Rejected Alternatives" section that you considered >> changing the signature of the `preCommit` method but rejected it because >> it >> would break backward compatibility. Strictly speaking, it is possible to >> do >> this without breaking compatibility by introducing a new `preCommit` >> method, deprecating the old one, and having the new implementation call >> the >> old one. Such an approach would be complicated, and I'm not sure it adds >> any value. In fact, one of the benefits of having a context object is th= at >> we can add methods like the one you're proposing without causing any >> compatibility issues. Anyway, it probably is worth updating this rejecte= d >> alternative to be a bit more precise. >> >> Otherwise, I think this is a good approach, though I'd request that we >> update the `preCommit` JavaDoc to add a paragraph that explains this >> scenario. Thoughts? >> >> Randall >> >> On Wed, Mar 28, 2018 at 9:29 PM, Ted Yu wrote: >> >> > I looked at WorkerSinkTask and it seems using a boolean for KIP-275 >> should >> > suffice for now. >> > >> > Thanks >> > >> > On Wed, Mar 28, 2018 at 7:20 PM, Matt Farmer wrote: >> > >> > > Hey Ted, >> > > >> > > I have not, actually! >> > > >> > > Do you think that we're likely to add multiple states here soon? >> > > >> > > My instinct is to keep it simple until there are multiple states tha= t >> we >> > > would want >> > > to consider. I really like the simplicity of just getting a boolean >> and >> > the >> > > implementation of WorkerSinkTask already passes around a boolean to >> > > indicate this is happening internally. We're really just shuttling >> that >> > > value into >> > > the context at the correct moments. >> > > >> > > Once we have multiple states, we could choose to provide a more >> > > appropriately >> > > named method (e.g. getState?) and reimplement isClosing by checking >> that >> > > enum >> > > without breaking compatibility. >> > > >> > > However, if we think multiple states here are imminent for some >> reason, I >> > > would >> > > be pretty easy to convince adding that would be worth the extra >> > complexity! >> > > :) >> > > >> > > Matt >> > > >> > > =E2=80=94 >> > > Matt Farmer | Blog | Twitter >> > > >> > > GPG: CD57 2E26 F60C 0A61 E6D8 FC72 4493 8917 D667 4D07 >> > > >> > > On Wed, Mar 28, 2018 at 10:02 PM, Ted Yu wrote= : >> > > >> > > > The enhancement gives SinkTaskContext state information. >> > > > >> > > > Have you thought of exposing the state retrieval as an enum >> (initially >> > > with >> > > > two values) ? >> > > > >> > > > Thanks >> > > > >> > > > On Wed, Mar 28, 2018 at 6:55 PM, Matt Farmer wrote: >> > > > >> > > > > Hello all, >> > > > > >> > > > > I am proposing KIP-275 to improve Connect's SinkTaskContext so >> that >> > > Sinks >> > > > > can be informed >> > > > > in their preCommit hook if the hook is being invoked as a part o= f >> a >> > > > > rebalance or Connect >> > > > > shutdown. >> > > > > >> > > > > The KIP is here: >> > > > > https://cwiki.apache.org/confluence/pages/viewpage. >> > > > action?pageId=3D75977607 >> > > > > >> > > > > Please let me know what feedback y'all have. Thanks! >> > > > > >> > > > >> > > >> > >> > > --00000000000039b6ce0568dedfab--