In general, I'm okay with the suggestion to make the API more generic, but,
there are a couple of things, that I'd like to highlight:
1. The definition of the enum being generic makes future evolutions of
different functional keys all over the place, rather than grouping related info
closer to preserve any sort of backward compatibility (I admit this is probably
not that big of a deal).
2. The API would *work* in different hooks for different set of info. For
example, the cache related info would only be available after CACHE_LOOKUP
complete state, while a different set of info (say, request header related
info) may be available at an earlier hook and a different set of info (say,
response header related info) may be available at a latter hook. This is
slightly not-in-line with the current API model, where a given API would either
work completely past a specific hook or would not work at all prior to a
specific hook.
If there's no concern on either of the above, I'd be okay to implement a
generic API with cache specific info for now and let it be extendible for
various other info.
Thanks,
Sudheer
On Wednesday, September 2, 2015 4:42 PM, James Peach <jpe...@apache.org>
wrote:
> On Sep 1, 2015, at 3:41 PM, Sudheer Vinukonda
> <sudhe...@yahoo-inc.com.INVALID> wrote:
>
> [resending with (hopefully) better formatting]
Summarizing the IRC discussion, this specific proposal is a move towards a
generalized API that aligns with logging tags. The goal there is to give
plugins access to the same information as logging tags have. Therefore, I
propose that we slightly retarget this proposal so that it can be cleanly
extended towards that goal.
Rename TSHttpTxnCacheStateGet() to TSHttpTxnInfoIntGet(). The "IntGet"
nomenclature is consistent with our other API families that can return
different data types. The "TSHttpTxnInfo" prefix should be read ad "give me
some arbitrary information about this transaction".
Rename TSCacheTxnStateKey to TSHttpTxnInfoKey, and the enumeration values to
TS_TXN_INFO_XXXX (eg. TS_TXN_INFO_CACHE_HIT_RAM).
This API could then evolve with
- the addition of more enums until it is equivalent to the logging tags
- the addition of TSHttpTxnInfoStringGet
- the addition of something like TSHttpLookupLoggingTag() so a plugin can
get the TxnInfo enum and data type from a logging tag string
I think that something like this would evolve relatively cleanly, and we would
not have to deprecate and replace this API later. One problem I can see is that
there's not a clean way to detect the valid set of enum values at compile time.
We should add duplicate #define names to help with this.
If there's no consensus on the above, I'm reasonably OK with this proposal :)
> On Tuesday, September 1, 2015 2:54 PM, Sudheer Vinukonda
> <sudhe...@yahoo-inc.com> wrote:
>
>
>
> I'd like to add a new API TSHttpTxnCacheStateGet() to be able to get more
> insight into cache operations (e.g read_while_write, open read tries, open
> write tries, cache volume used etc) on a given Txn.
>
> Below's the proposal with the info I'd like the new API return along with the
> API signature. I've opened TS-3881 to track this.
>
> Please provide comments/suggestions.
>
> +typedef enum {
> + TS_CACHE_TXN_STATE_HIT_RAM,
> + TS_CACHE_TXN_STATE_READ_WHILE_WRITER,
> + TS_CACHE_TXN_STATE_OPEN_READ_TRIES,
> + TS_CACHE_TXN_STATE_OPEN_WRITE_TRIES,
> + TS_CACHE_TXN_STATE_CACHE_VOLUME,
> + TS_CACHE_TXN_STATE_LAST_ENTRY
> +} TSCacheTxnStateKey;
> +
>
> +/* Get Cache Lookup state, useful for understanding how a lookup was
> performed */
> +/**
> + Return the particular cache lookup info requested.
> +
> + @param txnp the transaction pointer
> + @param key the requested cache lookup info.
> + @param TSMgmtInt a pointer to a integer where we will store the metric
> value
> +
> + @return @c TS_SUCCESS if the metric is supported, TS_ERROR otherwise
> +
> +*/
> +tsapi TSReturnCode TSHttpTxnCacheStateGet(TSHttpTxn txnp, TSCacheStateKey
> key, TSMgmtInt *value);
> +
>
>
> Thanks,
>
> Sudheer
