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

Re: [PATCH 2/2] read-cache: fix reading of split index

From
Duy Nguyen <pclouds@gmail.com>
Date
Mar 24, 2015, 13:02 UTC
Message-ID
<CACsJy8CYi+hYu8zwOy=m7zZk3-8fr+Jq9uT4kEf8fLCOcjHJzw@mail.gmail.com>
In-Reply-To
<1426887794-9655-3-git-send-email-t.gummerer@gmail.com>
On Sat, Mar 21, 2015 at 4:43 AM, Thomas Gummerer <t.gummerer@gmail.com> wrote:
Show 12 quoted lines
> The split index extension uses ewah bitmaps to mark index entries as
> deleted, instead of removing them from the index directly.  This can
> result in an on-disk index, in which entries of stage #0 and higher
> stages appear, which are removed later when the index bases are merged.
>
> 15999d0 read_index_from(): catch out of order entries when reading an
> index file introduces a check which checks if the entries are in order
> after each index entry is read in do_read_index.  This check may however
> fail when a split index is read.
>
> Fix this by moving checking the index after we know there is no split
> index or after the split index bases are successfully merged instead.

Thank you for catching this. I was about to write "would be nice to point out what tests fail so the reviewer has easier time trying themselves", but whoa.. quite a few of them!

May I suggest a slight modification. Even though stage info is messed up before the index is merged, I think we should still check that both front and base indexes have all the names sorted correctly (and even stronger, the base index should never contain staged entries). If sorting order is messed up, it could lead to other problems. So instead of removing the test from do_read_index(), perhaps add a flag in check_ce_order() to optionally detect the stage problem, but print/do nothing, only set a flag so the caller know there may be a problem. In the two new call sites you added, we still call the new check_ce_order() again to make sure everything is in order. In the call site where split index is not active, if the previous check_ce_order() call from inside do_read_index() says "everything is ok", we could even skip the check.

-- 
Duy
Previous: Thomas GummererNext: Thomas Gummerer
Message 13 of 18 in “t1700: make test pass with index-v4”
  1. t1700: make test pass with index-v4Thomas Gummerer, Mar 20, 2015
  2. Junio C HamanoMar 20, 2015
  3. Thomas GummererMar 20, 2015
  4. Junio C HamanoMar 20, 2015
  5. t1700: make test pass with index-v4Thomas Gummerer, Mar 20, 2015
  6. Junio C HamanoMar 20, 2015
  7. Thomas GummererMar 20, 2015
  8. 0/2 fix test suite with split indexThomas Gummerer, Mar 20, 2015
  9. 1/2 test-lib: allow using split index in the test suiteThomas Gummerer, Mar 20, 2015
  10. Junio C HamanoMar 20, 2015
  11. Thomas GummererMar 20, 2015
  12. 2/2 read-cache: fix reading of split indexThomas Gummerer, Mar 20, 2015
  13. Duy NguyenMar 24, 2015
  14. read-cache: tighten checks for do_read_indexThomas Gummerer, Mar 24, 2015
  15. Junio C HamanoMar 24, 2015
  16. Thomas GummererMar 24, 2015
  17. Thomas GummererMar 24, 2015
  18. Junio C HamanoMar 24, 2015

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.