You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@trafficserver.apache.org by Leif Hedstrom <zw...@apache.org> on 2016/06/10 17:29:16 UTC
API-review: UUID APIs
Hi,
as part generalizing the concept from https://github.com/apache/trafficserver/pull/199 <https://github.com/apache/trafficserver/pull/199>, I’ve filed a Jira to add appropriate APIs and I_Machine.h / Machine.cc <http://machine.cc/> support for the concept of a process UUID. The apidefs.h are
/* --------------------------------------------------------------------------
Interface for the UUID APIs. https://www.ietf.org/rfc/rfc4122.txt <https://www.ietf.org/rfc/rfc4122.txt> . */
typedef enum {
TS_UUID_V1 = 1,
TS_UUID_V2,
TS_UUID_V3,
TS_UUID_V4, /* At this point, this is the only implemented version (or variant) */
TS_UUID_V5,
} TSUuidVersion;
#define TSUuidStringLen 36
typedef struct tsapi_uuid *TSUuid;
The ts.h APIs are:
/* APIs for dealing with UUIDs, either self made, or the system wide process UUID. */
tsapi TSUuid TSUuidCreate(TSUuidVersion v);
tsapi TSReturnCode TSUuidDestroy(TSUuid uuid);
tsapi TSReturnCode TSUuidCopy(TSUuid dest, TSUuid src);
tsapi TSReturnCode TSUuidInitialize(TSUuid uuid); /* Re-generate the UUID */
tsapi const TSUuid TSProcessUuidGet();
tsapi const char *TSUuidStringGet(const TSUuid uuid);
and an additional API to expose the already existing HttpSM (Txn) sequence ID:
/* Get the Txn's (HttpSM's) unique identifier, which is a sequence number since server start) */
tsapi int64_t TSHttpTxnIdGet();
The APIs are documented in a separate Sphinx file, i.e. no doxygen crud in the ts.h file :). The TSUuidInitialize() functions is maybe a little confusing, what it does is basically re-initialize the UUID object, thus creating a new UUID value. Also, the UUID implementation does not rely on any external dependency, it’s easy enough to implement this ourselves.
I’m definitely open for suggestions for improvements and better naming conventions, but of course have reasonable arguments. There will be a couple of more Jira’s to complete Acacio’s initial work here, go get log tags as well as use these UUIDs in e.g. HttpSM diagnostic traces.
Cheers,
— leif
P.s
It does have regression tests:
[root@fedora ats]# ./bin/traffic_server -R 1 -r SDK_API_UUID
traffic_server: using root directory '/opt/ats'
REGRESSION_TEST initialization begun
REGRESSION TEST SDK_API_UUID started
[SDK_API_UUID] TSProcessUuidGet : [TestCase1] <<PASS>> { ok }
[SDK_API_UUID] TSProcessUuidGet : [TestCase2] <<PASS>> { ok }
[SDK_API_UUID] TSUuidStringGet : [TestCase1] <<PASS>> { ok }
[SDK_API_UUID] TSUuidCreate : [TestCase1] <<PASS>> { ok }
[SDK_API_UUID] TSUuidCopy : [TestCase1] <<PASS>> { ok }
[SDK_API_UUID] TSUuidCopy : [TestCase2] <<PASS>> { ok }
[SDK_API_UUID] TSUuidInitialize : [TestCase1] <<PASS>> { ok }
[SDK_API_UUID] TSUuidInitialize : [TestCase2] <<PASS>> { ok }
[SDK_API_UUID] TSUuidDestroy : [TestCase1] <<PASS>> { ok }
REGRESSION_RESULT SDK_API_UUID: PASSED
REGRESSION_TEST DONE: PASSED
Re: API-review: UUID APIs
Posted by Jason Kenny <jk...@yahoo-inc.com.INVALID>.
I think the only real thing I could add to this is that it would be nice if we could use the same internal C++ class as part of the C++api, vs wrapping another class around the C API.
Jason
From: Leif Hedstrom <zw...@apache.org>
To: dev@trafficserver.apache.org
Sent: Saturday, June 11, 2016 4:56 PM
Subject: Re: API-review: UUID APIs
> On Jun 11, 2016, at 11:07 AM, James Peach <jp...@apache.org> wrote:
>
>>
>> On Jun 10, 2016, at 3:19 PM, Leif Hedstrom <zw...@apache.org> wrote:
>>
>> Thanks James, as usual great review. Replies inline.
>>
>>> On Jun 10, 2016, at 2:30 PM, James Peach <ja...@me.com> wrote:
>>>
>>>
>>>> On Jun 10, 2016, at 10:29 AM, Leif Hedstrom <zw...@apache.org> wrote:
>>>>
>>>> Hi,
>>>>
>>>> as part generalizing the concept from https://github.com/apache/trafficserver/pull/199 <https://github.com/apache/trafficserver/pull/199>, I’ve filed a Jira to add appropriate APIs and I_Machine.h / Machine.cc <http://machine.cc/> support for the concept of a process UUID. The apidefs.h are
>>>>
>>>> /* --------------------------------------------------------------------------
>>>> Interface for the UUID APIs. https://www.ietf.org/rfc/rfc4122.txt <https://www.ietf.org/rfc/rfc4122.txt> . */
>>>> typedef enum {
>>>> TS_UUID_V1 = 1,
>>>> TS_UUID_V2,
>>>> TS_UUID_V3,
>>>> TS_UUID_V4, /* At this point, this is the only implemented version (or variant) */
>>>> TS_UUID_V5,
>>>> } TSUuidVersion;
>>>>
>>>> #define TSUuidStringLen 36
>>>
>>> Is this define useful? I think we should drop it ... in libuuid it is mainly used for formatting a string and this API doesn't need that.
>>
>> Yeah, I’m fine with that, the only time it’d be useful is if you want to allocate something on the stack to hold a copy I think, or want to avoid doing a strlen() for e.g. a malloc. No strong opinion from me. :)
>
> That makes sense. It would be useful for TSUuidStringParse too I think.
>
>>>> typedef struct tsapi_uuid *TSUuid;
>>>>
>>>>
>>>> The ts.h APIs are:
>>>>
>>>> /* APIs for dealing with UUIDs, either self made, or the system wide process UUID. */
>>>> tsapi TSUuid TSUuidCreate(TSUuidVersion v);
>>>
>>> A helpful extension of this might be TSHttpTxnUuidCreate() which would allocate the UUID in the transaction arena.
>>
>> Yep. I’ll file a Jira if no one else beats me to it.
>>
>>
>>>
>>>> tsapi TSReturnCode TSUuidDestroy(TSUuid uuid);
>>>> tsapi TSReturnCode TSUuidCopy(TSUuid dest, TSUuid src);
>>>> tsapi TSReturnCode TSUuidInitialize(TSUuid uuid); /* Re-generate the UUID */
>>>
>>> If you TSUuidCreate() but don't TSUuidInitialize(), what do you get?
>>
>> It’ll still be initialized. The Initialize is really just a “re-initialize”, to get a new, random UUID. I don’t know how useful it is, but it’s also harmless. Maybe we need a better name? TSUuidReinitialize()
>
> Ok, do we need this at all then? I am not sure why I would get a UUID and then change its value?
The thinking was if you want to generate a new UUID many times, without the allocations necessary to get the “generator” object.
Now, after we added the TSUuidStringParse() function (done), I started thinking that maybe a cleaner API is something in the line of (without error checking):
uuid = TSUuidCreate();
TSUuidInitialize(uuid, TS_UUID_V4);
and
uuid = TSUuidCreate();
TSUuidStringParse(uuid, "65c65060-072e-4f9c-a76f-89f5f245354a”); // The string holds the version, so no issue there
This definitely seems cleaner to me, specially now that we have at least two ways of “generating” the actual UUID? Thoughts?
>>
>> Roger. So,
>>
>> if (TS_SUCCESS == TSUuidStringParse(uuid, "8E3C2CA3-3603-4A85-A814-8415F395D904”)) { …},
>>
>> right ? Do we want an int len third (optional) parameter? For the case of non-null terminated strings.
>
> I don’t think you need the length since the string is known fixed length. uuid_parse(3) doesn’t use it either. Though if there’s no length parameter maybe we should add a constant for the length so the caller can check?
Ok, so we agree on:
tsapi TSReturnCode TSUuidStringParse(TSUuid uuid, const char *uuid_str);
>
>>>> and an additional API to expose the already existing HttpSM (Txn) sequence ID:
>>>>
>>>> /* Get the Txn's (HttpSM's) unique identifier, which is a sequence number since server start) */
>>>> tsapi int64_t TSHttpTxnIdGet();
>>>
>>> Why int64_t rather than uint64_t?
>>
>> Cause HttpSM.sm_id is int64_t :-). We’d have to change things in a lot of places for that to be addressed, which we can do if you feel strongly, but it’s a fair amount of work.
>
> Yeh I think we should do this. uint64_t wraps nicely. Just cast it for this API, then come back and change it everywhere.
I doubt anyone will ever change the code, and this is borderline bike shedding; It’s clearly a non issue, since we’ll never, ever wrap this ID. Even at 300,000 QPS, it’d take a million years to wrap it. But, I don’t really care, so I’ll change it to uint64_t.
Cheers,
— leif
Re: API-review: UUID APIs
Posted by Leif Hedstrom <zw...@apache.org>.
> On Jun 11, 2016, at 11:07 AM, James Peach <jp...@apache.org> wrote:
>
>>
>> On Jun 10, 2016, at 3:19 PM, Leif Hedstrom <zw...@apache.org> wrote:
>>
>> Thanks James, as usual great review. Replies inline.
>>
>>> On Jun 10, 2016, at 2:30 PM, James Peach <ja...@me.com> wrote:
>>>
>>>
>>>> On Jun 10, 2016, at 10:29 AM, Leif Hedstrom <zw...@apache.org> wrote:
>>>>
>>>> Hi,
>>>>
>>>> as part generalizing the concept from https://github.com/apache/trafficserver/pull/199 <https://github.com/apache/trafficserver/pull/199>, I’ve filed a Jira to add appropriate APIs and I_Machine.h / Machine.cc <http://machine.cc/> support for the concept of a process UUID. The apidefs.h are
>>>>
>>>> /* --------------------------------------------------------------------------
>>>> Interface for the UUID APIs. https://www.ietf.org/rfc/rfc4122.txt <https://www.ietf.org/rfc/rfc4122.txt> . */
>>>> typedef enum {
>>>> TS_UUID_V1 = 1,
>>>> TS_UUID_V2,
>>>> TS_UUID_V3,
>>>> TS_UUID_V4, /* At this point, this is the only implemented version (or variant) */
>>>> TS_UUID_V5,
>>>> } TSUuidVersion;
>>>>
>>>> #define TSUuidStringLen 36
>>>
>>> Is this define useful? I think we should drop it ... in libuuid it is mainly used for formatting a string and this API doesn't need that.
>>
>> Yeah, I’m fine with that, the only time it’d be useful is if you want to allocate something on the stack to hold a copy I think, or want to avoid doing a strlen() for e.g. a malloc. No strong opinion from me. :)
>
> That makes sense. It would be useful for TSUuidStringParse too I think.
>
>>>> typedef struct tsapi_uuid *TSUuid;
>>>>
>>>>
>>>> The ts.h APIs are:
>>>>
>>>> /* APIs for dealing with UUIDs, either self made, or the system wide process UUID. */
>>>> tsapi TSUuid TSUuidCreate(TSUuidVersion v);
>>>
>>> A helpful extension of this might be TSHttpTxnUuidCreate() which would allocate the UUID in the transaction arena.
>>
>> Yep. I’ll file a Jira if no one else beats me to it.
>>
>>
>>>
>>>> tsapi TSReturnCode TSUuidDestroy(TSUuid uuid);
>>>> tsapi TSReturnCode TSUuidCopy(TSUuid dest, TSUuid src);
>>>> tsapi TSReturnCode TSUuidInitialize(TSUuid uuid); /* Re-generate the UUID */
>>>
>>> If you TSUuidCreate() but don't TSUuidInitialize(), what do you get?
>>
>> It’ll still be initialized. The Initialize is really just a “re-initialize”, to get a new, random UUID. I don’t know how useful it is, but it’s also harmless. Maybe we need a better name? TSUuidReinitialize()
>
> Ok, do we need this at all then? I am not sure why I would get a UUID and then change its value?
The thinking was if you want to generate a new UUID many times, without the allocations necessary to get the “generator” object.
Now, after we added the TSUuidStringParse() function (done), I started thinking that maybe a cleaner API is something in the line of (without error checking):
uuid = TSUuidCreate();
TSUuidInitialize(uuid, TS_UUID_V4);
and
uuid = TSUuidCreate();
TSUuidStringParse(uuid, "65c65060-072e-4f9c-a76f-89f5f245354a”); // The string holds the version, so no issue there
This definitely seems cleaner to me, specially now that we have at least two ways of “generating” the actual UUID? Thoughts?
>>
>> Roger. So,
>>
>> if (TS_SUCCESS == TSUuidStringParse(uuid, "8E3C2CA3-3603-4A85-A814-8415F395D904”)) { …},
>>
>> right ? Do we want an int len third (optional) parameter? For the case of non-null terminated strings.
>
> I don’t think you need the length since the string is known fixed length. uuid_parse(3) doesn’t use it either. Though if there’s no length parameter maybe we should add a constant for the length so the caller can check?
Ok, so we agree on:
tsapi TSReturnCode TSUuidStringParse(TSUuid uuid, const char *uuid_str);
>
>>>> and an additional API to expose the already existing HttpSM (Txn) sequence ID:
>>>>
>>>> /* Get the Txn's (HttpSM's) unique identifier, which is a sequence number since server start) */
>>>> tsapi int64_t TSHttpTxnIdGet();
>>>
>>> Why int64_t rather than uint64_t?
>>
>> Cause HttpSM.sm_id is int64_t :-). We’d have to change things in a lot of places for that to be addressed, which we can do if you feel strongly, but it’s a fair amount of work.
>
> Yeh I think we should do this. uint64_t wraps nicely. Just cast it for this API, then come back and change it everywhere.
I doubt anyone will ever change the code, and this is borderline bike shedding; It’s clearly a non issue, since we’ll never, ever wrap this ID. Even at 300,000 QPS, it’d take a million years to wrap it. But, I don’t really care, so I’ll change it to uint64_t.
Cheers,
— leif
Re: API-review: UUID APIs
Posted by James Peach <jp...@apache.org>.
> On Jun 10, 2016, at 3:19 PM, Leif Hedstrom <zw...@apache.org> wrote:
>
> Thanks James, as usual great review. Replies inline.
>
>> On Jun 10, 2016, at 2:30 PM, James Peach <ja...@me.com> wrote:
>>
>>
>>> On Jun 10, 2016, at 10:29 AM, Leif Hedstrom <zw...@apache.org> wrote:
>>>
>>> Hi,
>>>
>>> as part generalizing the concept from https://github.com/apache/trafficserver/pull/199 <https://github.com/apache/trafficserver/pull/199>, I’ve filed a Jira to add appropriate APIs and I_Machine.h / Machine.cc <http://machine.cc/> support for the concept of a process UUID. The apidefs.h are
>>>
>>> /* --------------------------------------------------------------------------
>>> Interface for the UUID APIs. https://www.ietf.org/rfc/rfc4122.txt <https://www.ietf.org/rfc/rfc4122.txt> . */
>>> typedef enum {
>>> TS_UUID_V1 = 1,
>>> TS_UUID_V2,
>>> TS_UUID_V3,
>>> TS_UUID_V4, /* At this point, this is the only implemented version (or variant) */
>>> TS_UUID_V5,
>>> } TSUuidVersion;
>>>
>>> #define TSUuidStringLen 36
>>
>> Is this define useful? I think we should drop it ... in libuuid it is mainly used for formatting a string and this API doesn't need that.
>
> Yeah, I’m fine with that, the only time it’d be useful is if you want to allocate something on the stack to hold a copy I think, or want to avoid doing a strlen() for e.g. a malloc. No strong opinion from me. :)
That makes sense. It would be useful for TSUuidStringParse too I think.
>>> typedef struct tsapi_uuid *TSUuid;
>>>
>>>
>>> The ts.h APIs are:
>>>
>>> /* APIs for dealing with UUIDs, either self made, or the system wide process UUID. */
>>> tsapi TSUuid TSUuidCreate(TSUuidVersion v);
>>
>> A helpful extension of this might be TSHttpTxnUuidCreate() which would allocate the UUID in the transaction arena.
>
> Yep. I’ll file a Jira if no one else beats me to it.
>
>
>>
>>> tsapi TSReturnCode TSUuidDestroy(TSUuid uuid);
>>> tsapi TSReturnCode TSUuidCopy(TSUuid dest, TSUuid src);
>>> tsapi TSReturnCode TSUuidInitialize(TSUuid uuid); /* Re-generate the UUID */
>>
>> If you TSUuidCreate() but don't TSUuidInitialize(), what do you get?
>
> It’ll still be initialized. The Initialize is really just a “re-initialize”, to get a new, random UUID. I don’t know how useful it is, but it’s also harmless. Maybe we need a better name? TSUuidReinitialize() ?
Ok, do we need this at all then? I am not sure why I would get a UUID and then change its value?
>>> tsapi const TSUuid TSProcessUuidGet();
>>
>> Is this persistent across restarts? As we discussed on IRC, it would be good to expose this value in metrics and traffic_ctl.
>
> No, not persistent. I added the metric as discussed, but if we need more oomph in traffic_ctl, lets file a separate Jira.
>
> For persistent UUID, I’m ok adding that as well, although sort of feels that the “hostname” is that unique, persistent ID.
>
>
>>
>>> tsapi const char *TSUuidStringGet(const TSUuid uuid);
>>
>> Does this return an allocated string?
>
> No, it’s part of the UUID object. It has the same lifetime as the underlying TSUuid object (which is forever in the case of the Process UUID). I’ll make sure to document this properly.
>
> To be clear, there is no transfer of ownership here, you just get a pointer right into the implementations data. If we feel strongly, we can change that, but that makes for more complex APIs, and not sure of the value. We do the exact same things with most strings, to avoid unnecessary copies in our APIs. :).
That makes sense.
>> Consider TSUuid TSUuidStringParse() as an additional API.
>
>
> Roger. So,
>
> if (TS_SUCCESS == TSUuidStringParse(uuid, "8E3C2CA3-3603-4A85-A814-8415F395D904”)) { …},
>
> right ? Do we want an int len third (optional) parameter? For the case of non-null terminated strings.
I don’t think you need the length since the string is known fixed length. uuid_parse(3) doesn’t use it either. Though if there’s no length parameter maybe we should add a constant for the length so the caller can check?
>>> and an additional API to expose the already existing HttpSM (Txn) sequence ID:
>>>
>>> /* Get the Txn's (HttpSM's) unique identifier, which is a sequence number since server start) */
>>> tsapi int64_t TSHttpTxnIdGet();
>>
>> Why int64_t rather than uint64_t?
>
> Cause HttpSM.sm_id is int64_t :-). We’d have to change things in a lot of places for that to be addressed, which we can do if you feel strongly, but it’s a fair amount of work.
Yeh I think we should do this. uint64_t wraps nicely. Just cast it for this API, then come back and change it everywhere.
>>> The APIs are documented in a separate Sphinx file, i.e. no doxygen crud in the ts.h file :).
>>
>> The documentation should be posted with the review to save a lot of redundant questions.
>
> Agreed. I’m in a special chicken-and-egg situation, so don’t do what I do :-/.
>
> New metric that was added per the IRC discussions:
>
> [root@fedora ats]# ./bin/traffic_ctl metric match uuid
> proxy.process.version.server.uuid 42f4da08-29cf-4a93-942a-d72eb526d18d
Nice.
J
Re: API-review: UUID APIs
Posted by Leif Hedstrom <zw...@apache.org>.
Thanks James, as usual great review. Replies inline.
> On Jun 10, 2016, at 2:30 PM, James Peach <ja...@me.com> wrote:
>
>
>> On Jun 10, 2016, at 10:29 AM, Leif Hedstrom <zw...@apache.org> wrote:
>>
>> Hi,
>>
>> as part generalizing the concept from https://github.com/apache/trafficserver/pull/199 <https://github.com/apache/trafficserver/pull/199>, I’ve filed a Jira to add appropriate APIs and I_Machine.h / Machine.cc <http://machine.cc/> support for the concept of a process UUID. The apidefs.h are
>>
>> /* --------------------------------------------------------------------------
>> Interface for the UUID APIs. https://www.ietf.org/rfc/rfc4122.txt <https://www.ietf.org/rfc/rfc4122.txt> . */
>> typedef enum {
>> TS_UUID_V1 = 1,
>> TS_UUID_V2,
>> TS_UUID_V3,
>> TS_UUID_V4, /* At this point, this is the only implemented version (or variant) */
>> TS_UUID_V5,
>> } TSUuidVersion;
>>
>> #define TSUuidStringLen 36
>
> Is this define useful? I think we should drop it ... in libuuid it is mainly used for formatting a string and this API doesn't need that.
Yeah, I’m fine with that, the only time it’d be useful is if you want to allocate something on the stack to hold a copy I think, or want to avoid doing a strlen() for e.g. a malloc. No strong opinion from me. :)
>
>> typedef struct tsapi_uuid *TSUuid;
>>
>>
>> The ts.h APIs are:
>>
>> /* APIs for dealing with UUIDs, either self made, or the system wide process UUID. */
>> tsapi TSUuid TSUuidCreate(TSUuidVersion v);
>
> A helpful extension of this might be TSHttpTxnUuidCreate() which would allocate the UUID in the transaction arena.
Yep. I’ll file a Jira if no one else beats me to it.
>
>> tsapi TSReturnCode TSUuidDestroy(TSUuid uuid);
>> tsapi TSReturnCode TSUuidCopy(TSUuid dest, TSUuid src);
>> tsapi TSReturnCode TSUuidInitialize(TSUuid uuid); /* Re-generate the UUID */
>
> If you TSUuidCreate() but don't TSUuidInitialize(), what do you get?
It’ll still be initialized. The Initialize is really just a “re-initialize”, to get a new, random UUID. I don’t know how useful it is, but it’s also harmless. Maybe we need a better name? TSUuidReinitialize() ?
>
>> tsapi const TSUuid TSProcessUuidGet();
>
> Is this persistent across restarts? As we discussed on IRC, it would be good to expose this value in metrics and traffic_ctl.
No, not persistent. I added the metric as discussed, but if we need more oomph in traffic_ctl, lets file a separate Jira.
For persistent UUID, I’m ok adding that as well, although sort of feels that the “hostname” is that unique, persistent ID.
>
>> tsapi const char *TSUuidStringGet(const TSUuid uuid);
>
> Does this return an allocated string?
No, it’s part of the UUID object. It has the same lifetime as the underlying TSUuid object (which is forever in the case of the Process UUID). I’ll make sure to document this properly.
To be clear, there is no transfer of ownership here, you just get a pointer right into the implementations data. If we feel strongly, we can change that, but that makes for more complex APIs, and not sure of the value. We do the exact same things with most strings, to avoid unnecessary copies in our APIs. :).
>
> Consider TSUuid TSUuidStringParse() as an additional API.
Roger. So,
if (TS_SUCCESS == TSUuidStringParse(uuid, "8E3C2CA3-3603-4A85-A814-8415F395D904”)) { …},
right ? Do we want an int len third (optional) parameter? For the case of non-null terminated strings.
>
>> and an additional API to expose the already existing HttpSM (Txn) sequence ID:
>>
>> /* Get the Txn's (HttpSM's) unique identifier, which is a sequence number since server start) */
>> tsapi int64_t TSHttpTxnIdGet();
>
> Why int64_t rather than uint64_t?
Cause HttpSM.sm_id is int64_t :-). We’d have to change things in a lot of places for that to be addressed, which we can do if you feel strongly, but it’s a fair amount of work.
>
>> The APIs are documented in a separate Sphinx file, i.e. no doxygen crud in the ts.h file :).
>
> The documentation should be posted with the review to save a lot of redundant questions.
Agreed. I’m in a special chicken-and-egg situation, so don’t do what I do :-/.
New metric that was added per the IRC discussions:
[root@fedora ats]# ./bin/traffic_ctl metric match uuid
proxy.process.version.server.uuid 42f4da08-29cf-4a93-942a-d72eb526d18d
Cheers,
— leif
Re: API-review: UUID APIs
Posted by James Peach <ja...@me.com>.
> On Jun 10, 2016, at 10:29 AM, Leif Hedstrom <zw...@apache.org> wrote:
>
> Hi,
>
> as part generalizing the concept from https://github.com/apache/trafficserver/pull/199 <https://github.com/apache/trafficserver/pull/199>, I’ve filed a Jira to add appropriate APIs and I_Machine.h / Machine.cc <http://machine.cc/> support for the concept of a process UUID. The apidefs.h are
>
> /* --------------------------------------------------------------------------
> Interface for the UUID APIs. https://www.ietf.org/rfc/rfc4122.txt <https://www.ietf.org/rfc/rfc4122.txt> . */
> typedef enum {
> TS_UUID_V1 = 1,
> TS_UUID_V2,
> TS_UUID_V3,
> TS_UUID_V4, /* At this point, this is the only implemented version (or variant) */
> TS_UUID_V5,
> } TSUuidVersion;
>
> #define TSUuidStringLen 36
Is this define useful? I think we should drop it ... in libuuid it is mainly used for formatting a string and this API doesn't need that.
> typedef struct tsapi_uuid *TSUuid;
>
>
> The ts.h APIs are:
>
> /* APIs for dealing with UUIDs, either self made, or the system wide process UUID. */
> tsapi TSUuid TSUuidCreate(TSUuidVersion v);
A helpful extension of this might be TSHttpTxnUuidCreate() which would allocate the UUID in the transaction arena.
> tsapi TSReturnCode TSUuidDestroy(TSUuid uuid);
> tsapi TSReturnCode TSUuidCopy(TSUuid dest, TSUuid src);
> tsapi TSReturnCode TSUuidInitialize(TSUuid uuid); /* Re-generate the UUID */
If you TSUuidCreate() but don't TSUuidInitialize(), what do you get?
> tsapi const TSUuid TSProcessUuidGet();
Is this persistent across restarts? As we discussed on IRC, it would be good to expose this value in metrics and traffic_ctl.
> tsapi const char *TSUuidStringGet(const TSUuid uuid);
Does this return an allocated string?
Consider TSUuid TSUuidStringParse() as an additional API.
> and an additional API to expose the already existing HttpSM (Txn) sequence ID:
>
> /* Get the Txn's (HttpSM's) unique identifier, which is a sequence number since server start) */
> tsapi int64_t TSHttpTxnIdGet();
Why int64_t rather than uint64_t?
> The APIs are documented in a separate Sphinx file, i.e. no doxygen crud in the ts.h file :).
The documentation should be posted with the review to save a lot of redundant questions.
> The TSUuidInitialize() functions is maybe a little confusing, what it does is basically re-initialize the UUID object, thus creating a new UUID value. Also, the UUID implementation does not rely on any external dependency, it’s easy enough to implement this ourselves.
>
> I’m definitely open for suggestions for improvements and better naming conventions, but of course have reasonable arguments. There will be a couple of more Jira’s to complete Acacio’s initial work here, go get log tags as well as use these UUIDs in e.g. HttpSM diagnostic traces.
>
> Cheers,
>
> — leif
>
> P.s
> It does have regression tests:
Nice!
>
> [root@fedora ats]# ./bin/traffic_server -R 1 -r SDK_API_UUID
> traffic_server: using root directory '/opt/ats'
> REGRESSION_TEST initialization begun
> REGRESSION TEST SDK_API_UUID started
> [SDK_API_UUID] TSProcessUuidGet : [TestCase1] <<PASS>> { ok }
> [SDK_API_UUID] TSProcessUuidGet : [TestCase2] <<PASS>> { ok }
> [SDK_API_UUID] TSUuidStringGet : [TestCase1] <<PASS>> { ok }
> [SDK_API_UUID] TSUuidCreate : [TestCase1] <<PASS>> { ok }
> [SDK_API_UUID] TSUuidCopy : [TestCase1] <<PASS>> { ok }
> [SDK_API_UUID] TSUuidCopy : [TestCase2] <<PASS>> { ok }
> [SDK_API_UUID] TSUuidInitialize : [TestCase1] <<PASS>> { ok }
> [SDK_API_UUID] TSUuidInitialize : [TestCase2] <<PASS>> { ok }
> [SDK_API_UUID] TSUuidDestroy : [TestCase1] <<PASS>> { ok }
> REGRESSION_RESULT SDK_API_UUID: PASSED
> REGRESSION_TEST DONE: PASSED
>