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 12:49:55 GMT
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 
>> <hcorg@minions.org.pl>
>> 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 <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