You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@trafficserver.apache.org by "Alan M. Carroll" <am...@network-geographics.com> on 2017/11/08 15:08:17 UTC
[API Proposal] - TSVConnArgs
This came up with issues #2380 and #2388 and PR #2783. I had been waiting for some internal feedback on my proposal but since this is now active I am sending in my API proposal for attaching plugin data to NetVConnections (TSVConn).
https://solidwallofcode.github.io/api/TSVConnArgs.en.html#tsvconnargs
Some background on this proposal
https://solidwallofcode.github.io/vconn-args.en.html
Re: [API Proposal] - TSVConnArgs
Posted by Leif Hedstrom <zw...@apache.org>.
> On Nov 8, 2017, at 11:08 PM, Alan M. Carroll <am...@network-geographics.com> wrote:
>
> This came up with issues #2380 and #2388 and PR #2783. I had been waiting for some internal feedback on my proposal but since this is now active I am sending in my API proposal for attaching plugin data to NetVConnections (TSVConn).
>
> https://solidwallofcode.github.io/api/TSVConnArgs.en.html#tsvconnargs
>
> Some background on this proposal
>
> https://solidwallofcode.github.io/vconn-args.en.html
>
+1. Seems inline with the two existing “arg slot” mechanisms.
— leif
Re: [API Proposal] - TSVConnArgs
Posted by Chao Xu <ok...@apache.org>.
IMO, It is time to pull the ssl hooks from TSHttpHookID enum.
```
typedef enum {
TS_VCONN_FIRST_HOOK,
TS_VCONN_ACCEPTED_HOOK = TS_VCONN_FIRST_HOOK,
TS_VCONN_SSL_SNI_HOOK,
TS_VCONN_SSL_CERT_HOOK = TS_VCONN_SSL_SNI_HOOK,
TS_VCONN_SSL_SERVERNAME_HOOK,
TS_VCONN_OPENED_HOOK,
TS_VCONN_SSL_LAST_HOOK = TS_VCONN_OPENED_HOOK
} TSVConnHookID;
```
- Oknet
2017-11-15 22:58 GMT+08:00 Chao Xu <ok...@apache.org>:
> Hi AMC,
>
> " We should rename TS_VCONN_PRE_ACCEPT_HOOK to TS_VCONN_START_HOOK. "
>
> IMO, TS_VCONN_OPENED_HOOK when the OS connection is established.
> TS_VCONN_ACCEPTED_HOOK as a instead for TS_VCONN_PRE_ACCEPT_HOOK.
>
> - Oknet
>
> 2017-11-14 23:48 GMT+08:00 Dk Jack <dn...@gmail.com>:
>
>> I concur with the idea that connection level APIs should be different from
>> the
>> HTTP txn or ssn level APIs. For my use case, I am saving attributes at the
>> connection
>> level and accessing them during HTTP txn processing.
>>
>> On Tue, Nov 14, 2017 at 6:11 AM, Alan Carroll <
>> solidwallofcode@oath.com.invalid> wrote:
>>
>> > I thought we'd discussed this already, but I think having the same index
>> > for all three is a bad API design. I think the use cases are generally
>> > separate and conflating them effectively reduces the size of the
>> arrays. If
>> > I could, I'd change the TXN and SSN args to use separate indices and
>> would
>> > be happy to make a PR that does that. I suspect there is not even one
>> > plugin that depends on that behavior.
>> >
>> > On Tue, Nov 14, 2017 at 1:18 AM, Leif Hedstrom <zw...@apache.org>
>> wrote:
>> >
>> > >
>> > >
>> > > > On Nov 8, 2017, at 11:08 PM, Alan M. Carroll <
>> > > amc@network-geographics.com> wrote:
>> > > >
>> > > > This came up with issues #2380 and #2388 and PR #2783. I had been
>> > > waiting for some internal feedback on my proposal but since this is
>> now
>> > > active I am sending in my API proposal for attaching plugin data to
>> > > NetVConnections (TSVConn).
>> > > >
>> > > > https://solidwallofcode.github.io/api/TSVConnArgs.en.html#
>> tsvconnargs
>> > > >
>> > > > Some background on this proposal
>> > > >
>> > > > https://solidwallofcode.github.io/vconn-args.en.html
>> > > >
>> > >
>> > >
>> > > I redact my +1 :-).
>> > >
>> > > It seems we used one “index” lookup / storage for TXN and SSNs. Are we
>> > > sure we want a separate lookup function and table for the TSVConn?
>> That
>> > > seems inconsistent. I think if we’re going to do this, we should break
>> > > compatibility on the old SSN, and break that out of all of this. I.e.
>> > make
>> > >
>> > > TSHttpSsnArgIndexReserve
>> > >
>> > > and
>> > >
>> > > TSHttpTxnArgIndexReserve
>> > >
>> > >
>> > > etc. Otherwise, the proposal here seems very inconsistent with the
>> > > existing APIs, to the point of being confusing as hell. We should
>> either
>> > > change the new proposal to reuse the same index slots as previous
>> (they
>> > > really are per Plugins anyways), or we should fix the old APIs IMO.
>> > >
>> > > — Leif
>> > >
>> > >
>> >
>>
>
>
Re: [API Proposal] - TSVConnArgs
Posted by Leif Hedstrom <zw...@apache.org>.
> On Nov 15, 2017, at 8:33 AM, Alan Carroll <so...@oath.com.INVALID> wrote:
>
> 1) Generally the hook names aren't conjugated, more like
> 'TS_VCONN_CONNECT_HOOK' and 'TS_VCONN_ACCEPT_HOOK'.
> 2) 'TS_VCONN_CLOSE_HOOK' is required. That's one the the issues that
> sparked all of this, the inability to have a hook where VConn related data
> can be cleaned up.
>
Sorry for the late reply here. I think if we’re going to add a new “registry” for the VConn slots, we probably also should break the Ssn and Txn registry as well. I’m slightly torn on it, it seems more convenient if a plugin just registers for a single slot, and use that for all 3 APIs. On the flip side, separate registries could, in theory at least, allow for better utilization of the slots.
But, I think we should be consistent regardless of which route we take. Meaning, if we add this new registry, as another 8.0.0 incompatible change, we should break the registry for Txn and Ssn’s.
— leif
Re: [API Proposal] - TSVConnArgs
Posted by Alan Carroll <so...@oath.com.INVALID>.
1) Generally the hook names aren't conjugated, more like
'TS_VCONN_CONNECT_HOOK' and 'TS_VCONN_ACCEPT_HOOK'.
2) 'TS_VCONN_CLOSE_HOOK' is required. That's one the the issues that
sparked all of this, the inability to have a hook where VConn related data
can be cleaned up.
On Wed, Nov 15, 2017 at 9:18 AM, Chao Xu <ok...@apache.org> wrote:
> Yes, Origin Server. :-)
>
> TS_VCONN_CONNECTED_HOOK maybe more accurate than TS_VCONN_OPENED_HOOK for
> "once a connection is established"
>
> - Oknet
>
> 2017-11-15 23:08 GMT+08:00 Alan Carroll <solidwallofcode@oath.com.
> invalid>:
>
> > Ah, by "OS" you mean "Origin Server", not "Operating System".
> >
> > On Wed, Nov 15, 2017 at 9:06 AM, Chao Xu <ok...@apache.org> wrote:
> >
> > > TS_VCONN_OPENED_HOOK for OS side and TS_VCONN_ACCEPTED_HOOK for client
> > > side.
> > >
> > > - Oknet
> > >
> > > 2017-11-15 23:04 GMT+08:00 Alan Carroll <solidwallofcode@oath.com.
> > > invalid>:
> > >
> > > > How are those different? In terms of names, if you want consistency
> > then
> > > > TS_NET_ACCEPT_HOOK might be the best choice, aligning with
> > > > TS_EVENT_NET_ACCEPT which is the event that signals that action.
> > > >
> > > > On Wed, Nov 15, 2017 at 8:58 AM, Chao Xu <ok...@apache.org> wrote:
> > > >
> > > > > Hi AMC,
> > > > >
> > > > > " We should rename TS_VCONN_PRE_ACCEPT_HOOK to
> TS_VCONN_START_HOOK. "
> > > > >
> > > > > IMO, TS_VCONN_OPENED_HOOK when the OS connection is established.
> > > > > TS_VCONN_ACCEPTED_HOOK as a instead for TS_VCONN_PRE_ACCEPT_HOOK.
> > > > >
> > > > > - Oknet
> > > > >
> > > > > 2017-11-14 23:48 GMT+08:00 Dk Jack <dn...@gmail.com>:
> > > > >
> > > > > > I concur with the idea that connection level APIs should be
> > different
> > > > > from
> > > > > > the
> > > > > > HTTP txn or ssn level APIs. For my use case, I am saving
> attributes
> > > at
> > > > > the
> > > > > > connection
> > > > > > level and accessing them during HTTP txn processing.
> > > > > >
> > > > > > On Tue, Nov 14, 2017 at 6:11 AM, Alan Carroll <
> > > > > > solidwallofcode@oath.com.invalid> wrote:
> > > > > >
> > > > > > > I thought we'd discussed this already, but I think having the
> > same
> > > > > index
> > > > > > > for all three is a bad API design. I think the use cases are
> > > > generally
> > > > > > > separate and conflating them effectively reduces the size of
> the
> > > > > arrays.
> > > > > > If
> > > > > > > I could, I'd change the TXN and SSN args to use separate
> indices
> > > and
> > > > > > would
> > > > > > > be happy to make a PR that does that. I suspect there is not
> even
> > > one
> > > > > > > plugin that depends on that behavior.
> > > > > > >
> > > > > > > On Tue, Nov 14, 2017 at 1:18 AM, Leif Hedstrom <
> zwoop@apache.org
> > >
> > > > > wrote:
> > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > > > On Nov 8, 2017, at 11:08 PM, Alan M. Carroll <
> > > > > > > > amc@network-geographics.com> wrote:
> > > > > > > > >
> > > > > > > > > This came up with issues #2380 and #2388 and PR #2783. I
> had
> > > been
> > > > > > > > waiting for some internal feedback on my proposal but since
> > this
> > > is
> > > > > now
> > > > > > > > active I am sending in my API proposal for attaching plugin
> > data
> > > to
> > > > > > > > NetVConnections (TSVConn).
> > > > > > > > >
> > > > > > > > > https://solidwallofcode.github.io/api/TSVConnArgs.en.
> > > > > > html#tsvconnargs
> > > > > > > > >
> > > > > > > > > Some background on this proposal
> > > > > > > > >
> > > > > > > > > https://solidwallofcode.github.io/vconn-args.en.html
> > > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > > I redact my +1 :-).
> > > > > > > >
> > > > > > > > It seems we used one “index” lookup / storage for TXN and
> SSNs.
> > > Are
> > > > > we
> > > > > > > > sure we want a separate lookup function and table for the
> > > TSVConn?
> > > > > That
> > > > > > > > seems inconsistent. I think if we’re going to do this, we
> > should
> > > > > break
> > > > > > > > compatibility on the old SSN, and break that out of all of
> > this.
> > > > I.e.
> > > > > > > make
> > > > > > > >
> > > > > > > > TSHttpSsnArgIndexReserve
> > > > > > > >
> > > > > > > > and
> > > > > > > >
> > > > > > > > TSHttpTxnArgIndexReserve
> > > > > > > >
> > > > > > > >
> > > > > > > > etc. Otherwise, the proposal here seems very inconsistent
> with
> > > the
> > > > > > > > existing APIs, to the point of being confusing as hell. We
> > should
> > > > > > either
> > > > > > > > change the new proposal to reuse the same index slots as
> > previous
> > > > > (they
> > > > > > > > really are per Plugins anyways), or we should fix the old
> APIs
> > > IMO.
> > > > > > > >
> > > > > > > > — Leif
> > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: [API Proposal] - TSVConnArgs
Posted by Chao Xu <ok...@apache.org>.
Yes, Origin Server. :-)
TS_VCONN_CONNECTED_HOOK maybe more accurate than TS_VCONN_OPENED_HOOK for
"once a connection is established"
- Oknet
2017-11-15 23:08 GMT+08:00 Alan Carroll <so...@oath.com.invalid>:
> Ah, by "OS" you mean "Origin Server", not "Operating System".
>
> On Wed, Nov 15, 2017 at 9:06 AM, Chao Xu <ok...@apache.org> wrote:
>
> > TS_VCONN_OPENED_HOOK for OS side and TS_VCONN_ACCEPTED_HOOK for client
> > side.
> >
> > - Oknet
> >
> > 2017-11-15 23:04 GMT+08:00 Alan Carroll <solidwallofcode@oath.com.
> > invalid>:
> >
> > > How are those different? In terms of names, if you want consistency
> then
> > > TS_NET_ACCEPT_HOOK might be the best choice, aligning with
> > > TS_EVENT_NET_ACCEPT which is the event that signals that action.
> > >
> > > On Wed, Nov 15, 2017 at 8:58 AM, Chao Xu <ok...@apache.org> wrote:
> > >
> > > > Hi AMC,
> > > >
> > > > " We should rename TS_VCONN_PRE_ACCEPT_HOOK to TS_VCONN_START_HOOK. "
> > > >
> > > > IMO, TS_VCONN_OPENED_HOOK when the OS connection is established.
> > > > TS_VCONN_ACCEPTED_HOOK as a instead for TS_VCONN_PRE_ACCEPT_HOOK.
> > > >
> > > > - Oknet
> > > >
> > > > 2017-11-14 23:48 GMT+08:00 Dk Jack <dn...@gmail.com>:
> > > >
> > > > > I concur with the idea that connection level APIs should be
> different
> > > > from
> > > > > the
> > > > > HTTP txn or ssn level APIs. For my use case, I am saving attributes
> > at
> > > > the
> > > > > connection
> > > > > level and accessing them during HTTP txn processing.
> > > > >
> > > > > On Tue, Nov 14, 2017 at 6:11 AM, Alan Carroll <
> > > > > solidwallofcode@oath.com.invalid> wrote:
> > > > >
> > > > > > I thought we'd discussed this already, but I think having the
> same
> > > > index
> > > > > > for all three is a bad API design. I think the use cases are
> > > generally
> > > > > > separate and conflating them effectively reduces the size of the
> > > > arrays.
> > > > > If
> > > > > > I could, I'd change the TXN and SSN args to use separate indices
> > and
> > > > > would
> > > > > > be happy to make a PR that does that. I suspect there is not even
> > one
> > > > > > plugin that depends on that behavior.
> > > > > >
> > > > > > On Tue, Nov 14, 2017 at 1:18 AM, Leif Hedstrom <zwoop@apache.org
> >
> > > > wrote:
> > > > > >
> > > > > > >
> > > > > > >
> > > > > > > > On Nov 8, 2017, at 11:08 PM, Alan M. Carroll <
> > > > > > > amc@network-geographics.com> wrote:
> > > > > > > >
> > > > > > > > This came up with issues #2380 and #2388 and PR #2783. I had
> > been
> > > > > > > waiting for some internal feedback on my proposal but since
> this
> > is
> > > > now
> > > > > > > active I am sending in my API proposal for attaching plugin
> data
> > to
> > > > > > > NetVConnections (TSVConn).
> > > > > > > >
> > > > > > > > https://solidwallofcode.github.io/api/TSVConnArgs.en.
> > > > > html#tsvconnargs
> > > > > > > >
> > > > > > > > Some background on this proposal
> > > > > > > >
> > > > > > > > https://solidwallofcode.github.io/vconn-args.en.html
> > > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > I redact my +1 :-).
> > > > > > >
> > > > > > > It seems we used one “index” lookup / storage for TXN and SSNs.
> > Are
> > > > we
> > > > > > > sure we want a separate lookup function and table for the
> > TSVConn?
> > > > That
> > > > > > > seems inconsistent. I think if we’re going to do this, we
> should
> > > > break
> > > > > > > compatibility on the old SSN, and break that out of all of
> this.
> > > I.e.
> > > > > > make
> > > > > > >
> > > > > > > TSHttpSsnArgIndexReserve
> > > > > > >
> > > > > > > and
> > > > > > >
> > > > > > > TSHttpTxnArgIndexReserve
> > > > > > >
> > > > > > >
> > > > > > > etc. Otherwise, the proposal here seems very inconsistent with
> > the
> > > > > > > existing APIs, to the point of being confusing as hell. We
> should
> > > > > either
> > > > > > > change the new proposal to reuse the same index slots as
> previous
> > > > (they
> > > > > > > really are per Plugins anyways), or we should fix the old APIs
> > IMO.
> > > > > > >
> > > > > > > — Leif
> > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: [API Proposal] - TSVConnArgs
Posted by Alan Carroll <so...@oath.com.INVALID>.
Ah, by "OS" you mean "Origin Server", not "Operating System".
On Wed, Nov 15, 2017 at 9:06 AM, Chao Xu <ok...@apache.org> wrote:
> TS_VCONN_OPENED_HOOK for OS side and TS_VCONN_ACCEPTED_HOOK for client
> side.
>
> - Oknet
>
> 2017-11-15 23:04 GMT+08:00 Alan Carroll <solidwallofcode@oath.com.
> invalid>:
>
> > How are those different? In terms of names, if you want consistency then
> > TS_NET_ACCEPT_HOOK might be the best choice, aligning with
> > TS_EVENT_NET_ACCEPT which is the event that signals that action.
> >
> > On Wed, Nov 15, 2017 at 8:58 AM, Chao Xu <ok...@apache.org> wrote:
> >
> > > Hi AMC,
> > >
> > > " We should rename TS_VCONN_PRE_ACCEPT_HOOK to TS_VCONN_START_HOOK. "
> > >
> > > IMO, TS_VCONN_OPENED_HOOK when the OS connection is established.
> > > TS_VCONN_ACCEPTED_HOOK as a instead for TS_VCONN_PRE_ACCEPT_HOOK.
> > >
> > > - Oknet
> > >
> > > 2017-11-14 23:48 GMT+08:00 Dk Jack <dn...@gmail.com>:
> > >
> > > > I concur with the idea that connection level APIs should be different
> > > from
> > > > the
> > > > HTTP txn or ssn level APIs. For my use case, I am saving attributes
> at
> > > the
> > > > connection
> > > > level and accessing them during HTTP txn processing.
> > > >
> > > > On Tue, Nov 14, 2017 at 6:11 AM, Alan Carroll <
> > > > solidwallofcode@oath.com.invalid> wrote:
> > > >
> > > > > I thought we'd discussed this already, but I think having the same
> > > index
> > > > > for all three is a bad API design. I think the use cases are
> > generally
> > > > > separate and conflating them effectively reduces the size of the
> > > arrays.
> > > > If
> > > > > I could, I'd change the TXN and SSN args to use separate indices
> and
> > > > would
> > > > > be happy to make a PR that does that. I suspect there is not even
> one
> > > > > plugin that depends on that behavior.
> > > > >
> > > > > On Tue, Nov 14, 2017 at 1:18 AM, Leif Hedstrom <zw...@apache.org>
> > > wrote:
> > > > >
> > > > > >
> > > > > >
> > > > > > > On Nov 8, 2017, at 11:08 PM, Alan M. Carroll <
> > > > > > amc@network-geographics.com> wrote:
> > > > > > >
> > > > > > > This came up with issues #2380 and #2388 and PR #2783. I had
> been
> > > > > > waiting for some internal feedback on my proposal but since this
> is
> > > now
> > > > > > active I am sending in my API proposal for attaching plugin data
> to
> > > > > > NetVConnections (TSVConn).
> > > > > > >
> > > > > > > https://solidwallofcode.github.io/api/TSVConnArgs.en.
> > > > html#tsvconnargs
> > > > > > >
> > > > > > > Some background on this proposal
> > > > > > >
> > > > > > > https://solidwallofcode.github.io/vconn-args.en.html
> > > > > > >
> > > > > >
> > > > > >
> > > > > > I redact my +1 :-).
> > > > > >
> > > > > > It seems we used one “index” lookup / storage for TXN and SSNs.
> Are
> > > we
> > > > > > sure we want a separate lookup function and table for the
> TSVConn?
> > > That
> > > > > > seems inconsistent. I think if we’re going to do this, we should
> > > break
> > > > > > compatibility on the old SSN, and break that out of all of this.
> > I.e.
> > > > > make
> > > > > >
> > > > > > TSHttpSsnArgIndexReserve
> > > > > >
> > > > > > and
> > > > > >
> > > > > > TSHttpTxnArgIndexReserve
> > > > > >
> > > > > >
> > > > > > etc. Otherwise, the proposal here seems very inconsistent with
> the
> > > > > > existing APIs, to the point of being confusing as hell. We should
> > > > either
> > > > > > change the new proposal to reuse the same index slots as previous
> > > (they
> > > > > > really are per Plugins anyways), or we should fix the old APIs
> IMO.
> > > > > >
> > > > > > — Leif
> > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: [API Proposal] - TSVConnArgs
Posted by Chao Xu <ok...@apache.org>.
TS_VCONN_OPENED_HOOK for OS side and TS_VCONN_ACCEPTED_HOOK for client side.
- Oknet
2017-11-15 23:04 GMT+08:00 Alan Carroll <so...@oath.com.invalid>:
> How are those different? In terms of names, if you want consistency then
> TS_NET_ACCEPT_HOOK might be the best choice, aligning with
> TS_EVENT_NET_ACCEPT which is the event that signals that action.
>
> On Wed, Nov 15, 2017 at 8:58 AM, Chao Xu <ok...@apache.org> wrote:
>
> > Hi AMC,
> >
> > " We should rename TS_VCONN_PRE_ACCEPT_HOOK to TS_VCONN_START_HOOK. "
> >
> > IMO, TS_VCONN_OPENED_HOOK when the OS connection is established.
> > TS_VCONN_ACCEPTED_HOOK as a instead for TS_VCONN_PRE_ACCEPT_HOOK.
> >
> > - Oknet
> >
> > 2017-11-14 23:48 GMT+08:00 Dk Jack <dn...@gmail.com>:
> >
> > > I concur with the idea that connection level APIs should be different
> > from
> > > the
> > > HTTP txn or ssn level APIs. For my use case, I am saving attributes at
> > the
> > > connection
> > > level and accessing them during HTTP txn processing.
> > >
> > > On Tue, Nov 14, 2017 at 6:11 AM, Alan Carroll <
> > > solidwallofcode@oath.com.invalid> wrote:
> > >
> > > > I thought we'd discussed this already, but I think having the same
> > index
> > > > for all three is a bad API design. I think the use cases are
> generally
> > > > separate and conflating them effectively reduces the size of the
> > arrays.
> > > If
> > > > I could, I'd change the TXN and SSN args to use separate indices and
> > > would
> > > > be happy to make a PR that does that. I suspect there is not even one
> > > > plugin that depends on that behavior.
> > > >
> > > > On Tue, Nov 14, 2017 at 1:18 AM, Leif Hedstrom <zw...@apache.org>
> > wrote:
> > > >
> > > > >
> > > > >
> > > > > > On Nov 8, 2017, at 11:08 PM, Alan M. Carroll <
> > > > > amc@network-geographics.com> wrote:
> > > > > >
> > > > > > This came up with issues #2380 and #2388 and PR #2783. I had been
> > > > > waiting for some internal feedback on my proposal but since this is
> > now
> > > > > active I am sending in my API proposal for attaching plugin data to
> > > > > NetVConnections (TSVConn).
> > > > > >
> > > > > > https://solidwallofcode.github.io/api/TSVConnArgs.en.
> > > html#tsvconnargs
> > > > > >
> > > > > > Some background on this proposal
> > > > > >
> > > > > > https://solidwallofcode.github.io/vconn-args.en.html
> > > > > >
> > > > >
> > > > >
> > > > > I redact my +1 :-).
> > > > >
> > > > > It seems we used one “index” lookup / storage for TXN and SSNs. Are
> > we
> > > > > sure we want a separate lookup function and table for the TSVConn?
> > That
> > > > > seems inconsistent. I think if we’re going to do this, we should
> > break
> > > > > compatibility on the old SSN, and break that out of all of this.
> I.e.
> > > > make
> > > > >
> > > > > TSHttpSsnArgIndexReserve
> > > > >
> > > > > and
> > > > >
> > > > > TSHttpTxnArgIndexReserve
> > > > >
> > > > >
> > > > > etc. Otherwise, the proposal here seems very inconsistent with the
> > > > > existing APIs, to the point of being confusing as hell. We should
> > > either
> > > > > change the new proposal to reuse the same index slots as previous
> > (they
> > > > > really are per Plugins anyways), or we should fix the old APIs IMO.
> > > > >
> > > > > — Leif
> > > > >
> > > > >
> > > >
> > >
> >
>
Re: [API Proposal] - TSVConnArgs
Posted by Alan Carroll <so...@oath.com.INVALID>.
How are those different? In terms of names, if you want consistency then
TS_NET_ACCEPT_HOOK might be the best choice, aligning with
TS_EVENT_NET_ACCEPT which is the event that signals that action.
On Wed, Nov 15, 2017 at 8:58 AM, Chao Xu <ok...@apache.org> wrote:
> Hi AMC,
>
> " We should rename TS_VCONN_PRE_ACCEPT_HOOK to TS_VCONN_START_HOOK. "
>
> IMO, TS_VCONN_OPENED_HOOK when the OS connection is established.
> TS_VCONN_ACCEPTED_HOOK as a instead for TS_VCONN_PRE_ACCEPT_HOOK.
>
> - Oknet
>
> 2017-11-14 23:48 GMT+08:00 Dk Jack <dn...@gmail.com>:
>
> > I concur with the idea that connection level APIs should be different
> from
> > the
> > HTTP txn or ssn level APIs. For my use case, I am saving attributes at
> the
> > connection
> > level and accessing them during HTTP txn processing.
> >
> > On Tue, Nov 14, 2017 at 6:11 AM, Alan Carroll <
> > solidwallofcode@oath.com.invalid> wrote:
> >
> > > I thought we'd discussed this already, but I think having the same
> index
> > > for all three is a bad API design. I think the use cases are generally
> > > separate and conflating them effectively reduces the size of the
> arrays.
> > If
> > > I could, I'd change the TXN and SSN args to use separate indices and
> > would
> > > be happy to make a PR that does that. I suspect there is not even one
> > > plugin that depends on that behavior.
> > >
> > > On Tue, Nov 14, 2017 at 1:18 AM, Leif Hedstrom <zw...@apache.org>
> wrote:
> > >
> > > >
> > > >
> > > > > On Nov 8, 2017, at 11:08 PM, Alan M. Carroll <
> > > > amc@network-geographics.com> wrote:
> > > > >
> > > > > This came up with issues #2380 and #2388 and PR #2783. I had been
> > > > waiting for some internal feedback on my proposal but since this is
> now
> > > > active I am sending in my API proposal for attaching plugin data to
> > > > NetVConnections (TSVConn).
> > > > >
> > > > > https://solidwallofcode.github.io/api/TSVConnArgs.en.
> > html#tsvconnargs
> > > > >
> > > > > Some background on this proposal
> > > > >
> > > > > https://solidwallofcode.github.io/vconn-args.en.html
> > > > >
> > > >
> > > >
> > > > I redact my +1 :-).
> > > >
> > > > It seems we used one “index” lookup / storage for TXN and SSNs. Are
> we
> > > > sure we want a separate lookup function and table for the TSVConn?
> That
> > > > seems inconsistent. I think if we’re going to do this, we should
> break
> > > > compatibility on the old SSN, and break that out of all of this. I.e.
> > > make
> > > >
> > > > TSHttpSsnArgIndexReserve
> > > >
> > > > and
> > > >
> > > > TSHttpTxnArgIndexReserve
> > > >
> > > >
> > > > etc. Otherwise, the proposal here seems very inconsistent with the
> > > > existing APIs, to the point of being confusing as hell. We should
> > either
> > > > change the new proposal to reuse the same index slots as previous
> (they
> > > > really are per Plugins anyways), or we should fix the old APIs IMO.
> > > >
> > > > — Leif
> > > >
> > > >
> > >
> >
>
Re: [API Proposal] - TSVConnArgs
Posted by Chao Xu <ok...@apache.org>.
Hi AMC,
" We should rename TS_VCONN_PRE_ACCEPT_HOOK to TS_VCONN_START_HOOK. "
IMO, TS_VCONN_OPENED_HOOK when the OS connection is established.
TS_VCONN_ACCEPTED_HOOK as a instead for TS_VCONN_PRE_ACCEPT_HOOK.
- Oknet
2017-11-14 23:48 GMT+08:00 Dk Jack <dn...@gmail.com>:
> I concur with the idea that connection level APIs should be different from
> the
> HTTP txn or ssn level APIs. For my use case, I am saving attributes at the
> connection
> level and accessing them during HTTP txn processing.
>
> On Tue, Nov 14, 2017 at 6:11 AM, Alan Carroll <
> solidwallofcode@oath.com.invalid> wrote:
>
> > I thought we'd discussed this already, but I think having the same index
> > for all three is a bad API design. I think the use cases are generally
> > separate and conflating them effectively reduces the size of the arrays.
> If
> > I could, I'd change the TXN and SSN args to use separate indices and
> would
> > be happy to make a PR that does that. I suspect there is not even one
> > plugin that depends on that behavior.
> >
> > On Tue, Nov 14, 2017 at 1:18 AM, Leif Hedstrom <zw...@apache.org> wrote:
> >
> > >
> > >
> > > > On Nov 8, 2017, at 11:08 PM, Alan M. Carroll <
> > > amc@network-geographics.com> wrote:
> > > >
> > > > This came up with issues #2380 and #2388 and PR #2783. I had been
> > > waiting for some internal feedback on my proposal but since this is now
> > > active I am sending in my API proposal for attaching plugin data to
> > > NetVConnections (TSVConn).
> > > >
> > > > https://solidwallofcode.github.io/api/TSVConnArgs.en.
> html#tsvconnargs
> > > >
> > > > Some background on this proposal
> > > >
> > > > https://solidwallofcode.github.io/vconn-args.en.html
> > > >
> > >
> > >
> > > I redact my +1 :-).
> > >
> > > It seems we used one “index” lookup / storage for TXN and SSNs. Are we
> > > sure we want a separate lookup function and table for the TSVConn? That
> > > seems inconsistent. I think if we’re going to do this, we should break
> > > compatibility on the old SSN, and break that out of all of this. I.e.
> > make
> > >
> > > TSHttpSsnArgIndexReserve
> > >
> > > and
> > >
> > > TSHttpTxnArgIndexReserve
> > >
> > >
> > > etc. Otherwise, the proposal here seems very inconsistent with the
> > > existing APIs, to the point of being confusing as hell. We should
> either
> > > change the new proposal to reuse the same index slots as previous (they
> > > really are per Plugins anyways), or we should fix the old APIs IMO.
> > >
> > > — Leif
> > >
> > >
> >
>
Re: [API Proposal] - TSVConnArgs
Posted by Dk Jack <dn...@gmail.com>.
I concur with the idea that connection level APIs should be different from
the
HTTP txn or ssn level APIs. For my use case, I am saving attributes at the
connection
level and accessing them during HTTP txn processing.
On Tue, Nov 14, 2017 at 6:11 AM, Alan Carroll <
solidwallofcode@oath.com.invalid> wrote:
> I thought we'd discussed this already, but I think having the same index
> for all three is a bad API design. I think the use cases are generally
> separate and conflating them effectively reduces the size of the arrays. If
> I could, I'd change the TXN and SSN args to use separate indices and would
> be happy to make a PR that does that. I suspect there is not even one
> plugin that depends on that behavior.
>
> On Tue, Nov 14, 2017 at 1:18 AM, Leif Hedstrom <zw...@apache.org> wrote:
>
> >
> >
> > > On Nov 8, 2017, at 11:08 PM, Alan M. Carroll <
> > amc@network-geographics.com> wrote:
> > >
> > > This came up with issues #2380 and #2388 and PR #2783. I had been
> > waiting for some internal feedback on my proposal but since this is now
> > active I am sending in my API proposal for attaching plugin data to
> > NetVConnections (TSVConn).
> > >
> > > https://solidwallofcode.github.io/api/TSVConnArgs.en.html#tsvconnargs
> > >
> > > Some background on this proposal
> > >
> > > https://solidwallofcode.github.io/vconn-args.en.html
> > >
> >
> >
> > I redact my +1 :-).
> >
> > It seems we used one “index” lookup / storage for TXN and SSNs. Are we
> > sure we want a separate lookup function and table for the TSVConn? That
> > seems inconsistent. I think if we’re going to do this, we should break
> > compatibility on the old SSN, and break that out of all of this. I.e.
> make
> >
> > TSHttpSsnArgIndexReserve
> >
> > and
> >
> > TSHttpTxnArgIndexReserve
> >
> >
> > etc. Otherwise, the proposal here seems very inconsistent with the
> > existing APIs, to the point of being confusing as hell. We should either
> > change the new proposal to reuse the same index slots as previous (they
> > really are per Plugins anyways), or we should fix the old APIs IMO.
> >
> > — Leif
> >
> >
>
Re: [API Proposal] - TSVConnArgs
Posted by Alan Carroll <so...@oath.com.INVALID>.
I thought we'd discussed this already, but I think having the same index
for all three is a bad API design. I think the use cases are generally
separate and conflating them effectively reduces the size of the arrays. If
I could, I'd change the TXN and SSN args to use separate indices and would
be happy to make a PR that does that. I suspect there is not even one
plugin that depends on that behavior.
On Tue, Nov 14, 2017 at 1:18 AM, Leif Hedstrom <zw...@apache.org> wrote:
>
>
> > On Nov 8, 2017, at 11:08 PM, Alan M. Carroll <
> amc@network-geographics.com> wrote:
> >
> > This came up with issues #2380 and #2388 and PR #2783. I had been
> waiting for some internal feedback on my proposal but since this is now
> active I am sending in my API proposal for attaching plugin data to
> NetVConnections (TSVConn).
> >
> > https://solidwallofcode.github.io/api/TSVConnArgs.en.html#tsvconnargs
> >
> > Some background on this proposal
> >
> > https://solidwallofcode.github.io/vconn-args.en.html
> >
>
>
> I redact my +1 :-).
>
> It seems we used one “index” lookup / storage for TXN and SSNs. Are we
> sure we want a separate lookup function and table for the TSVConn? That
> seems inconsistent. I think if we’re going to do this, we should break
> compatibility on the old SSN, and break that out of all of this. I.e. make
>
> TSHttpSsnArgIndexReserve
>
> and
>
> TSHttpTxnArgIndexReserve
>
>
> etc. Otherwise, the proposal here seems very inconsistent with the
> existing APIs, to the point of being confusing as hell. We should either
> change the new proposal to reuse the same index slots as previous (they
> really are per Plugins anyways), or we should fix the old APIs IMO.
>
> — Leif
>
>
Re: [API Proposal] - TSVConnArgs
Posted by Leif Hedstrom <zw...@apache.org>.
> On Nov 8, 2017, at 11:08 PM, Alan M. Carroll <am...@network-geographics.com> wrote:
>
> This came up with issues #2380 and #2388 and PR #2783. I had been waiting for some internal feedback on my proposal but since this is now active I am sending in my API proposal for attaching plugin data to NetVConnections (TSVConn).
>
> https://solidwallofcode.github.io/api/TSVConnArgs.en.html#tsvconnargs
>
> Some background on this proposal
>
> https://solidwallofcode.github.io/vconn-args.en.html
>
I redact my +1 :-).
It seems we used one “index” lookup / storage for TXN and SSNs. Are we sure we want a separate lookup function and table for the TSVConn? That seems inconsistent. I think if we’re going to do this, we should break compatibility on the old SSN, and break that out of all of this. I.e. make
TSHttpSsnArgIndexReserve
and
TSHttpTxnArgIndexReserve
etc. Otherwise, the proposal here seems very inconsistent with the existing APIs, to the point of being confusing as hell. We should either change the new proposal to reuse the same index slots as previous (they really are per Plugins anyways), or we should fix the old APIs IMO.
— Leif