git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
	e@80x24.org, tboegi@web.de,
	"Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
Subject: [PATCH v3] t7063: work around FreeBSD's lazy mtime update feature
Date: Wed,  3 Aug 2016 19:45:22 +0200	[thread overview]
Message-ID: <20160803174522.5571-1-pclouds@gmail.com> (raw)
In-Reply-To: <20160803160536.15596-1-pclouds@gmail.com>

Let's start with the commit message of [1] from freebsd.git [2]

    Sync timestamp changes for inodes of special files to disk as late
    as possible (when the inode is reclaimed).  Temporarily only do
    this if option UFS_LAZYMOD configured and softupdates aren't
    enabled.  UFS_LAZYMOD is intentionally left out of
    /sys/conf/options.

    This is mainly to avoid almost useless disk i/o on battery powered
    machines.  It's silly to write to disk (on the next sync or when
    the inode becomes inactive) just because someone hit a key or
    something wrote to the screen or /dev/null.

    PR:             5577 [3]

The short version of that, in the context of t7063, is that when a
directory is updated, its mtime may be updated later, not
immediately. This can be shown with a simple command sequence

    date; sleep 1; touch abc; rm abc; sleep 10; ls -lTd .

One would expect that the date shown in `ls` would be one second from
`date`, but it's 10 seconds later. If we put another `ls -lTd .` in
front of `sleep 10`, then the date of the last `ls` comes as
expected. The first `ls` somehow forces mtime to be updated.

t7063 is really sensitive to directory mtime. When mtime is too "new",
git code suspects racy timestamps and will not trigger the shortcut in
untracked cache, in t7063.24 and eventually be detected in t7063.27

We have two options thanks to this special FreeBSD feature:

1) Stop supporting untracked cache on FreeBSD. Skip t7063 entirely
   when running on FreeBSD

2) Work around this problem (using the same 'ls' trick) and continue
   to support untracked cache on FreeBSD

I initially wanted to go with 1) because I didn't know the exact
nature of this feature and feared that it would make untracked cache
work unreliably, using the cached version when it should not.

Since the behavior of this thing is clearer now. The picture is not
that bad. If this indeed happens often, untracked cache would assume
racy condition more often and _fall back_ to non-untracked cache code
paths. Which means it may be less effective, but it will not show
wrong things.

This patch goes with option 2.

PS. For those who want to look further in FreeBSD source code, this
flag is now called IN_LAZYMOD. I can see it's effective in ext2 and
ufs. zfs is not affected.

[1] 660e6408e6df99a20dacb070c5e7f9739efdf96d
[2] git://github.com/freebsd/freebsd.git
[3] https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=5577

Reported-by: Eric Wong <e@80x24.org>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 v3 differs from v2 in one line

    --- a/t/t7063-status-untracked-cache.sh
    +++ b/t/t7063-status-untracked-cache.sh
    @@ -11,7 +11,7 @@ test_description='test untracked cache'
     # containing directory in sync with the reality after doing blah and
     # before checking the fast path behaviour
     sync_mtime () {
    -	find . -type d -exec ls -ld {} \; >/dev/null
    +	find . -type d -ls >/dev/null
     }
 
 avoid_racy() {
 t/t7063-status-untracked-cache.sh | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/t/t7063-status-untracked-cache.sh b/t/t7063-status-untracked-cache.sh
index a971884..d31d080 100755
--- a/t/t7063-status-untracked-cache.sh
+++ b/t/t7063-status-untracked-cache.sh
@@ -4,6 +4,16 @@ test_description='test untracked cache'
 
 . ./test-lib.sh
 
+# On some filesystems (e.g. FreeBSD's ext2 and ufs) this and that
+# happens when we do blah, which forces the untracked cache code to
+# take the slow path.  A test that wants to make sure the fast path
+# works correctly should call this helper to make mtime of the
+# containing directory in sync with the reality after doing blah and
+# before checking the fast path behaviour
+sync_mtime () {
+	find . -type d -ls >/dev/null
+}
+
 avoid_racy() {
 	sleep 1
 }
@@ -416,7 +426,8 @@ test_expect_success 'create/modify files, some of which are gitignored' '
 	echo four >done/four && # four is gitignored at a higher level
 	echo five >done/five && # five is not gitignored
 	echo test >base && #we need to ensure that the root dir is touched
-	rm base
+	rm base &&
+	sync_mtime
 '
 
 test_expect_success 'test sparse status with untracked cache' '
-- 
2.9.1.566.gbd532d4


  parent reply	other threads:[~2016-08-03 17:46 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-18 22:30 t7063 failure on FreeBSD 10.3 i386/amd64 Eric Wong
2016-07-18 22:54 ` Eric Wong
2016-07-19 16:12   ` Duy Nguyen
2016-07-20  3:02     ` Eric Wong
2016-07-20 14:57       ` Duy Nguyen
2016-07-27 17:33     ` Duy Nguyen
2016-07-30 13:31       ` Duy Nguyen
2016-07-30 13:54         ` Duy Nguyen
2016-07-30 18:20 ` [PATCH] t7063: work around FreeBSD's lazy mtime update feature Nguyễn Thái Ngọc Duy
2016-07-31  0:15   ` Eric Wong
2016-07-31  1:07     ` Eric Wong
2016-07-31 14:30       ` Duy Nguyen
2016-08-01  1:37   ` Torstem Bögershausen
2016-08-01 17:10     ` Duy Nguyen
2016-08-01 21:04       ` Junio C Hamano
2016-08-02 15:37         ` Duy Nguyen
2016-08-02 17:17           ` Junio C Hamano
2016-08-03 16:05   ` [PATCH v2] " Nguyễn Thái Ngọc Duy
2016-08-03 16:16     ` Junio C Hamano
2016-08-03 16:25       ` Duy Nguyen
2016-08-03 17:45     ` Nguyễn Thái Ngọc Duy [this message]
2016-08-03 17:52       ` [PATCH v3] " Junio C Hamano
2016-08-03 19:07         ` Junio C Hamano
2016-08-04 15:46           ` Duy Nguyen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160803174522.5571-1-pclouds@gmail.com \
    --to=pclouds@gmail.com \
    --cc=e@80x24.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=tboegi@web.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).