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 23149200D61 for ; Tue, 5 Dec 2017 03:39:29 +0100 (CET) Received: by cust-asf.ponee.io (Postfix) id 216D5160C05; Tue, 5 Dec 2017 02:39:29 +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 95C6C160BF9 for ; Tue, 5 Dec 2017 03:39:27 +0100 (CET) Received: (qmail 53307 invoked by uid 500); 5 Dec 2017 02:39:25 -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 53295 invoked by uid 99); 5 Dec 2017 02:39:25 -0000 Received: from pnap-us-west-generic-nat.apache.org (HELO spamd2-us-west.apache.org) (209.188.14.142) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 05 Dec 2017 02:39:25 +0000 Received: from localhost (localhost [127.0.0.1]) by spamd2-us-west.apache.org (ASF Mail Server at spamd2-us-west.apache.org) with ESMTP id 5B73B1A0218 for ; Tue, 5 Dec 2017 02:39:24 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamd2-us-west.apache.org X-Spam-Flag: NO X-Spam-Score: 3.131 X-Spam-Level: *** X-Spam-Status: No, score=3.131 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_H3=-0.01, RCVD_IN_MSPIKE_WL=-0.01, RCVD_IN_SORBS_SPAM=0.5, UNPARSEABLE_RELAY=0.001] autolearn=disabled Authentication-Results: spamd2-us-west.apache.org (amavisd-new); dkim=pass (1024-bit key) header.d=frmr.me Received: from mx1-lw-us.apache.org ([10.40.0.8]) by localhost (spamd2-us-west.apache.org [10.40.0.9]) (amavisd-new, port 10024) with ESMTP id ePIy8QTtLo4H for ; Tue, 5 Dec 2017 02:39:16 +0000 (UTC) Received: from mail-yb0-f180.google.com (mail-yb0-f180.google.com [209.85.213.180]) by mx1-lw-us.apache.org (ASF Mail Server at mx1-lw-us.apache.org) with ESMTPS id 9EA4B5F2FE for ; Tue, 5 Dec 2017 02:39:15 +0000 (UTC) Received: by mail-yb0-f180.google.com with SMTP id i15so7514845ybk.0 for ; Mon, 04 Dec 2017 18:39:15 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=frmr.me; s=frmrme; h=from:in-reply-to:references:mime-version:date:message-id:subject:to; bh=U7aUhcR/XaaTtaTamPCGLmTgcNaAxNlZ0WMo3kSk22k=; b=C4oTWqBbJZykTOCe35M3P2vtwlaahX+wFd4vbMfIf88Tgkr64lDe3B+WLSO41tFf3D PDjzynyeQIziUWzOVTj2wWJiNDicQiW2C2Zo9UanvhES2yw9fVwoGrElhdr8Zcmwq48y 2O2C/UYdWF6TKH1Js8hnVy+K7wIqRRU5lc7R0= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:in-reply-to:references:mime-version:date :message-id:subject:to; bh=U7aUhcR/XaaTtaTamPCGLmTgcNaAxNlZ0WMo3kSk22k=; b=Us+zB+nTMAhPEypf2KZLNIsGb0VUaUQoteiBImxY0P0nXqnu/p97GSSAz2gwqg5Ey3 6H25CyIRX0Xir7gTKXqIrOuMVo9+hTSLOiM1+A8WUgot9PIxpLY9RMYbNvV4Z2DlA8pg Q99BQ0Eh4fYcNZO88xnBoYtiBbAqYQl4qSo7DYtvfqfQL+V9AK3I5tnA4iQ/qefusGYA FNJTGk7wtJsga+f9F6hzAHu4lFr/hR2HkvvYSR73EHbRck8BQRSIboH1Xeae1lh/k5X6 fsv3J0uqcWDuED1k8QiRCTVXjO4CVfCT3k39ghcVhHwo9mGjRpbilUzMrasiYKx0lmRz yjBg== X-Gm-Message-State: AJaThX7F+5eSaXDhXzilXCQzHDWIpo7mYaIlzJSKDiDU3LHL8UpGeL3A NE4X8JZ3SwZdOU/c78fhPEG1zkLZwvGwy+jTpJRRNLdY X-Google-Smtp-Source: AGs4zMa3lcyEZ8I5zq81gVQvGGs+1/SEcpB6iGBw1toflI/hkFiJ6XHZpXUnx9JUd7dKpAtAccJdumW/AQYJIN946NM= X-Received: by 10.37.14.68 with SMTP id 65mr10386248ybo.310.1512441554642; Mon, 04 Dec 2017 18:39:14 -0800 (PST) Received: from 1058052472880 named unknown by gmailapi.google.com with HTTPREST; Mon, 4 Dec 2017 18:39:13 -0800 From: Matt Farmer In-Reply-To: <67749cf4-4afc-442e-f5d4-2fd71e569d65@confluent.io> References: <3c92bb5a-a8bc-5c48-46f5-677f1f35b68b@confluent.io> <67749cf4-4afc-442e-f5d4-2fd71e569d65@confluent.io> X-Mailer: Airmail (461) MIME-Version: 1.0 Date: Mon, 4 Dec 2017 18:39:13 -0800 Message-ID: Subject: Re: [DISCUSS] KIP-210: Provide for custom error handling when Kafka Streams fails to produce To: dev@kafka.apache.org Content-Type: multipart/alternative; boundary="001a113e80309bbd5d055f8ebd22" archived-at: Tue, 05 Dec 2017 02:39:29 -0000 --001a113e80309bbd5d055f8ebd22 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hey Matthias, thanks for getting back to me. That's fine. But if we add it to `test` package, we don't need to talk about it in the KIP. `test` is not public API. Yes, that makes sense. It was in the KIP originally because I was, at one point, planning on including it. We can remove it now that we=E2=80=99ve de= cided we won=E2=80=99t include it in the public API. Understood. That makes sense. We should explain this clearly in the KIP and maybe log all other following exceptions at DEBUG level? I thought it was clear in the KIP, but I can go back and double check my wording and revise it to try and make it clearer. I=E2=80=99ll take a look at doing more work on the KIP and the Pull Request tomorrow. Thanks again! On December 4, 2017 at 5:50:33 PM, Matthias J. Sax (matthias@confluent.io) wrote: Hey, About your questions: >>> Acknowledged, so is ProducerFencedException the only kind of exception I >>> need to change my behavior on? Or are there other types I need to check? Is >>> there a comprehensive list somewhere? I cannot think if any other atm. We should list all fatal exceptions for which we don't call the handler and explain why (exception is "global" and will affect all other records, too | ProducerFenced is self-healing). We started to collect and categorize exception here (not completed yet): https://cwiki.apache.org/confluence/display/KAFKA/Kafka+Streams+Architectur= e#KafkaStreamsArchitecture-TypesofExceptions: This list should be a good starting point though. > I include it in the test package because I have tests that assert that if > the record collector impl encounters an Exception and receives a CONTINUE > that it actually does CONTINUE. That's fine. But if we add it to `test` package, we don't need to talk about it in the KIP. `test` is not public API. > I didn't want to invoke the handler in places where the CONTINUE or FAIL > result would just be ignored. Presumably, after a FAIL has been returned, > subsequent exceptions are likely to be repeats or noise from my > understanding of the code paths. If this is incorrect we can revisit. Understood. That makes sense. We should explain this clearly in the KIP and maybe log all other following exceptions at DEBUG level? -Matthias On 12/1/17 11:43 AM, Matt Farmer wrote: > Bump! It's been three days here and I haven't seen any further feedback. > Eager to get this resolved, approved, and merged. =3D) > > On Tue, Nov 28, 2017 at 9:53 AM Matt Farmer wrote: > >> Hi there, sorry for the delay in responding. Last week had a holiday and >> several busy work days in it so I'm just now getting around to responding. >> >>> We would only exclude >>> exception Streams can handle itself (like ProducerFencedException) -- >>> thus, if the handler has code to react to this, it would not be bad, as >>> this code is just never called. >> [...] >>> Thus, I am still in favor of not calling the ProductionExceptionHandler >>> for fatal exception. >> >> Acknowledged, so is ProducerFencedException the only kind of exception I >> need to change my behavior on? Or are there other types I need to check? Is >> there a comprehensive list somewhere? >> >>> About the "always continue" case. Sounds good to me to remove it (not >>> sure why we need it in test package?) >> >> I include it in the test package because I have tests that assert that if >> the record collector impl encounters an Exception and receives a CONTINUE >> that it actually does CONTINUE. >> >>> What is there reasoning for invoking the handler only for the first >>> exception? >> >> I didn't want to invoke the handler in places where the CONTINUE or FAIL >> result would just be ignored. Presumably, after a FAIL has been returned, >> subsequent exceptions are likely to be repeats or noise from my >> understanding of the code paths. If this is incorrect we can revisit. >> >> Once I get the answers to these questions I can make another pass on the >> pull request! >> >> Matt >> >> On Mon, Nov 20, 2017 at 4:07 PM Matthias J. Sax >> wrote: >> >>> Thanks for following up! >>> >>> One thought about an older reply from you: >>> >>>>>>> I strongly disagree here. The purpose of this handler isn't *just* to >>>>>>> make a decision for streams. There may also be desirable side >>> effects that >>>>>>> users wish to cause when production exceptions occur. There may be >>>>>>> side-effects that they wish to cause when AuthenticationExceptions >>> occur, >>>>>>> as well. We should still give them the hooks to preform those side >>> effects. >>> >>> And your follow up: >>> >>>>> - I think I would rather invoke it for all exceptions that could >>> occur >>>>> from attempting to produce - even those exceptions were returning >>> CONTINUE >>>>> may not be a good idea (e.g. Authorization exception). Until there >>> is a >>>>> different type for exceptions that are totally fatal (for example a >>>>> FatalStreamsException or some sort), maintaining a list of >>> exceptions that >>>>> you can intercept with this handler and exceptions you cannot would >>> be >>>>> really error-prone and hard to keep correct. >>> >>> I understand what you are saying, however, consider that Streams needs >>> to die for a fatal exception. Thus, if you call the handler for a fatal >>> exception, we would need to ignore the return value and fail -- this >>> seems to be rather intuitive. Furthermore, users can register an >>> uncaught-exception-handler and side effects for fatal errors can be >>> triggered there. >>> >>> Btw: an AuthorizationException is fatal -- not sure what you mean by an >>> "totally fatal" exception -- there is no superlative to fatal from my >>> understanding. >>> >>> About maintaining a list of exceptions: I don't think this is too hard, >>> and users also don't need to worry about it IMHO. We would only exclude >>> exception Streams can handle itself (like ProducerFencedException) -- >>> thus, if the handler has code to react to this, it would not be bad, as >>> this code is just never called. In case that there is an exception >>> Streams could actually handle but we still call the handler (ie, bug), >>> there should be no harm either -- the handler needs to return either >>> CONTINUE or FAIL and we would obey. It could only happen, that Streams >>> dies---as request by the user(!)---even if Streams could actually handle >>> the error and move on. But this seems to be not a issue from my point of >>> view. >>> >>> Thus, I am still in favor of not calling the ProductionExceptionHandler >>> for fatal exception. >>> >>> >>> >>> About the "always continue" case. Sounds good to me to remove it (not >>> sure why we need it in test package?) and to rename the "failing" >>> handler to "Default" (even if "default" is less descriptive and I would >>> still prefer "Fail" in the name). >>> >>> >>> Last question: >>> >>>>> - Continue to *only* invoke it on the first exception that we >>>>> encounter (before sendException is set) >>> >>> >>> What is there reasoning for invoking the handler only for the first >>> exception? >>> >>> >>> >>> >>> -Matthias >>> >>> On 11/20/17 10:43 AM, Matt Farmer wrote: >>>> Alright, here are some updates I'm planning to make after thinking on >>> this >>>> for awhile: >>>> >>>> - Given that the "always continue" handler isn't something I'd >>> recommend >>>> for production use as is, I'm going to move it into the test >>> namespace and >>>> remove it from mention in the public API. >>>> - I'm going to rename the "AlwaysFailProductionExceptionHandler" to >>>> "DefaultProductionExceptionHandler" >>>> - Given that the API for the exception handler involves being >>> invoked by >>>> streams to make a decision about whether or not to continue =E2=80=94 = I >>> think that >>>> we should: >>>> - Continue to *only* invoke it on the first exception that we >>>> encounter (before sendException is set) >>>> - Stop invoking it for the self-healing fenced exceptions. >>>> - I think I would rather invoke it for all exceptions that could >>> occur >>>> from attempting to produce - even those exceptions were returning >>> CONTINUE >>>> may not be a good idea (e.g. Authorization exception). Until there >>> is a >>>> different type for exceptions that are totally fatal (for example a >>>> FatalStreamsException or some sort), maintaining a list of >>> exceptions that >>>> you can intercept with this handler and exceptions you cannot would >>> be >>>> really error-prone and hard to keep correct. >>>> - I'm happy to file a KIP for the creation of this new Exception >>> type >>>> if there is interest. >>>> >>>> @ Matthias =E2=80=94 What do you think about the above? >>>> >>>> On Tue, Nov 14, 2017 at 9:44 AM Matt Farmer wrote: >>>> >>>>> I responded before reading your code review and didn't see the bit >>> about >>>>> how ProducerFencedException is self-healing. This error handling logic >>> is >>>>> *quite* confusing to reason about... I think I'm going to sit down with >>>>> the code a bit more today, but I'm inclined to think that if the fenced >>>>> exceptions are, indeed, self healing that we still invoke the handler >>> but >>>>> ignore its result for those exceptions. >>>>> >>>>> On Tue, Nov 14, 2017 at 9:37 AM Matt Farmer wrote: >>>>> >>>>>> Hi there, >>>>>> >>>>>> Following up here... >>>>>> >>>>>>> One tiny comment: I would prefer to remove the "Always" from the >>>>>> handler implementation names -- it sounds "cleaner" to me without it. >>>>>> >>>>>> Let me think on this. I generally prefer expressiveness to clean-ness, >>>>>> and I think that comes out in the names I chose for things. The straw >>> man >>>>>> in my head is the person that tried to substitute in the >>> "AlwaysContinue" >>>>>> variant and the "Always" is a trigger to really consider whether or >>> not >>>>>> they *always* want to try to continue. >>>>>> >>>>>> To be truthful, after some thought, using the "AlwaysContinue" variant >>>>>> isn't something I'd recommend generally in a production system. >>> Ideally >>>>>> these handlers are targeted at handling a specific series of >>> exceptions >>>>>> that a user wants to ignore, and not ignoring all exceptions. More on >>> this >>>>>> in a minute. >>>>>> >>>>>>> For the first category, it seems to not make sense to call the >>> handle but >>>>>> Streams should always fail. If we follow this design, the KIP should >>> list >>>>>> all exceptions for which the handler is not called. >>>>>> >>>>>> I strongly disagree here. The purpose of this handler isn't *just* to >>>>>> make a decision for streams. There may also be desirable side effects >>> that >>>>>> users wish to cause when production exceptions occur. There may be >>>>>> side-effects that they wish to cause when AuthenticationExceptions >>> occur, >>>>>> as well. We should still give them the hooks to preform those side >>> effects. >>>>>> >>>>>> In light of the above, I'm thinking that the >>>>>> "AlwaysContinueProductionExceptionHandler" variant should probably be >>>>>> removed from the public API and moved into tests since that's where >>> it's >>>>>> most useful. The more I think on it, the more I feel that having that >>> in >>>>>> the public API is a landmine. If you agree, then, we can rename the >>>>>> "AlwaysFailProductionExceptionHandler" to >>>>>> "DefaultProductionExceptionHandler". >>>>>> >>>>>> Thoughts? >>>>>> >>>>>> On Fri, Nov 10, 2017 at 6:13 PM Matthias J. Sax < >>> matthias@confluent.io> >>>>>> wrote: >>>>>> >>>>>>> I just review the PR, and there is one thing we should discuss. >>>>>>> >>>>>>> There are different types of exceptions that could occur. Some apply >>> to >>>>>>> all records (like Authorization exception) while others are for >>> single >>>>>>> records only (like record too large). >>>>>>> >>>>>>> For the first category, it seems to not make sense to call the handle >>>>>>> but Streams should always fail. If we follow this design, the KIP >>> should >>>>>>> list all exceptions for which the handler is not called. >>>>>>> >>>>>>> WDYT? >>>>>>> >>>>>>> >>>>>>> -Matthias >>>>>>> >>>>>>> >>>>>>> On 11/10/17 2:56 PM, Matthias J. Sax wrote: >>>>>>>> Just catching up on this KIP. >>>>>>>> >>>>>>>> One tiny comment: I would prefer to remove the "Always" from the >>>>>>> handler >>>>>>>> implementation names -- it sounds "cleaner" to me without it. >>>>>>>> >>>>>>>> >>>>>>>> -Matthias >>>>>>>> >>>>>>>> On 11/5/17 12:57 PM, Matt Farmer wrote: >>>>>>>>> It is agreed, then. I've updated the pull request. I'm trying to >>> also >>>>>>>>> update the KIP accordingly, but cwiki is being slow and dropping >>>>>>>>> connections..... I'll try again a bit later but please consider the >>>>>>> KIP >>>>>>>>> updated for all intents and purposes. heh. >>>>>>>>> >>>>>>>>> On Sun, Nov 5, 2017 at 3:45 PM Guozhang Wang >>>>>>> wrote: >>>>>>>>> >>>>>>>>>> That makes sense. >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> Guozhang >>>>>>>>>> >>>>>>>>>> On Sun, Nov 5, 2017 at 12:33 PM, Matt Farmer >>> wrote: >>>>>>>>>> >>>>>>>>>>> Interesting. I'm not sure I agree. I've been bitten many times by >>>>>>>>>>> unintentionally shipping code that fails to properly implement >>>>>>> logging. I >>>>>>>>>>> always discover this at the exact *worst* moment, too. (Normally >>> at >>>>>>> 3 AM >>>>>>>>>>> during an on-call shift. Hah.) However, if others feel the same >>> way >>>>>>> I >>>>>>>>>> could >>>>>>>>>>> probably be convinced to remove it. >>>>>>>>>>> >>>>>>>>>>> We could also meet halfway and say that when a customized >>>>>>>>>>> ProductionExceptionHandler instructs Streams to CONTINUE, we log >>> at >>>>>>> DEBUG >>>>>>>>>>> level instead of WARN level. Would that alternative be appealing >>> to >>>>>>> you? >>>>>>>>>>> >>>>>>>>>>> On Sun, Nov 5, 2017 at 12:32 PM Guozhang Wang < >>> wangguoz@gmail.com> >>>>>>>>>> wrote: >>>>>>>>>>> >>>>>>>>>>>> Thanks for the updates. I made a pass over the wiki again and it >>>>>>> looks >>>>>>>>>>>> good. >>>>>>>>>>>> >>>>>>>>>>>> About whether record collector should still internally log the >>>>>>> error in >>>>>>>>>>>> addition to what the customized ProductionExceptionHandler >>> does. I >>>>>>>>>>>> personally would prefer only to log if the returned value is >>> FAIL >>>>>>> to >>>>>>>>>>>> indicate that this thread is going to shutdown and trigger the >>>>>>>>>> exception >>>>>>>>>>>> handler. >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> Guozhang >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> On Sun, Nov 5, 2017 at 6:09 AM, Matt Farmer >>> wrote: >>>>>>>>>>>> >>>>>>>>>>>>> Hello, a bit later than I'd anticipated, but I've updated this >>>>>>> KIP as >>>>>>>>>>>>> outlined above. The updated KIP is now ready for review again! >>>>>>>>>>>>> >>>>>>>>>>>>> On Sat, Nov 4, 2017 at 1:03 PM Matt Farmer >>> wrote: >>>>>>>>>>>>> >>>>>>>>>>>>>> Ah. I actually created both of those in the PR and forgot to >>>>>>>>>> mention >>>>>>>>>>>> them >>>>>>>>>>>>>> by name in the KIP! Thanks for pointing out the oversight. >>>>>>>>>>>>>> >>>>>>>>>>>>>> I=E2=80=99ll revise the KIP this afternoon accordingly. >>>>>>>>>>>>>> >>>>>>>>>>>>>> The logging is actually provided for in the record collector. >>>>>>>>>>> Whenever >>>>>>>>>>>> a >>>>>>>>>>>>>> handler continues it=E2=80=99ll log a warning to ensure that= it=E2=80=99s >>>>>>>>>>> *impossible* >>>>>>>>>>>> to >>>>>>>>>>>>>> write a handler that totally suppresses production exceptions >>>>>>> from >>>>>>>>>>> the >>>>>>>>>>>>> log. >>>>>>>>>>>>>> As such, the default continue handler just returns the >>> continue >>>>>>>>>>> value. >>>>>>>>>>>> I >>>>>>>>>>>>>> can add details about those semantics to the KIP as well. >>>>>>>>>>>>>> On Sat, Nov 4, 2017 at 12:46 PM Matthias J. Sax < >>>>>>>>>>> matthias@confluent.io >>>>>>>>>>>>> >>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>> >>>>>>>>>>>>>>> One more comment. >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> You mention a default implementation for the handler that >>>>>>> fails. I >>>>>>>>>>>>>>> think, this should be part of the public API and thus should >>>>>>> have >>>>>>>>>> a >>>>>>>>>>>>>>> proper defined named that is mentioned in the KIP. >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> We could also add a second implementation for the >>>>>>> log-and-move-on >>>>>>>>>>>>>>> strategy, as both are the two most common cases. This class >>>>>>> should >>>>>>>>>>>> also >>>>>>>>>>>>>>> be part of public API (so users can just set in the config) >>>>>>> with a >>>>>>>>>>>>>>> proper name. >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> Otherwise, I like the KIP a lot! Thanks. >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> -Matthias >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> On 11/1/17 12:23 AM, Matt Farmer wrote: >>>>>>>>>>>>>>>> Thanks for the heads up. Yes, I think my changes are >>> compatible >>>>>>>>>>> with >>>>>>>>>>>>>>> that >>>>>>>>>>>>>>>> PR, but there will be a merge conflict that happens whenever >>>>>>> one >>>>>>>>>>> of >>>>>>>>>>>>> the >>>>>>>>>>>>>>> PRs >>>>>>>>>>>>>>>> is merged. Happy to reconcile the changes in my PR if 4148 >>> goes >>>>>>>>>> in >>>>>>>>>>>>>>> first. :) >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> On Tue, Oct 31, 2017 at 6:44 PM Guozhang Wang < >>>>>>>>>> wangguoz@gmail.com >>>>>>>>>>>> >>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> That sounds reasonable, thanks Matt. >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> As for the implementation, please note that there is >>> another >>>>>>>>>>>> ongoing >>>>>>>>>>>>> PR >>>>>>>>>>>>>>>>> that may touch the same classes that you are working on: >>>>>>>>>>>>>>>>> https://github.com/apache/kafka/pull/4148 >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> So it may help if you can also take a look at that PR and >>> see >>>>>>>>>> if >>>>>>>>>>> it >>>>>>>>>>>>> is >>>>>>>>>>>>>>>>> compatible with your changes. >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> Guozhang >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> On Tue, Oct 31, 2017 at 10:59 AM, Matt Farmer < >>> matt@frmr.me> >>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> I've opened this pull request to implement the KIP as >>>>>>>>>> currently >>>>>>>>>>>>>>> written: >>>>>>>>>>>>>>>>>> https://github.com/apache/kafka/pull/4165. It still needs >>>>>>>>>> some >>>>>>>>>>>>> tests >>>>>>>>>>>>>>>>>> added, >>>>>>>>>>>>>>>>>> but largely represents the shape I was going for. >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> If there are more points that folks would like to discuss, >>>>>>>>>>> please >>>>>>>>>>>>> let >>>>>>>>>>>>>>> me >>>>>>>>>>>>>>>>>> know. If I don't hear anything by tomorrow afternoon I'll >>>>>>>>>>> probably >>>>>>>>>>>>>>> start >>>>>>>>>>>>>>>>> a >>>>>>>>>>>>>>>>>> [VOTE] thread. >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> Thanks, >>>>>>>>>>>>>>>>>> Matt >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> On Fri, Oct 27, 2017 at 7:33 PM Matt Farmer >>> >>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> I can=E2=80=99t think of a reason that would be problem= atic. >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> Most of the time I would write a handler like this, I >>> either >>>>>>>>>>> want >>>>>>>>>>>>> to >>>>>>>>>>>>>>>>>>> ignore the error or fail and bring everything down so >>> that I >>>>>>>>>>> can >>>>>>>>>>>>> spin >>>>>>>>>>>>>>>>> it >>>>>>>>>>>>>>>>>>> back up later and resume from earlier offsets. When we >>> start >>>>>>>>>> up >>>>>>>>>>>>> after >>>>>>>>>>>>>>>>>>> crashing we=E2=80=99ll eventually try to process the me= ssage we >>>>>>>>>> failed >>>>>>>>>>> to >>>>>>>>>>>>>>>>> produce >>>>>>>>>>>>>>>>>>> again. >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> I=E2=80=99m concerned that =E2=80=9Cputting in a queue = for later=E2=80=9D opens >>> you >>>>>>>>>> up >>>>>>>>>>> to >>>>>>>>>>>>>>>>> putting >>>>>>>>>>>>>>>>>>> messages into the destination topic in an unexpected >>> order. >>>>>>>>>>>> However >>>>>>>>>>>>>>> if >>>>>>>>>>>>>>>>>>> others feel differently, I=E2=80=99m happy to talk abou= t it. >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> On Fri, Oct 27, 2017 at 7:10 PM Guozhang Wang < >>>>>>>>>>>> wangguoz@gmail.com> >>>>>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>> Please correct me if I'm wrong, but my understanding is >>>>>>>>>> that >>>>>>>>>>>> the >>>>>>>>>>>>>>>>>> record >>>>>>>>>>>>>>>>>>>>> metadata is always null if an exception occurred while >>>>>>>>>> trying >>>>>>>>>>>> to >>>>>>>>>>>>>>>>>>>> produce. >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> That is right. Thanks. >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> I looked at the example code, and one thing I realized >>> that >>>>>>>>>>>> since >>>>>>>>>>>>> we >>>>>>>>>>>>>>>>> are >>>>>>>>>>>>>>>>>>>> not passing the context in the handle function, we may >>> not >>>>>>>>>> be >>>>>>>>>>>>>>>>> implement >>>>>>>>>>>>>>>>>>>> the >>>>>>>>>>>>>>>>>>>> logic to send the fail records into another queue for >>>>>>> future >>>>>>>>>>>>>>>>> processing. >>>>>>>>>>>>>>>>>>>> Would people think that would be a big issue? >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> Guozhang >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> On Thu, Oct 26, 2017 at 12:14 PM, Matt Farmer < >>>>>>> matt@frmr.me >>>>>>>>>>> >>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>> Hello all, >>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>> I've updated the KIP based on this conversation, and >>> made >>>>>>>>>> it >>>>>>>>>>> so >>>>>>>>>>>>>>> that >>>>>>>>>>>>>>>>>> its >>>>>>>>>>>>>>>>>>>>> interface, config setting, and parameters line up more >>>>>>>>>>> closely >>>>>>>>>>>>> with >>>>>>>>>>>>>>>>>> the >>>>>>>>>>>>>>>>>>>>> interface in KIP-161 (deserialization handler). >>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>> I believe there are a few specific questions I need to >>>>>>>>>> reply >>>>>>>>>>>>>>> to..... >>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>> The question I had about then handle parameters are >>>>>>> around >>>>>>>>>>> the >>>>>>>>>>>>>>>>>> record, >>>>>>>>>>>>>>>>>>>>>> should it be `ProducerRecord`, or be >>>>>>>>>>> generics >>>>>>>>>>>> of >>>>>>>>>>>>>>>>>>>>>> `ProducerRecord` or >>>>>>>>>>>> `ProducerRecord>>>>>>>>>>>>>>>>>>> extends >>>>>>>>>>>>>>>>>>>>>> Object, ? extends Object>`? >>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>> At this point in the code we're guaranteed that this >>> is a >>>>>>>>>>>>>>>>>>>>> ProducerRecord, so the generics would >>> just >>>>>>>>>>> make >>>>>>>>>>>>> it >>>>>>>>>>>>>>>>>>>> harder >>>>>>>>>>>>>>>>>>>>> to work with the key and value. >>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>> Also, should the handle function include the >>>>>>>>>>> `RecordMetadata` >>>>>>>>>>>> as >>>>>>>>>>>>>>>>>> well >>>>>>>>>>>>>>>>>>>> in >>>>>>>>>>>>>>>>>>>>>> case it is not null? >>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>> Please correct me if I'm wrong, but my understanding is >>>>>>>>>> that >>>>>>>>>>>> the >>>>>>>>>>>>>>>>>> record >>>>>>>>>>>>>>>>>>>>> metadata is always null if an exception occurred while >>>>>>>>>> trying >>>>>>>>>>>> to >>>>>>>>>>>>>>>>>>>> produce. >>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>> We may probably try to write down at least the >>> following >>>>>>>>>>>>> handling >>>>>>>>>>>>>>>>>>>> logic >>>>>>>>>>>>>>>>>>>>> and >>>>>>>>>>>>>>>>>>>>>> see if the given API is sufficient for it >>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>> I've added some examples to the KIP. Let me know what >>> you >>>>>>>>>>>> think. >>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>> Cheers, >>>>>>>>>>>>>>>>>>>>> Matt >>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>> On Mon, Oct 23, 2017 at 9:00 PM Matt Farmer < >>> matt@frmr.me >>>>>>>> >>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>> Thanks for this feedback. I=E2=80=99m at a conferenc= e right >>> now >>>>>>>>>> and >>>>>>>>>>> am >>>>>>>>>>>>>>>>>>>> planning >>>>>>>>>>>>>>>>>>>>> on >>>>>>>>>>>>>>>>>>>>>> updating the KIP again with details from this >>>>>>> conversation >>>>>>>>>>>> later >>>>>>>>>>>>>>>>>> this >>>>>>>>>>>>>>>>>>>>> week. >>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>> I=E2=80=99ll shoot you a more detailed response then= ! :) >>>>>>>>>>>>>>>>>>>>>> On Mon, Oct 23, 2017 at 8:16 PM Guozhang Wang < >>>>>>>>>>>>> wangguoz@gmail.com >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>> Thanks for the KIP Matt. >>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>> Regarding the handle interface of >>>>>>>>>>>>> ProductionExceptionHandlerResp >>>>>>>>>>>>>>>>>> onse, >>>>>>>>>>>>>>>>>>>>>>> could >>>>>>>>>>>>>>>>>>>>>>> you write it on the wiki also, along with the actual >>>>>>>>>> added >>>>>>>>>>>>> config >>>>>>>>>>>>>>>>>>>> names >>>>>>>>>>>>>>>>>>>>>>> (e.g. what >>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP- >>>>>>>>>>> 161%3A+streams+ >>>>>>>>>>>>>>>>>>>>> deserialization+exception+handlers >>>>>>>>>>>>>>>>>>>>>>> described). >>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>> The question I had about then handle parameters are >>>>>>>>>> around >>>>>>>>>>>> the >>>>>>>>>>>>>>>>>>>> record, >>>>>>>>>>>>>>>>>>>>>>> should it be `ProducerRecord`, or be >>>>>>>>>>> generics >>>>>>>>>>>>> of >>>>>>>>>>>>>>>>>>>>>>> `ProducerRecord` or >>>>>>>>>>>> `ProducerRecord>>>>>>>>>>>>>>>>>>> extends >>>>>>>>>>>>>>>>>>>>>>> Object, ? extends Object>`? >>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>> Also, should the handle function include the >>>>>>>>>>> `RecordMetadata` >>>>>>>>>>>>> as >>>>>>>>>>>>>>>>>>>> well in >>>>>>>>>>>>>>>>>>>>>>> case it is not null? >>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>> We may probably try to write down at least the >>> following >>>>>>>>>>>>> handling >>>>>>>>>>>>>>>>>>>> logic >>>>>>>>>>>>>>>>>>>>>>> and >>>>>>>>>>>>>>>>>>>>>>> see if the given API is sufficient for it: 1) throw >>>>>>>>>>> exception >>>>>>>>>>>>>>>>>>>>> immediately >>>>>>>>>>>>>>>>>>>>>>> to fail fast and stop the world, 2) log the error and >>>>>>>>>> drop >>>>>>>>>>>>> record >>>>>>>>>>>>>>>>>> and >>>>>>>>>>>>>>>>>>>>>>> proceed silently, 3) send such errors to a specific >>>>>>>>>> "error" >>>>>>>>>>>>> Kafka >>>>>>>>>>>>>>>>>>>> topic, >>>>>>>>>>>>>>>>>>>>>>> or >>>>>>>>>>>>>>>>>>>>>>> record it as an app-level metrics ( >>>>>>>>>>>>>>>>>>>>>>> >>> https://kafka.apache.org/documentation/#kafka_streams_ >>>>>>>>>>>>> monitoring >>>>>>>>>>>>>>>>> ) >>>>>>>>>>>>>>>>>>>> for >>>>>>>>>>>>>>>>>>>>>>> monitoring. >>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>> Guozhang >>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>> On Fri, Oct 20, 2017 at 5:47 PM, Matt Farmer < >>>>>>>>>> matt@frmr.me >>>>>>>>>>>> >>>>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> I did some more digging tonight. >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> @Ted: It looks like the deserialization handler uses >>>>>>>>>>>>>>>>>>>>>>>> "default.deserialization.exception.handler" for the >>>>>>>>>>> config >>>>>>>>>>>>>>>>>> name. No >>>>>>>>>>>>>>>>>>>>>>>> ".class" on the end. I'm inclined to think this >>> should >>>>>>>>>> use >>>>>>>>>>>>>>>>>>>>>>>> "default.production.exception.handler". >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> On Fri, Oct 20, 2017 at 8:22 PM Matt Farmer < >>>>>>>>>> matt@frmr.me >>>>>>>>>>>> >>>>>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> Okay, I've dug into this a little bit. >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> I think getting access to the serialized record is >>>>>>>>>>>> possible, >>>>>>>>>>>>>>>>>> and >>>>>>>>>>>>>>>>>>>>>>> changing >>>>>>>>>>>>>>>>>>>>>>>>> the naming and return type is certainly doable. >>>>>>>>>> However, >>>>>>>>>>>>>>>>>> because >>>>>>>>>>>>>>>>>>>>> we're >>>>>>>>>>>>>>>>>>>>>>>>> hooking into the onCompletion callback we have no >>>>>>>>>>> guarantee >>>>>>>>>>>>>>>>>> that >>>>>>>>>>>>>>>>>>>> the >>>>>>>>>>>>>>>>>>>>>>>>> ProcessorContext state hasn't changed by the time >>> this >>>>>>>>>>>>>>>>>> particular >>>>>>>>>>>>>>>>>>>>>>> handler >>>>>>>>>>>>>>>>>>>>>>>>> runs. So I think the signature would change to >>>>>>>>>> something >>>>>>>>>>>>>>>>> like: >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> ProductionExceptionHandlerResponse handle(final >>>>>>>>>>>>>>>>>>>> ProducerRecord<..> >>>>>>>>>>>>>>>>>>>>>>>> record, >>>>>>>>>>>>>>>>>>>>>>>>> final Exception exception) >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> Would this be acceptable? >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> On Thu, Oct 19, 2017 at 7:33 PM Matt Farmer < >>>>>>>>>>> matt@frmr.me> >>>>>>>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>> Ah good idea. Hmmm. I can line up the naming and >>>>>>>>>> return >>>>>>>>>>>> type >>>>>>>>>>>>>>>>>> but >>>>>>>>>>>>>>>>>>>>> I=E2=80=99m >>>>>>>>>>>>>>>>>>>>>>> not >>>>>>>>>>>>>>>>>>>>>>>>>> sure if I can get my hands on the context and the >>>>>>>>>> record >>>>>>>>>>>>>>>>>> itself >>>>>>>>>>>>>>>>>>>>>>> without >>>>>>>>>>>>>>>>>>>>>>>>>> other changes. >>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>> Let me dig in and follow up here tomorrow. >>>>>>>>>>>>>>>>>>>>>>>>>> On Thu, Oct 19, 2017 at 7:14 PM Matthias J. Sax < >>>>>>>>>>>>>>>>>>>>>>> matthias@confluent.io> >>>>>>>>>>>>>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>> Thanks for the KIP. >>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>> Are you familiar with KIP-161? >>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP- >>>>>>>>>>> 161%3A+streams+ >>>>>>>>>>>>>>>>>>>>>>>> deserialization+exception+handlers >>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>> I thinks, we should align the design (parameter >>>>>>>>>> naming, >>>>>>>>>>>>>>>>>> return >>>>>>>>>>>>>>>>>>>>>>> types, >>>>>>>>>>>>>>>>>>>>>>>>>>> class names etc) of KIP-210 to KIP-161 to get a >>>>>>>>>> unified >>>>>>>>>>>>>>>>> user >>>>>>>>>>>>>>>>>>>>>>>> experience. >>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>> -Matthias >>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>> On 10/18/17 4:20 PM, Matt Farmer wrote: >>>>>>>>>>>>>>>>>>>>>>>>>>>> I=E2=80=99ll create the JIRA ticket. >>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>> I think that config name will work. I=E2=80=99= ll update >>> the >>>>>>>>>>> KIP >>>>>>>>>>>>>>>>>>>>>>> accordingly. >>>>>>>>>>>>>>>>>>>>>>>>>>>> On Wed, Oct 18, 2017 at 6:09 PM Ted Yu < >>>>>>>>>>>>>>>>>> yuzhihong@gmail.com> >>>>>>>>>>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>> Can you create JIRA that corresponds to the >>> KIP ? >>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>> For the new config, how about naming it >>>>>>>>>>>>>>>>>>>>>>>>>>>>> production.exception.processor.class >>>>>>>>>>>>>>>>>>>>>>>>>>>>> ? This way it is clear that class name should >>> be >>>>>>>>>>>>>>>>>> specified. >>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>> Cheers >>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>> On Wed, Oct 18, 2017 at 2:40 PM, Matt Farmer < >>>>>>>>>>>>>>>>>> matt@frmr.me> >>>>>>>>>>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Hello everyone, >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> This is the discussion thread for the KIP >>> that I >>>>>>>>>>> just >>>>>>>>>>>>>>>>>> filed >>>>>>>>>>>>>>>>>>>>>>> here: >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP- >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 210+-+Provide+for+custom+ >>>>>>>>>>> error+handling++when+Kafka+ >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Streams+fails+to+produce >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Looking forward to getting some feedback from >>>>>>>>>> folks >>>>>>>>>>>>>>>>> about >>>>>>>>>>>>>>>>>>>> this >>>>>>>>>>>>>>>>>>>>>>> idea >>>>>>>>>>>>>>>>>>>>>>>>>>> and >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> working toward a solution we can contribute >>> back. >>>>>>>>>> :) >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Cheers, >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Matt Farmer >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>> -- >>>>>>>>>>>>>>>>>>>>>>> -- Guozhang >>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> -- >>>>>>>>>>>>>>>>>>>> -- Guozhang >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> -- >>>>>>>>>>>>>>>>> -- Guozhang >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> -- >>>>>>>>>>>> -- Guozhang >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> -- >>>>>>>>>> -- Guozhang >>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>>>> >>>> >>> >>> > --001a113e80309bbd5d055f8ebd22--