On Fri, Mar 28, 2014 at 8:00 AM, James Peach <jpe...@apache.org> wrote:
> On Mar 13, 2014, at 2:56 PM, James Peach <jpe...@apache.org> wrote:
>
> > On Mar 11, 2014, at 8:12 PM, yun...@apache.org wrote:
> >
> >> TS-2612: Indroduce TSHttpConnectWithProtoStack() API
> >>
> >> 1) Firstly, add a bitmask, *proto_stack*, in NetVConnection/HttpSM,
> >> and set it properly.
> >>
> >> 2) For some plugins that using TSHttpConnect() API to do request,
> >> the Logging module can't know what protocol stack is used, so I
> >> add a new API:
> >>
> >> TSHttpConnectWithProtoStack(struct sockaddr const* addr,
> >> TSClientProtoStack proto_stack);
> >>
> >> After introducing TSHttpConnectWithProtoStack() API, TSHttpConnect() API
> >> would be a special case of it:
> >>
> >> TSVConn
> >> TSHttpConnect(sockaddr const* addr)
> >> {
> >> return TSHttpConnectWithProtoStack(addr, (1u << TS_PROTO_HTTP));
> >> }
> >>
> >> Signed-off-by: Yunkai Zhang <qiushu....@taobao.com>
> >
> > This needs API review since the final form of the API was not reviewed
> on dev@. I'll try to review this next week. Everyone else who reviewed
> the original proposal should also review :)
>
> I like the name "TSClientProtoStack". I don't like that it is tied to
> TSHttpConnect; since it is a property of the VConn, doesn't it make more
> sense to be able to get and set it on a TSVConn?
>
It seems not easy to do that, before a new VConn returned by
TSHttpConnect() API, the connection might have been established
asynchronously. That is say, unexpected client proto stack whould be passed
to HttpSM before it was set, and HttpSM will copy the unexpected value to
its internal HttpSM->proto_stack variable -- IIUC, logging module should
not read VConn->proto_stack directly as VConn might have been released in
logging phase.
>
> I don't think that users should have to deal with the bitmask
> representation directly. It would be better to separate this into 2 types
> (transport protocol and application protocol), and then do the bitshifting
> internally. We should have internal helper functions to unpack the
> bitfields.
>
I just worry about that there may exist some clients contain more than 2
types in its proto stack in the future.
>
> Would "WS" be a more conventional abbreviation for WebSockets? It took me
> a while to figure out "WBSK" :)
>
OK to me:)
>
> J
>
--
Yunkai Zhang
Work at Taobao
