Re: [PATCH v5 04/14] core.fsyncmethod: batched disk flushes for loose-objects
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 31, 2022, 18:05 UTC
- Message-ID
- <xmqqtube2e1d.fsf@gitster.g>
- In-Reply-To
- <CANQDOdd-G0VHOKWjWQL75jAJ7Az4izB33HKzayqnk-F-nkHj_A@mail.gmail.com>
Neeraj Singh <nksingh85@gmail.com> writes:
Show 8 quoted lines
> To describe the above if it doesn't render correctly, we have a > bulleted list where the batch after the * is bolded. Other instances > of single backtick quoted text just appears as plaintext. The > descriptive "Currently `batch` mode..." paragraph is under the bullet > point and well-indented. > > In HTML the output looks good as well, except that the descriptive > paragraph is in monospace for some reason.
The "except" part admits that it does not render well, no?
What happens if you modify the second and subsequent paragraph after the "+" continuation in the way suggested? Does it make it worse, or does it make it worse?
Show 5 quoted lines
> Keeping fsync separate from packfile should help the reader see that > the two sets of functions access only their respective global state. > If we add another optimization strategy (e.g. appendable pack files), > it would get its own separate state and functions that are independent > of the large-blob packfile and loose-object fsync optimizations.
OK.
Show 23 quoted lines
>> > +void fsync_loose_object_bulk_checkin(int fd, const char *filename)
>> > +{
>> > + /*
>> > + * If we have an active ODB transaction, we issue a call that
>> > + * cleans the filesystem page cache but avoids a hardware flush
>> > + * command. Later on we will issue a single hardware flush
>> > + * before as part of do_batch_fsync.
>> > + */
>> > + if (!bulk_fsync_objdir ||
>> > + git_fsync(fd, FSYNC_WRITEOUT_ONLY) < 0) {
>> > + fsync_or_die(fd, filename);
>> > + }
>> > +}
>>
>> Ah, if we have successfully created the temporary directory, we
>> don't do full fsync but just writeout-only one, so there is no need
>> for the worry I mentioned in the previous paragraph. OK.
>
> There is the possibility that we might create the objdir when calling
> prepare_loose_object_bulk_checkin but somehow fail to write the object
> and yet still make it to end_odb_transaction. In that case, we'd
> issue an extra dummy fsync. I don't think this case is worth extra
> code to track, since it's a single fsync in a weird failure case.Yup.