On Tue, Jan 26, 2021 at 6:40 PM Junio C Hamano <gitster@xxxxxxxxx> wrote:
>
> Han-Wen Nienhuys <hanwen@xxxxxxxxxx> writes:
>
> >> Is this because we have been assuming that in step 5. we can
> >> "overwrite" (i.e. take over the name, implicitly unlinking the
> >> existing one) the existing 0000001-00000001.ref with the newly
> >> prepared one, which is not doable on Windows?
> >
> > No, the protocol for adding a table to the end of the stack is
> > impervious to problems on Windows, as everything happens under lock,
> > so there is no possibility of collisions.
> >
> >> We must prepare for two "randoms" colliding and retrying the
> >> renaming step anyway, so would it make more sense to instead
> >> use a non-random suffix (i.e. try "-0.ref" first, and when it we
> >> fails, readdir for 0000001-00000001-*.ref to find the latest
> >> suffix and increment it)?
> >
> > This is a lot of complexity, and both transactions and compactions can
> > always fail because they fail to get the lock, or because the data to
> > be written is out of date. So callers need to be prepared for a retry
> > anyway.
>
> Sorry, are we saying the same thing and reaching different
> conclusions?
>
> My question was, under the assumption that the callers need to be
> prepared for a retry anyway,
>
> (1) would it be possible to use "seq" (or "take max from existing
> and add one") as the random number generator for the ${random}
> part of your document, and
>
> (2) if the answer to the previous question is yes, would it result
> in a system that is easier for Git developers, who observe what
> happens inside the .git directory, to understand the behaviour
> of the system, as they can immediately see that 1-1-47 is newer
> than 1-1-22 instead of 1-1-$random1 and 1-1-$random2 that
> cannot be compared?
The first two parts of the file name (${min}-${max}) already provide
visibility into what is going on, and the file system timestamp
already indicates which file is newer. I picked a random name as
suffix, as it gets the job done and is simple.
We could do what you suggest, but it adds semantics to the filenames
that aren't really there: currently, tables.list is a list of
filenames, and no part of the code parses back the file names. If we'd
do what you suggest, we have more ways in which the system can break
subtly, and needs to handle error conditions if the names are
malformed. This is the complexity I was alluding to in my previous
message.
We could stipulate that a compaction must always increase the logical
timestamp, ie. in the scenario I sketched, the compacted table should
be written with a max-timestamp of 3, even though it contains no
entries at timestamp 3. This avoids the error condition, but it's
also surprising because it is actually inconsistent with how the
format is described. But maybe we could update the description of the
format.
Or, we could rename to ${min}-${max}-0 and if that fails try
${min}-${max}-1, and if that fails ${min}-${max}-2 etc. I think that
is somewhat nicer than parsing back a counter from the existing
filenames, but it could have the effect that 1-1-0 could be newer than
1-1-2.
--
Han-Wen Nienhuys - Google Munich
I work 80%. Don't expect answers from me on Fridays.
--
Google Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
