git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Mar 25, 2019, 08:21 UTC
Message-ID
<874l7rcqk9.fsf@evledraar.gmail.com>
In-Reply-To
<xmqqlg13ofqw.fsf@gitster-ct.c.googlers.com>
On Mon, Mar 25 2019, Junio C Hamano wrote:
Show 52 quoted lines
> Æ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.

Oh yeah, the SID.%d fallback is easy enough, and I'd 100% agree that if that was *all* it was this would all make sense. Let's just retry, just like a tempfile implementation will retry.

But I'm not interested in using the SID-per-file format Josh is adding here, but *am* interested in having a logging system where over a bunch of machines I can expect the SID to be unique with a high degree of probability.

So this open(O_CREAT|O_EXCL) code is revealing a problem that we're also going to have in a different form in the GIT_TR2_EVENT=/var/log/one-big-file-of.json format.

It seems to me that the only reason both of these are an issue is something we can fix with the SID generation.

Previous: Junio C HamanoNext: Jeff Hostetler
Message 31 of 34 in “Randomize / timestamp trace2 targets”
  1. 0/2 Randomize / timestamp trace2 targetsJosh Steadmon, Mar 13, 2019
  2. 1/2 date: make get_time() publicJosh Steadmon, Mar 13, 2019
  3. 2/2 trace2: randomize/timestamp trace2 targetsJosh Steadmon, Mar 13, 2019
  4. Ævar Arnfjörð BjarmasonMar 13, 2019
  5. Jeff HostetlerMar 15, 2019
  6. Ævar Arnfjörð BjarmasonMar 15, 2019
  7. Jeff HostetlerMar 15, 2019
  8. Josh SteadmonMar 15, 2019
  9. Josh SteadmonMar 15, 2019
  10. Junio C HamanoMar 18, 2019
  11. Jeff KingMar 19, 2019
  12. Jeff KingMar 14, 2019
  13. Junio C HamanoMar 14, 2019
  14. Johannes SchindelinMar 14, 2019
  15. Josh SteadmonMar 15, 2019
  16. Jeff HostetlerMar 15, 2019
  17. Josh SteadmonMar 15, 2019
  18. Jeff HostetlerMar 18, 2019
  19. 0/1 Write trace2 output to directoriesJosh Steadmon, Mar 21, 2019
  20. 1/1 trace2: write to directory targetsJosh Steadmon, Mar 21, 2019
  21. Junio C HamanoMar 21, 2019
  22. Jeff HostetlerMar 21, 2019
  23. Junio C HamanoMar 22, 2019
  24. Jeff HostetlerMar 22, 2019
  25. 0/1 Write trace2 output to directoriesJosh Steadmon, Mar 21, 2019
  26. 1/1 trace2: write to directory targetsJosh Steadmon, Mar 21, 2019
  27. Ævar Arnfjörð BjarmasonMar 23, 2019
  28. Junio C HamanoMar 24, 2019
  29. Ævar Arnfjörð BjarmasonMar 24, 2019
  30. Junio C HamanoMar 25, 2019
  31. Ævar Arnfjörð BjarmasonMar 25, 2019
  32. Jeff HostetlerMar 25, 2019
  33. Jeff HostetlerMar 21, 2019
  34. Junio C HamanoMar 22, 2019

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.