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 18:03:30 GMT
Somebody started to need that :)
oneway calls were just a "half" of normal call, when they should be 
treated differently - like in Steven's zeromq integration or Ben's 
pass-through server.
oneways are a really nice feature of thrift, so it would be nice to make 
them work properly. Also - if thrift already defines "4 / oneway" as 
additional message type it should use it :)

Konrad

On 16.09.2014 19:49, Jens Geyer wrote:
>
> Seems as if nobody really needs it?
>
>
> -----Ursprüngliche Nachricht----- From: Konrad Grochowski
> Sent: Tuesday, September 16, 2014 2:49 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
>
> 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