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 <[email protected]> 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 <[email protected]> wrote on 09/14/2014 09:08:28 >>>>> AM: >>>>> >>>>> From: Konrad Grochowski <[email protected]> >>>>>> To: [email protected], >>>>>> 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 <[email protected]> >>>>>> Data:14.09.2014 14:41 (GMT+01:00) >>>>>> Do: [email protected] >>>>>> 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 < >>>>>> [email protected]> 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" <[email protected]> >>>>>>>> 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 >>>>>>>>> >>>>>>>> <[email protected]> >>>>> >>>>>> 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(); >>>>>>>>>> } >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>> >>> >> >
