From: Karthik Nayak Date: Fri, 30 Oct 2026 10:38:37 GMT Subject: Re: [PATCH v3 0/4] loose: fix migrating "loose-object-idx" with batch fsync Message-ID: In-Reply-To: <20261029232220.1245827-1-jltobler@gmail.com> Justin Tobler writes: > Greetings, > > While working on a separate patch series to convert the "files" ODB > transaction to be eagerly created instead of lazily, I encountered this > bug that causes an ODB transaction to fail on commit when a > compatibility hash is also configured. This occurs because the temporary > directory migration code does not know how to handle the loose object > index file. Okay so reading a bit, and paraphrasing for my understanding: When we hae object compatibility setup in a repository. THe 'loose-object-idx' tracks the mapping from hash1<>hash2. The issue is batch fsync, where we use a temporary folder. I'm not sure what the 'error' here is, but I guess we'll see with the patches. For pack files this is a non-issues as they're content addressed and individual packfiles have their own corresponding idx files. > This patch addresses updating the migration logic to explicitly merge > mapping entries in the transaction and atomically write a new > "loose-object-idx" file. > This means we write a 'loose-object-idx' to the temporary directory and we need to merge it with the one in the main directory. Which seems to make sense. > Some other approaches I considered: > - Storing the uncommitted object mapping entries in the transaction > without writing a file and only writing on transaction commit. I > chose to keep writing the temp index into the transaction so that > child processes that write to the tempdir could still append entries. So the child processes write the objects, so they write to the index too I presume. If not, we'd have to somehow get that information from all children to the parent which owns the transaction. That gets complicated / messy. Seems right. > - Also writing mapping entires to the main ODB loose object index. > This also didn't seem like a good apprach because it write entries > that may not ultimately get committed. > Yeah this approach definitely doesn't make sense from a transaction point of view. [snip] Will dig into the patches...