thrift-user mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Konrad Grochowski <hc...@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 20:37:24 GMT
Please check https://github.com/apache/thrift/pull/216

I've fixed most (hopefully) generators there.

I'll take care of Peek/StatsProcessor etc (commits will also go to the 
same branch)

On 16.09.2014 21:32, Jens Geyer wrote:
>> 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