Re: [PATCH v3 1/1] trace2: write to directory targets

[Junio C Hamano <gitster@pobox.com>]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> On Sun, Mar 24 2019, Junio C Hamano wrote:
>
>> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>>
>>> The reason I'm raising this is that it seems like sweeping an existing
>>> issue under the rug. We document that the "sid" is "unique", and it's just:
>>>
>>>     <nanotime / 1000 (i.e. *nix time in microseconds)>-<pid>
>>
>> If it is just that, then it cannot be unique, can it?
>>
>> Let's just fix the wrong doc and move on.
>
> I don't see why we wouldn't just fix the SID generation & move on if
> we're already carrying code purely as a work-around for it not being
> unique enough.

The thing is, the yardstick to determine "unique enough" depends on
the caller.  In this codepath, we want the uniqueness within the
directory that was given to us, and one reasonable way, among the
most sensible ones, is to ask open(O_CREAT|O_EXCL) and it makes 100%
sense to fall back with suffix when "enough" thought by the callee
turns out to be insufficient when judged by the caller.

So I do not see the .%d suffix a work-around at all.  I view it as
an integral part of the whole package.

By the way, is the "nanotime" implementation that may be in compat/
fine grained enough?

> Of course nothing is *guaranteed* to be unique, not even a 128-bit
> UUID. The point is to pick something that's "unique enough" given the
> problem space, which is already one where we'll ignore I/O errors on
> writing the file unless you opt-in to a warning.

Yes, the point is to pick something that is unique enough and then
give a reasonable fallback when it turns out insufficient.  I think
".%d" suffix is one reasonable fallback, but I realize that it is
not the only reasonable fallback.  Another reasonable fallback could
be "upon seeing a failure of open(O_CREAT|O_EXCL), we give up and do
not leave a logfile, because this should be a rare enough event as
our assumption is sid is unique enough for everyday operation".

I could buy that, especially if the ".%d" suffix fallback is too
expensive to carry and maintain into the future.  And in such a
case, it indeed would be a more reasonable workaround for a rare
non-unique sid problem to ignore and discard the log.

I just did not think the ".%d" suffix fallback is too expensive to
carry.