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 <solidwallofc...@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 <solidwallofc...@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 <dnj0...@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 < > > > > > > solidwallofc...@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 < > > > > > > > > a...@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 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
