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

Re: [PATCH v5 25/27] refs: add LMDB refs storage backend

From
David Turner <dturner@twopensource.com>
Date
Feb 24, 2016, 20:37 UTC
Message-ID
<1456346242.18017.16.camel@twopensource.com>
In-Reply-To
<CACsJy8BQZCBpfmvXk+o5PqM7=zad7cxgA9B2995Rb0D0YBxEVw@mail.gmail.com>
On Sat, 2016-02-20 at 15:59 +0700, Duy Nguyen wrote:
Show 16 quoted lines
> On Thu, Feb 18, 2016 at 12:17 PM, David Turner <
> dturner@twopensource.com> wrote:
> > LMDB has a few features that make it suitable for usage in git:
> > ...
> 
> I'm reading lmdb documents and hitting  the caveat section [1].
> Random thoughts
> 
> * "There is normally no pure read-only mode, since readers need write
> access to locks and lock file.".
> 
> This will be a problem for server side that serves git:// protocol
> only. Some of those servers disable write access to the entire
> repository and git still works fine (but won't when lmdb is used).
> Should we do something in this case? Just tell server admins to relax
> file access, or use MDB_NOLOCK (and when? based on config var?)

MDB_NOLOCK is a good idea. I'm going to add this to the "Weaknesses" section of the docs and plan to fix it later, unless you feel strongly otherwise.

Show 8 quoted lines
> *  " Use an MDB_env* in the process which opened it, without
> fork()ing.". We do use fork on non-Windows in run-command.c, but it
> should be followed by exec() with no ref access in between, so we're
> almost good.
> 
> I notice atexit() is used in this for/exec code, which reminds me we
> also use atexit() in many other places. I hope none of them access
> refs, or we could be in trouble.

I don't think they should do that anyway -- it's too much complexity for an atexit handler.

Show 9 quoted lines
> * "Do not have open an LMDB database twice in the same process at the
> same time. Not even from a plain open() call - close()ing it breaks
> flock() advisory locking"
> 
> I wonder what happens if we do open twice, will it error out or
> silently ignore and move on? Because if it's the latter case, we need
> some protection from the caller and I'm not sure if
> lmdb-backend.c:init_env() has it, especially when it open submodule's
> lmdb.

I think LMDB could check this, but it doesn't seem to. I've refactored some of the submodule transaction stuff so that now we have more protection there.

Show 8 quoted lines
> * "Avoid long-lived transactions...."
> 
> OK we don't have a problem with this. But it makes me realize lmdb
> transactions do not map with ref transactions. We don't open lmdb
> transaction at ref_transaction_begin(), for example. Is it simply
> more
> convenient to do transactions the current way, or is it impossible or
> incorrect to attach lmdb transactions to ref_transaction_*()?
That was what I did originally, but reviewers here noted that it had
some problems:
1. What if, while a transaction is open, git opens a subprocess that
wants to make its own transaction?  There can only be one writer
transaction open at a time.
2. As you note, long-lived transactions.

Also, the files backend also doesn't do any locking until the last moment, and it's reasonable to try to be compatible with that.

Show 7 quoted lines
> * "Avoid aborting a process with an active transaction. The
> transaction becomes "long-lived" as above until a check for stale
> readers is performed or the lockfile is reset, since the process may
> not remove it from the lockfile."
> 
> Does it mean we should at atexit() and signal handler to release
> currently active transaction?
Probably a good idea.
Show 9 quoted lines
> * "Do not use LMDB databases on remote filesystems, even between
> processes on the same host. This breaks flock() on some OSes,
> possibly
> memory map sync, and certainly sync between programs on different
> hosts."
> 
> OK can't do anything about it anyway, but maybe it should be
> mentioned
> somewhere in Git documentation.
Sure.
Show 7 quoted lines
> * "Opening a database can fail if another process is opening or
> closing it at exactly the same time."
> 
> We have some retry logic in resolve_ref_1(). Do we need the same for
> lmdb? Not very important though.
> 
> [1] http://symas.com/mdb/doc/
wat.

OK, I'll add a retry loop on that. I guess we can just keep retrying on EACCES or EAGAIN.

Previous: Duy NguyenNext: Duy Nguyen
Message 45 of 52 in “refs backends”
  1. 00/27 refs backendsDavid Turner, Feb 18, 2016
  2. 01/27 refs: Move head_ref{,_submodule} to the common codeDavid Turner, Feb 18, 2016
  3. 02/27 refs: move for_each_*ref* functions into common codeDavid Turner, Feb 18, 2016
  4. 03/27 refs: add a backend method structure with transaction functionsDavid Turner, Feb 18, 2016
  5. 04/27 refs: add methods for misc ref operationsDavid Turner, Feb 18, 2016
  6. 05/27 refs: add method for do_for_each_refDavid Turner, Feb 18, 2016
  7. 06/27 refs: add do_for_each_per_worktree_refDavid Turner, Feb 18, 2016
  8. 07/27 refs: add methods for reflogDavid Turner, Feb 18, 2016
  9. 08/27 refs: add method for initial ref transaction commitDavid Turner, Feb 18, 2016
  10. 09/27 refs: add method for delete_refsDavid Turner, Feb 18, 2016
  11. 10/27 refs: add methods to init refs dbDavid Turner, Feb 18, 2016
  12. 11/27 refs: add method to rename refsDavid Turner, Feb 18, 2016
  13. 12/27 refs: forbid cross-backend ref renamesDavid Turner, Feb 18, 2016
  14. Duy NguyenFeb 20, 2016
  15. David TurnerFeb 24, 2016
  16. 13/27 refs: make lock genericDavid Turner, Feb 18, 2016
  17. 14/27 refs: move duplicate check to common codeDavid Turner, Feb 18, 2016
  18. 15/27 refs: allow log-only updatesDavid Turner, Feb 18, 2016
  19. 16/27 refs: don't dereference on renameDavid Turner, Feb 18, 2016
  20. 17/27 refs: on symref reflog expire, lock symref not referrentDavid Turner, Feb 18, 2016
  21. 18/27 refs: resolve symbolic refs firstDavid Turner, Feb 18, 2016
  22. 19/27 refs: always handle non-normal refs in files backendDavid Turner, Feb 18, 2016
  23. 20/27 init: allow alternate ref strorage to be set for new reposDavid Turner, Feb 18, 2016
  24. 21/27 refs: check submodules' ref storage configDavid Turner, Feb 18, 2016
  25. 22/27 clone: allow ref storage backend to be set for cloneDavid Turner, Feb 18, 2016
  26. 23/27 svn: learn ref-storage argumentDavid Turner, Feb 18, 2016
  27. Eric WongFeb 20, 2016
  28. David TurnerFeb 23, 2016
  29. 24/27 refs: add register_ref_storage_backends()David Turner, Feb 18, 2016
  30. 25/27 refs: add LMDB refs storage backendDavid Turner, Feb 18, 2016
  31. Duy NguyenFeb 18, 2016
  32. David TurnerFeb 18, 2016
  33. Junio C HamanoFeb 18, 2016
  34. Duy NguyenFeb 19, 2016
  35. David TurnerFeb 19, 2016
  36. Duy NguyenFeb 20, 2016
  37. David TurnerFeb 24, 2016
  38. Junio C HamanoFeb 20, 2016
  39. David TurnerFeb 19, 2016
  40. Junio C HamanoFeb 19, 2016
  41. Duy NguyenFeb 20, 2016
  42. David TurnerFeb 24, 2016
  43. Duy NguyenFeb 25, 2016
  44. Duy NguyenFeb 20, 2016
  45. David TurnerFeb 24, 2016
  46. Duy NguyenFeb 25, 2016
  47. refs: document transaction semanticsDavid Turner, Feb 25, 2016
  48. David TurnerFeb 25, 2016
  49. Junio C HamanoFeb 25, 2016
  50. David TurnerFeb 25, 2016
  51. 26/27 refs: tests for lmdb backendDavid Turner, Feb 18, 2016
  52. 27/27 tests: add ref-storage argumentDavid Turner, Feb 18, 2016

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.