thrift-user mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jens Geyer" <jensge...@hotmail.com>
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 19:32:01 GMT
> Can somebody check have I missed any lang?

* There is an suspicious hardcoded T_CALL in the cpp-generator, line 2759
* Same in the t_c_glib generator at line 1528
* thrift-protocol-spec.md lacks T_ONEWAY in line 46
* PeekProcessor.cpp line 69
* StatsProcessor.h line 53
* not sure what to do with AllProtocolTests.tcc


I take care about these:
* all generators using TMessageType
* including AS3, Go, Delphi, C#, Java, JavaME, Lua, Perl ... and Haxe^W^W




-----Ursprüngliche Nachricht----- 
From: Konrad Grochowski
Sent: Tuesday, September 16, 2014 12:56 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

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 <hcorg@minions.org.pl> wrote on 09/14/2014 09:08:28 
>>> AM:
>>>
>>>> From: Konrad Grochowski <hcorg@minions.org.pl>
>>>> 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 <steven.varga@gmail.com>
>>>> 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" <nhed+thriftusr@aereo.com>
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
>>> <steven.varga@gmail.com>
>>>>>>> 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();
>>>>>>>> }
>>>>>>>>
>>>>>>>>
>>
>


Mime
View raw message