On Mon, Mar 25 2019, Junio C Hamano wrote: > Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes: > >> On Sun, Mar 24 2019, Junio C Hamano wrote: >> >>> Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> 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.
