From user-return-3941-apmail-thrift-user-archive=thrift.apache.org@thrift.apache.org Tue Sep 16 19:20:10 2014 Return-Path: X-Original-To: apmail-thrift-user-archive@www.apache.org Delivered-To: apmail-thrift-user-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 860A011BE2 for ; Tue, 16 Sep 2014 19:20:10 +0000 (UTC) Received: (qmail 5091 invoked by uid 500); 16 Sep 2014 19:20:10 -0000 Delivered-To: apmail-thrift-user-archive@thrift.apache.org Received: (qmail 5067 invoked by uid 500); 16 Sep 2014 19:20:10 -0000 Mailing-List: contact user-help@thrift.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: user@thrift.apache.org Delivered-To: mailing list user@thrift.apache.org Received: (qmail 5052 invoked by uid 99); 16 Sep 2014 19:20:09 -0000 Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 16 Sep 2014 19:20:09 +0000 X-ASF-Spam-Status: No, hits=-0.0 required=5.0 tests=RCVD_IN_DNSWL_NONE,SPF_PASS X-Spam-Check-By: apache.org Received-SPF: pass (nike.apache.org: domain of jensgeyer@hotmail.com designates 157.55.1.170 as permitted sender) Received: from [157.55.1.170] (HELO DUB004-OMC2S31.hotmail.com) (157.55.1.170) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 16 Sep 2014 19:19:43 +0000 Received: from DUB110-DS6 ([157.55.1.138]) by DUB004-OMC2S31.hotmail.com over TLS secured channel with Microsoft SMTPSVC(7.5.7601.22724); Tue, 16 Sep 2014 12:19:42 -0700 X-TMN: [hH5GqG7LnK3xWcG9MHFJ2HXUXdbRmYe8] X-Originating-Email: [jensgeyer@hotmail.com] Message-ID: From: "Jens Geyer" To: References: <54180A80.5040105@minions.org.pl> <54180E29.4090807@minions.org.pl> <54181754.5040601@minions.org.pl> <541819C5.1080102@minions.org.pl> <54181E47.5030706@minions.org.pl> <541831F3.7060205@minions.org.pl> <54187B72.60001@minions.org.pl> In-Reply-To: <54187B72.60001@minions.org.pl> Subject: Re: oneway Thrift version 0.9.1 oneway flag generates regular T_CALL as opposed to T_ONEWAY with c++ code Date: Tue, 16 Sep 2014 21:19:48 +0200 MIME-Version: 1.0 Content-Type: text/plain; format=flowed; charset="UTF-8"; reply-type=response Content-Transfer-Encoding: 8bit X-Priority: 3 X-MSMail-Priority: Normal Importance: Normal X-Mailer: Microsoft Windows Live Mail 16.4.3528.331 X-MimeOLE: Produced By Microsoft MimeOLE V16.4.3528.331 X-OriginalArrivalTime: 16 Sep 2014 19:19:42.0450 (UTC) FILETIME=[2AE58520:01CFD1E3] X-Virus-Checked: Checked by ClamAV on apache.org > Somebody started to need that :) It was merely kind of a joke, but with a background. If nobody notices absence of a feature etc. the question raises ... > oneways are a really nice feature of thrift, Absolutely. There are two contribs from me in the code base, both were extracted out of a MQ scenario (although it may be not so clear form the code), where I did use oneways quite heavily. > it would be nice to make them work properly. Also - if thrift already > defines "4 / oneway" as additional message type it should use it :) Sure. +1 -----Ursprüngliche Nachricht----- From: Konrad Grochowski Sent: Tuesday, September 16, 2014 8:03 PM To: user@thrift.apache.org Subject: Re: oneway Thrift version 0.9.1 oneway flag generates regular T_CALL as opposed to T_ONEWAY with c++ code Somebody started to need that :) oneway calls were just a "half" of normal call, when they should be treated differently - like in Steven's zeromq integration or Ben's pass-through server. oneways are a really nice feature of thrift, so it would be nice to make them work properly. Also - if thrift already defines "4 / oneway" as additional message type it should use it :) Konrad On 16.09.2014 19:49, Jens Geyer wrote: > > Seems as if nobody really needs it? > > > -----Ursprüngliche Nachricht----- From: Konrad Grochowski > Sent: Tuesday, September 16, 2014 2:49 PM > To: user@thrift.apache.org > Subject: Re: oneway Thrift version 0.9.1 oneway flag generates regular > T_CALL as opposed to T_ONEWAY with c++ code > > I've updated generators for other langs, unfortunately I had to fill new > issues: > > https://issues.apache.org/jira/browse/THRIFT-2706 > https://issues.apache.org/jira/browse/THRIFT-2707 > https://issues.apache.org/jira/browse/THRIFT-2708 > > seems that "oneway" message type was never fully supported. > > Konrad > > On 16.09.2014 13:25, Konrad Grochowski wrote: >> As I haven't change the way things are encoded, just fixed decoding >> procedure, no protocol version change is needed (imho). >> >> But still - old server with old decoding procedures may not support >> oneway methods sent by new clients in compact protocol. >> >> Konrad >> >> On 16.09.2014 13:20, Steven Varga wrote: >>> Hi Konrad, >>> >>> I can confirm both of your results; TDenseProtocol failed all my tests >>> as >>> well -- therefore I didn't take a closer look at it; will look at >>> TCompactProtocol correction as soon as I can. >>> >>> Also wondering that how you folks handle such changes: With bumping up >>> protocol version to avoid mismatch with other implementations or there >>> are >>> other less obvious solutions? >>> >>> These changes are great because allows filtering on message types; >>> ZeroMq >>> is very sensitive to such. >>> >>> >>> >>> On Tue, Sep 16, 2014 at 7:06 AM, Konrad Grochowski >>> >>> wrote: >>> >>>> Steven, >>>> >>>> Dense protocol fails tests as it does not support messages at all :) >>>> Compact protocol should be fixed now, awaiting CI results and merge. >>>> >>>> Konrad >>>> >>>> >>>> On 16.09.2014 12:56, Konrad Grochowski wrote: >>>> >>>>> Pushed fix for Compact protocol, moving to Dense :) >>>>> >>>>> Can somebody check have I missed any lang? >>>>> >>>>> Konrad >>>>> >>>>> On 16.09.2014 12:17, Konrad Grochowski wrote: >>>>> >>>>>> got it: >>>>>> >>>>>> is: >>>>>> messageType = (TMessageType)((versionAndType >> TYPE_SHIFT_AMOUNT) >>>>>> & >>>>>> 0x03); >>>>>> >>>>>> should be: >>>>>> messageType = (TMessageType)((versionAndType >> TYPE_SHIFT_AMOUNT) >>>>>> & >>>>>> 0x07); >>>>>> >>>>>> as TYPE_MASK = 0xE0 // 1110 0000 - 3bits, 0x03 - 2bits >>>>>> >>>>>> I'll commit it as soon as I check all other impl of compact protocol >>>>>> >>>>>> >>>>>> W dniu 2014-09-16 12:01, Konrad 'Hcorg' Grochowski pisze: >>>>>> >>>>>>> I was able to reproduce Steven's problem. Seems that T_ONEWAY >>>>>>> encodes >>>>>>> badly in Compact protocol: "Thrift: Tue Sep 16 11:54:30 2014 >>>>>>> received >>>>>>> invalid message type 0 from client" >>>>>>> >>>>>>> I'll debug all those shifts and masks etc, hopefully it's a >>>>>>> programming >>>>>>> mistake, not protocol issue (T_ONEWAY == 4 and there are 4 bits for >>>>>>> message >>>>>>> type, so everything should be ok...) >>>>>>> >>>>>>> Konrad >>>>>>> >>>>>>> W dniu 2014-09-15 15:07, Ben Craig pisze: >>>>>>> >>>>>>>> Looks like an oversight to me. The server side of the generated >>>>>>>> code >>>>>>>> in >>>>>>>> C++ respects T_ONEWAY and T_CALL. >>>>>>>> >>>>>>>> Coincidentally, someone at my company was likely going to run into >>>>>>>> this >>>>>>>> issue in about three weeks. They are doing some fancy pass-through >>>>>>>> server >>>>>>>> >>>>>>>> stuff, and as a 'server' without access to a specific .thrift file, >>>>>>>> they >>>>>>>> need to know whether to wait as a 'client' when passing an RPC. >>>>>>>> They >>>>>>>> were >>>>>>>> >>>>>>>> planning on looking at the message type to figure that out. >>>>>>>> >>>>>>>> I've looked at your commit, but have not tested it. +0.9. >>>>>>>> >>>>>>>> Konrad Grochowski wrote on 09/14/2014 >>>>>>>> 09:08:28 >>>>>>>> AM: >>>>>>>> >>>>>>>> From: Konrad Grochowski >>>>>>>>> To: user@thrift.apache.org, >>>>>>>>> Date: 09/14/2014 09:09 AM >>>>>>>>> Subject: ODP: Re: oneway Thrift version 0.9.1 oneway flag >>>>>>>>> generates >>>>>>>>> regular T_CALL as opposed to T_ONEWAY with c++ code >>>>>>>>> >>>>>>>>> I suspected that something will break - that's why I submited pull >>>>>>>>> req to see what will Travis say :) >>>>>>>>> >>>>>>>>> I'll look further into it, also I hope for some explaining >>>>>>>>> comments >>>>>>>>> to issue from devs ;) >>>>>>>>> >>>>>>>>> Konrad >>>>>>>>> >>>>>>>>> -------- Oryginalna wiadomość -------- >>>>>>>>> Od: Steven Varga >>>>>>>>> Data:14.09.2014 14:41 (GMT+01:00) >>>>>>>>> Do: user@thrift.apache.org >>>>>>>>> Temat: Re: oneway Thrift version 0.9.1 oneway flag generates >>>>>>>>> regular >>>>>>>>> T_CALL as opposed to T_ONEWAY with c++ code >>>>>>>>> >>>>>>>>> Hello Konrad, >>>>>>>>> >>>>>>>>> I looked at the patch but it only resolves the issue posted; and >>>>>>>>> may >>>>>>>>> >>>>>>>> lead >>>>>>>> >>>>>>>>> to deeper problems: >>>>>>>>> Whereas TBinaryProtocol is ok with T_ONEWAY flag, >>>>>>>>> TCompactProtocol >>>>>>>>> >>>>>>>> seems >>>>>>>> >>>>>>>>> to have a different way of handling things [it filters it out?], >>>>>>>>> and >>>>>>>>> TDenseProtocol also failed test but I didn't follow up with that >>>>>>>>> and >>>>>>>>> >>>>>>>> the >>>>>>>> >>>>>>>>> rest of the provided protocols. >>>>>>>>> >>>>>>>>> steve >>>>>>>>> >>>>>>>>> On Sun, Sep 14, 2014 at 7:19 AM, Konrad 'Hcorg' Grochowski < >>>>>>>>> hcorg@minions.org.pl> wrote: >>>>>>>>> >>>>>>>>> I've created issue >>>>>>>>> https://issues.apache.org/jira/browse/THRIFT-2704 >>>>>>>>> and >>>>>>>>> submited patch, maybe it'll get accepted :) >>>>>>>>>> W dniu 2014-09-12 18:25, Randy Abernethy pisze: >>>>>>>>>> >>>>>>>>>> +1 >>>>>>>>>> >>>>>>>>>>> On Sep 12, 2014 8:13 AM, "Nevo Hed" >>>>>>>>>>> wrote: >>>>>>>>>>> >>>>>>>>>>> Steven >>>>>>>>>>> >>>>>>>>>>>> I can confirm that for my oneway messages I observe the same >>>>>>>>>>>> T_CALL >>>>>>>>>>>> >>>>>>>>>>> (I >>>>>>>>> have >>>>>>>>>>>> not looked into why) >>>>>>>>>>>> >>>>>>>>>>>> The difference is that one way messages have fooClient::bar >>>>>>>>>>>> that >>>>>>>>>>>> >>>>>>>>>>> just >>>>>>>>> call >>>>>>>>>>>> send_bar and if it was not oneway it would call send_bar AND >>>>>>>>>>>> >>>>>>>>>>> recv_bar >>>>>>>>> Now realize that you do not have to use the RPC mechanism at all - >>>>>>>>>>> you >>>>>>>>> could just serialize objects into your messages, perhaps with a >>>>>>>>>>> custom >>>>>>>>> header (there may be something supported already). This is what >>>>>>>>> we >>>>>>>>>>> have >>>>>>>>> been doing since thrift0.6.x for SOME of our connections - for >>>>>>>>> that >>>>>>>>>>> case >>>>>>>>> we >>>>>>>>>>>> do not even define a service, just objects and an enumeration >>>>>>>>>>>> of >>>>>>>>>>>> >>>>>>>>>>> messages >>>>>>>>> On Fri, Sep 12, 2014 at 8:31 AM, Steven Varga >>>>>>>>>>> >>>>>>>>> wrote: >>>>>>>>>>>> Hi, >>>>>>>>>>>> >>>>>>>>>>>>> the following service generates oneway RPC call flagged with >>>>>>>>>>>>> T_CALL >>>>>>>>>>>>> >>>>>>>>>>>> flag >>>>>>>>> as >>>>>>>>>>>> opposed to expected T_ONEWAY; I need the T_ONEWAY flag to >>>>>>>>>>>>> implement >>>>>>>>>>>>> >>>>>>>>>>>>> proper >>>>>>>>>>>> zero MQ message passing. Zero MQ message passing system >>>>>>>>>>>> differentiates >>>>>>>>> between request - reply patterns and push - pull ones at socket >>>>>>>>>>>> level; >>>>>>>>> Am I doing something wrong ? >>>>>>>>>>>>> best, >>>>>>>>>>>>> steve >>>>>>>>>>>>> >>>>>>>>>>>>> service foo { >>>>>>>>>>>>> oneway void bar( 1:string value ); >>>>>>>>>>>>> } >>>>>>>>>>>>> >>>>>>>>>>>>> void fooClient::put( const std::string& value ) { >>>>>>>>>>>>> send_bar( value ); // <- this is correct that recv_xxx is >>>>>>>>>>>>> >>>>>>>>>>>> missing >>>>>>>>> } >>>>>>>>>>>>> // ----- incorrect or unreasoned T_CALL instead of T_ONEWAY >>>>>>>>>>>>> void fooClient::send_bar(const std::string& value) { >>>>>>>>>>>>> int32_t cseqid = 0; >>>>>>>>>>>>> oprot_->writeMessageBegin("bar", >>>>>>>>>>>>> >>>>>>>>>>>> ::apache::thrift::protocol::T_CALL, >>>>>>>>> cseqid); >>>>>>>>>>>>> .... >>>>>>>>>>>>> } >>>>>>>>>>>>> >>>>>>>>>>>>> ------ EXAMPLE ----- >>>>>>>>>>>>> service foo { >>>>>>>>>>>>> oneway void bar( 1:string value ); >>>>>>>>>>>>> } >>>>>>>>>>>>> >>>>>>>>>>>>> ------------- response excerpt ---------- >>>>>>>>>>>>> >>>>>>>>>>>>> void fooClient::bar(const std::string& value) >>>>>>>>>>>>> { >>>>>>>>>>>>> send_bar(value); >>>>>>>>>>>>> } >>>>>>>>>>>>> >>>>>>>>>>>>> void fooClient::send_bar(const std::string& value) >>>>>>>>>>>>> { >>>>>>>>>>>>> int32_t cseqid = 0; >>>>>>>>>>>>> oprot_->writeMessageBegin("bar", >>>>>>>>>>>>> >>>>>>>>>>>> ::apache::thrift::protocol::T_CALL, >>>>>>>>> cseqid); >>>>>>>>>>>>> foo_bar_pargs args; >>>>>>>>>>>>> args.value = &value; >>>>>>>>>>>>> args.write(oprot_); >>>>>>>>>>>>> >>>>>>>>>>>>> oprot_->writeMessageEnd(); >>>>>>>>>>>>> oprot_->getTransport()->writeEnd(); >>>>>>>>>>>>> oprot_->getTransport()->flush(); >>>>>>>>>>>>> } >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> >> >