patchdoc/reftable: document how to handle windows

From: Han-Wen Nienhuys <hanwen@google.com>

On Windows we can't delete or overwrite files opened by other processes. Here we sketch how to handle this situation.

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
    doc/reftable: document how to handle windows
    
    On Windows we can't delete or overwrite files opened by other processes.
    Here we sketch how to handle this situation.
    
    Signed-off-by: Han-Wen Nienhuys hanwen@google.com

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-951%2Fhanwen%2Fwindows-doc-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-951/hanwen/windows-doc-v1
Pull-Request: https://github.com/git/git/pull/951

 Documentation/technical/reftable.txt | 38 +++++++++++++++++++++-------
 1 file changed, 29 insertions(+), 9 deletions(-)

"Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@gmail.com> 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?

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 fails, readdir for 0000001-00000001-*.ref to find the latest suffix and increment it)?

Likewise.

On Tue, Jan 26, 2021 at 6:49 AM Junio C Hamano <gitster@pobox.com> wrote:

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.

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.

This case is different. Consider the following situation

1-1.ref:
  main=abc123 @ timestamp 1
  master=abc123 @ timestamp 1
2-2.ref:  bla=456def @ timestamp 2
3-3.ref:
  bla delete @ timestamp 3
  master delete @timestamp 3

The result of compacting this together would be a table containing

  main = abc123 @ timestamp 1

but in the previous naming convention, we'd name the resulting table "1-1.ref", which conflicts with the table in our starting situation.

Han-Wen Nienhuys <hanwen@google.com> writes:

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?

On Tue, Jan 26, 2021 at 6:40 PM Junio C Hamano <gitster@pobox.com> wrote:

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 <hanwen@google.com> writes:

OK, as long as two paths of the same ${min}-${max} part would not confuse people, I am perfectly fine.

I agree that such an approach that can get fooled by an existing gap would not achieve anything over the ${random} approach.

Thanks.

From: Han-Wen Nienhuys <hanwen@google.com>

On Windows we can't delete or overwrite files opened by other processes. Here we sketch how to handle this situation.

We propose to use a random element in the filename. It's possible to design an alternate solution based on counters, but that would assign semantics to the filenames that complicates implementation.

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
    doc/reftable: document how to handle windows
    
    On Windows we can't delete or overwrite files opened by other processes.
    Here we sketch how to handle this situation.
    
    Signed-off-by: Han-Wen Nienhuys hanwen@google.com

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-951%2Fhanwen%2Fwindows-doc-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-951/hanwen/windows-doc-v2
Pull-Request: https://github.com/git/git/pull/951

Range-diff vs v1:

 1:  a952bc478f86 ! 1:  e3854f2cc106 doc/reftable: document how to handle windows
     @@ Commit message
          On Windows we can't delete or overwrite files opened by other processes. Here we
          sketch how to handle this situation.
      
     +    We propose to use a random element in the filename. It's possible to design an
     +    alternate solution based on counters, but that would assign semantics to the
     +    filenames that complicates implementation.
     +
          Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
      
       ## Documentation/technical/reftable.txt ##
     -@@ Documentation/technical/reftable.txt: A collection of reftable files are stored in the `$GIT_DIR/reftable/`
     - directory:
     +@@ Documentation/technical/reftable.txt: A repository must set its `$GIT_DIR/config` to configure reftable:
     + Layout
     + ^^^^^^
       
     - ....
     +-A collection of reftable files are stored in the `$GIT_DIR/reftable/`
     +-directory:
     +-
     +-....
      -00000001-00000001.log
      -00000002-00000002.ref
      -00000003-00000003.ref
     -+00000001-00000001-RANDOM1.log
     -+00000002-00000002-RANDOM2.ref
     -+00000003-00000003-RANDOM3.ref
     - ....
     - 
     - where reftable files are named by a unique name such as produced by the
     +-....
     +-
     +-where reftable files are named by a unique name such as produced by the
      -function `${min_update_index}-${max_update_index}.ref`.
     -+function `${min_update_index}-${max_update_index}-${random}.ref`.
     ++A collection of reftable files are stored in the `$GIT_DIR/reftable/` directory.
     ++Their names should have a random element, such that each filename is globally
     ++unique; this helps avoid spurious failures on Windows, where open files cannot
     ++be removed or overwritten. It suggested to use
     ++`${min_update_index}-${max_update_index}-${random}.ref` as a naming convention.
       
       Log-only files use the `.log` extension, while ref-only and mixed ref
       and log files use `.ref`. extension.
     @@ Documentation/technical/reftable.txt: current files, one per line, in order, fro
       ....
       
       Readers must read `$GIT_DIR/reftable/tables.list` to determine which
     -@@ Documentation/technical/reftable.txt: Reftable files not listed in `tables.list` may be new (and about to be
     - added to the stack by the active writer), or ancient and ready to be
     - pruned.
     - 
     -+The random suffix added to table filenames ensures that we never attempt to
     -+overwrite an existing table, which is necessary for this scheme to work on
     -+Windows
     -+
     - Backward compatibility
     - ^^^^^^^^^^^^^^^^^^^^^^
     - 
      @@ Documentation/technical/reftable.txt: new reftable and atomically appending it to the stack:
       3.  Select `update_index` to be most recent file's
       `max_update_index + 1`.

 Documentation/technical/reftable.txt | 42 +++++++++++++++++-----------
 1 file changed, 26 insertions(+), 16 deletions(-)