From: Jeff Hostetler <git@jeffhostetler.com>
Date: Tue, 13 Jul 2021 15:15:08 GMT
Subject: Re: [PATCH v3 17/34] fsmonitor--daemon: define token-ids
Message-ID: <f38ed916-cfd8-06b2-43dc-689720a96a6e@jeffhostetler.com>
In-Reply-To: <87mtr5bpuw.fsf@evledraar.gmail.com>



On 7/1/21 6:58 PM, Ævar Arnfjörð Bjarmason wrote:
> 
> On Thu, Jul 01 2021, Jeff Hostetler via GitGitGadget wrote:
> 
>> +	if (!test_env_value) {
>> +		struct timeval tv;
>> +		struct tm tm;
>> +		time_t secs;
>> +
>> +		gettimeofday(&tv, NULL);
>> +		secs = tv.tv_sec;
>> +		gmtime_r(&secs, &tm);
>> +
>> +		strbuf_addf(&token->token_id,
>> +			    "%"PRIu64".%d.%4d%02d%02dT%02d%02d%02d.%06ldZ",
>> +			    flush_count++,
>> +			    getpid(),
>> +			    tm.tm_year + 1900, tm.tm_mon + 1, tm.tm_mday,
>> +			    tm.tm_hour, tm.tm_min, tm.tm_sec,
>> +			    (long)tv.tv_usec);
> 
> Just bikeshedding, but can we have tokens that mostly sort numeric-wise
> by time order? So time at the start, not the flush_count/getpid.

As I described in a rather large comment in the code, tokens are opaque
strings -- without a less-than / greater-than relationship -- just a
random string that the daemon can use (along with a sequence number) to
ensure that a later request is well-defined.

Here I'm using a counter, pid, and date-stamp.  I'd prefer using a GUID
or UUID just to drive that home, but I didn't want to add a new .lib or
.a to the build if not necessary.

Perhaps I should compute this portion as hex(hash(time())) to remove the
temptation to look inside my opaque token ??

> 
> Maybe I'm missing something, but couldn't we just re-use the trace2 SID
> + a more trivial trailer? It would have the nice property that you could
> find the trace2 SID whenever you looked at such a token (could
> e.g. split them by "/" too), and add the tv_usec, flush_count+whatever
> else is needed to make it unique after the "/", no?
> 

I would rather keep Trace2 out of this.  The SID is another opaque
string and I don't want to reach inside it.

Jeff