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

Re: ab/squelch-empty-fsync-traces & hx/unpack-streaming bug (was: What's cooking in git.git (Jul 2022, #04; Wed, 13))

From
HXHan Xin <chiyutianyi@gmail.com>
Date
Jul 16, 2022, 12:23 UTC
Message-ID
<CAO0brD3fdjQNfQaUBJRAHxDc24K00zpBUa62zST0=cZ5uz3vGA@mail.gmail.com>
In-Reply-To
<220715.86bktqzdb8.gmgdl@evledraar.gmail.com>
CC: Johannes Schindelin

On Fri, Jul 15, 2022 at 10:18 PM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:

Show 33 quoted lines
>
> I hadn't had time to look at this until now. There's some interesting
> behavior here.
>
> The code to check the hardware flush was added in aaf81223f48
> (unpack-objects: use stream_loose_object() to unpack large objects,
> 2022-06-11) (that series is now on master).
>
> But as my ab/squelch-empty-fsync-traces notes we always add this to the
> event, so the:
>
>         grep fsync/hardware-flush trace2.txt &&
>
> Is equivalent to:
>
>         true &&
>
> I.e. it's not testing worthwhile at all. The reason you're seeing a
> failure is deu to 412e4caee38 (tests: disable fsync everywhere,
> 2021-10-29), i.e. our tests disable fsync(). What you have queued will
> pass as:
>
>         GIT_TEST_FSYNC=true ./t5351-unpack-large-objects.sh
>
> But I think that would be meaningless, since we'll write out that on
> FSYNC_HARDWARE_FLUSH whether we actually support "bulk" or not. AFAICT
> the way to detect if we support "bulk" at all is to check for
> fsync/writeout-only.
>
> *Except* that we we unconditionally increment the "writeout only"
> counter, even if we don't actually support that "bulk" mode. We're just
> doing a regular fsync().
>
Agree with you.

In fact, since stream_loose_object() only works with objects of type *blob*, the rest objects of *commit* and *tree* will still use write_loose_object(), so "grep fsync/hardware-flush trace2.txt" did not check for the changes in stream_loose_object() at all.

I haven't found any reference cases in the existing tests.
Perhaps, we need more efficient "fsync" test cases?

Thanks. -Han Xin

Previous: Ævar Arnfjörð BjarmasonNext: Elijah Newren
Message 5 of 7 in “What's cooking in git.git (Jul 2022, #04; Wed, 13)”
  1. Junio C HamanoJul 14, 2022
  2. ds/rebase-update-ref (was Re: What's cooking in git.git (Jul 2022, #04; Wed, 13))Derrick Stolee, Jul 14, 2022
  3. Junio C HamanoJul 14, 2022
  4. ab/squelch-empty-fsync-traces & hx/unpack-streaming bug (was: What's cooking in git.git (Jul 2022, #04; Wed, 13))Ævar Arnfjörð Bjarmason, Jul 15, 2022
  5. Han XinJul 16, 2022
  6. en/merge-restore-to-pristine (Was: Re: What's cooking in git.git (Jul 2022, #04; Wed, 13))Elijah Newren, Jul 17, 2022
  7. ZheNing HuJul 17, 2022

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.