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 11:25:59 GMT
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