Skip to content

meta: fix dir stats after cloning a directory - #7746

Open
angushe wants to merge 1 commit into
juicedata:mainfrom
angushe:clone-dir-stats
Open

angushe wants to merge 1 commit into
juicedata:mainfrom
angushe:clone-dir-stats

Conversation

@angushe

@angushe angushe commented Oct 23, 2026 •

Copy link
Copy Markdown

Fixes #7744

This PR fixes two dir stat errors of Clone, and makes the flush that the first fix uses wait for a flush in progress:

  • The cloned directories copy the dir stats of the source directories from the metadata engine. Thus, the clone did not include the changes that this client did not flush yet. The clone kept the incorrect values. Now, before Clone clones a directory, it flushes the pending dir stats of this client.

  • Clone added the Length of the source to the dir stat of the parent. This value is 4096 for a directory, and the length of the target path for a symlink. But the length in the dir stats counts only regular files: Mknod adds 0, and Rename moves the length only for regular files. Now, Clone adds the length only for a regular file.

  • A flush that starts while another flush writes the stats finds no pending changes, and returns at once, before that write. Then the clone can still copy the old values. Now, doFlushDirStat holds a new lock, dirStatsFlushLock, until it has written the stats, as doFlushQuotas does with quotasFlushLock (meta:Add mutex to doFlushQuotas to prevent concurrent execution #7080). Thus, the flush in Clone waits for a flush in progress.

The change is in pkg/meta/base.go and pkg/meta/quota.go only, so all engines get it.

Cost: only the callers of doFlushDirStat take the new lock: the background flush (one time each second), FlushSession and Clone. updateDirStat, which each create, write and unlink calls, does not take it. We compared this PR with and without the lock:

  • The upstream benchmarks of the hot path (mkdir, create, unlink, rename, rmdir, mknod and write, on Redis, SQLite and Badger) show no difference.
  • For BenchmarkRedis/write, we ran fixed test binaries with 3000 iterations, 10 times each, in random order. The medians with and without the lock differ by 0.9%; two copies of the same binary differ by 0.5%.
  • Clone of a tree with 1111 directories takes the same time.

The flush does not include the changes that are pending in other clients. A full fix calculates the stats of each cloned directory from the cloned entries, and does not copy them. This fix changes all three engines. If you prefer the full fix, I can do it.

Test: this PR adds testCloneDirStats to testMeta. The test does these steps:

  1. It sets DirStatFlushPeriod to one hour for its session.
  2. It truncates a file to 2 MiB immediately before the clone. Thus, this change is pending at the time of the clone.
  3. It compares the dir stats of the clone with the dir stats of the source.
  4. It checks the dir stat of the parent.
  5. It truncates the file to 3 MiB. Then it acts as a flush in progress: it takes dirStatsFlushLock and the pending dir stats, and does not write them yet.
  6. It clones the directory again in a goroutine, waits up to 500 ms, writes the taken dir stats, and releases the lock. Then it compares the dir stats of the second clone with the dir stats of the source.

The results:

  • Without the fixes, the test fails at step 3 on SQLite, Redis and MemKV. With only the flush fix, the test fails at step 4 on SQLite and Redis.
  • Without the lock, the test fails at step 6 on SQLite, Redis and MemKV. The second clone has 2 MiB, and the source has 3 MiB. With the lock, the clone waits, and the test passes.
  • With this PR, the test passes. The full TestSQLiteClient, TestMemKVClient and TestRedisClient suites also pass. We also ran pkg/meta with SKIP_NON_CORE=true, without the tests that need MySQL, PostgreSQL, TiKV or etcd: 76 tests pass and 6 skip, the same as without this PR. pkg/vfs and pkg/fs pass too.
  • With -race, TestSQLiteClient, TestBadgerClient and TestRedisClient pass. TestMemKVClient reports a race in testClone in 2 of 10 runs, with and without this PR. The DeleteSlice callback writes sli1del, and the test reads it after time.Sleep.
  • With this PR, the command-line steps in Directory clone sets incorrect dir stats on the clone and on its parent #7744 show the correct values. fsck shows no warning, the clone has length 2097152, the parent has length 0, and the quota on the clone is consistent.

This PR does not need #7745. But #7745 changes the lines next to this change at the end of Clone, and it adds its test at the same location. Thus, the PR that you merge second needs a rebase. I will do the rebase.

@angushe
angushe requested a review from jiefenghuang as a code owner October 23, 2026 04:55
Two dir stat errors date from the clone refactor in juicedata#3397:

- The cloned directories copy the dir stats of the source directories from
  the metadata engine, so changes still pending in this client (flushed
  once per second) were not copied, and existing dir stats are not
  recomputed, so the clone kept the stale stats; `juicefs fsck
  --sync-dir-stat` reported the clone. Clone now flushes the pending dir
  stats of this client before cloning a directory. Changes pending in other
  clients can still be missed; computing the stats of each cloned directory
  from the cloned entries would fix that too, but touches all engines.
- Clone added the Length of the source to the parent's dir stat, which is
  4096 for a directory and the length of the target path for a symlink,
  while the length in dir stats (juicedata#3302) counts regular files only (Mknod
  adds 0, Rename moves the length of regular files only).

For the flush in Clone to see a change, doFlushDirStat now holds a new
lock until the stats are written, as doFlushQuotas does (juicedata#7080): before, a
flush that started while another one was writing found nothing to flush and
returned at once, before that write. Only the callers of doFlushDirStat
take the lock (the background flush, FlushSession and Clone), not
updateDirStat.

Add testCloneDirStats to testMeta.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Directory clone sets incorrect dir stats on the clone and on its parent

1 participant