New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Have VFS graphdriver use accelerated in-kernel copy #35537

Merged
merged 4 commits into from Dec 5, 2017

Conversation

Projects
None yet
8 participants
@sargun
Contributor

sargun commented Nov 17, 2017

- What I did
This change makes the VFS graphdriver use the kernel-accelerated
(copy_file_range) mechanism of copying files, which is able to
leverage reflinks.

The copy package should fulfill all of the capabilities that CopyWithTar
fulfilled as well.

I learned a lot about file system performance on Linux. At least about XFS on machines with reasonably fast I/O, and many cores with lots of memory. The primary barrier here is not disk I/O, as it turns out, but contention inside of the dentry, inode, and allocation group.
- How I did it
I reused the accelerated copy code I had written for the overlay graphdriver.

- How to verify it
I used the existing tests to verify the behaviour.

- Description for the changelog
Use copy_file_range to copy files for VFS on Linux.

@sargun sargun requested a review from dmcgowan as a code owner Nov 17, 2017

Show outdated Hide outdated daemon/graphdriver/vfs/copy_linux.go Outdated
// CopyWithTar defines the copy method to use.
CopyWithTar = chrootarchive.NewArchiver(nil).CopyWithTar
// CopyDir defines the copy method to use.
CopyDir = dirCopy

This comment has been minimized.

@cpuguy83

cpuguy83 Nov 17, 2017

Contributor

No need for this one, the functions can call copyDir directly.

@cpuguy83

cpuguy83 Nov 17, 2017

Contributor

No need for this one, the functions can call copyDir directly.

This comment has been minimized.

@cpuguy83

cpuguy83 Nov 17, 2017

Contributor

Oh nevermind I see the layerstore test is using this... something about nice things...

@cpuguy83

cpuguy83 Nov 17, 2017

Contributor

Oh nevermind I see the layerstore test is using this... something about nice things...

This comment has been minimized.

@sargun

sargun Nov 17, 2017

Contributor

👍

@sargun

sargun Nov 17, 2017

Contributor

👍

This comment has been minimized.

@sargun

sargun Nov 17, 2017

Contributor

@cpuguy83 Anything else then?

@sargun

sargun Nov 17, 2017

Contributor

@cpuguy83 Anything else then?

This comment has been minimized.

@dmcgowan

dmcgowan Nov 22, 2017

Member

Is chrootarchive really necessary now, without it then there would be no need to expose this, especially since Linux no longer needs it...

@dmcgowan

dmcgowan Nov 22, 2017

Member

Is chrootarchive really necessary now, without it then there would be no need to expose this, especially since Linux no longer needs it...

This comment has been minimized.

@sargun

sargun Nov 27, 2017

Contributor

What would we do with the non-windows bits? I can do it, but I'd not want to do it in this PR.

@sargun

sargun Nov 27, 2017

Contributor

What would we do with the non-windows bits? I can do it, but I'd not want to do it in this PR.

@cpuguy83

This comment has been minimized.

Show comment
Hide comment
@cpuguy83

cpuguy83 Nov 17, 2017

Contributor

Looks like experimental got stuck in a syscall:

goroutine 4277 [runnable]:
syscall.Syscall(0x4a, 0x13, 0x0, 0x0, 0x0, 0x0, 0x0)
	/usr/local/go/src/syscall/asm_linux_amd64.s:18 +0x5
syscall.Fsync(0x13, 0x10, 0x96cdb8)
	/usr/local/go/src/syscall/zsyscall_linux_amd64.go:492 +0x4a
os.(*File).Sync(0xc4216e04f0, 0x1e53018, 0xc4217ae148)
	/usr/local/go/src/os/file_posix.go:121 +0x5b
github.com/docker/docker/pkg/ioutils.(*atomicFileWriter).Close(0xc421bcef90, 0x0, 0x0)
	/go/src/github.com/docker/docker/pkg/ioutils/fswriters.go:68 +0x81
github.com/docker/docker/pkg/ioutils.AtomicWriteFile(0xc42134cb00, 0x71, 0xc421090000, 0xe57, 0xe57, 0xc400000180, 0xc420f24990, 0x2b899b0)
	/go/src/github.com/docker/docker/pkg/ioutils/fswriters.go:41 +0xcb
github.com/docker/docker/image.(*fs).Set(0xc420477e30, 0xc421090000, 0xe57, 0xe57, 0x0, 0x0, 0x0, 0x0)
	/go/src/github.com/docker/docker/image/fs.go:121 +0x122
github.com/docker/docker/image.(*store).Create(0xc42022f1a0, 0xc421090000, 0xe57, 0xe57, 0x0, 0x0, 0x0, 0x0)
	/go/src/github.com/docker/docker/image/store.go:146 +0x2c8
github.com/docker/docker/daemon.(*Daemon).Commit(0xc4203c3200, 0xc421c28580, 0x40, 0xc421b39d80, 0x0, 0x0, 0x0, 0x0)
	/go/src/github.com/docker/docker/daemon/commit.go:207 +0x696
github.com/docker/docker/builder/dockerfile.(*Builder).commitContainer(0xc4203eb4a0, 0xc4213c66c0, 0xc421c28580, 0x40, 0xc42123cc80, 0x2b9cba0, 0xc4217ddf60)
	/go/src/github.com/docker/docker/builder/dockerfile/internals.go:117 +0x117
github.com/docker/docker/builder/dockerfile.dispatchRun(0xc4213c66c0, 0xc420addd04, 0xc4203eb4a0, 0x2bafae0, 0xc421af1260, 0xc420feafc0, 0xc42048ff00, 0xc420feafc0, 0xc4217aed00)
	/go/src/github.com/docker/docker/builder/dockerfile/dispatchers.go:356 +0xa7e
github.com/docker/docker/builder/dockerfile.dispatch(0xc4213c66c0, 0xc420addd04, 0xc4203eb4a0, 0x2bafae0, 0xc421af1260, 0xc420feafc0, 0x7fa8b459a008, 0xc42048ff00, 0x0, 0x0)
	/go/src/github.com/docker/docker/builder/dockerfile/evaluator.go:81 +0x457
github.com/docker/docker/builder/dockerfile.(*Builder).dispatchDockerfileWithCancellation(0xc4203eb4a0, 0xc421a77d10, 0x1, 0x1, 0x0, 0x0, 0x0, 0x5c, 0x2bafae0, 0xc421af1260, ...)
	/go/src/github.com/docker/docker/builder/dockerfile/builder.go:322 +0x940
github.com/docker/docker/builder/dockerfile.(*Builder).build(0xc4203eb4a0, 0x2bafae0, 0xc421af1260, 0xc42048f240, 0x0, 0x0, 0x0)
	/go/src/github.com/docker/docker/builder/dockerfile/builder.go:240 +0x3bb
github.com/docker/docker/builder/dockerfile.(*BuildManager).Build(0xc4203b87c0, 0x7fa8b679bba0, 0xc42048f500, 0x2bacf20, 0xc420107280, 0x2b9bd60, 0xc4217ddf40, 0x2b9cba0, 0xc4217ddf60, 0x2b9cba0, ...)
	/go/src/github.com/docker/docker/builder/dockerfile/builder.go:125 +0x4aa
github.com/docker/docker/api/server/backend/build.(*Backend).Build(0xc42085eae0, 0x7fa8b679bba0, 0xc420107380, 0x2bacf20, 0xc420107280, 0x2b9bd60, 0xc4217ddf40, 0x2b9cba0, 0xc4217ddf60, 0x2b9cba0, ...)

Restarting it.

Contributor

cpuguy83 commented Nov 17, 2017

Looks like experimental got stuck in a syscall:

goroutine 4277 [runnable]:
syscall.Syscall(0x4a, 0x13, 0x0, 0x0, 0x0, 0x0, 0x0)
	/usr/local/go/src/syscall/asm_linux_amd64.s:18 +0x5
syscall.Fsync(0x13, 0x10, 0x96cdb8)
	/usr/local/go/src/syscall/zsyscall_linux_amd64.go:492 +0x4a
os.(*File).Sync(0xc4216e04f0, 0x1e53018, 0xc4217ae148)
	/usr/local/go/src/os/file_posix.go:121 +0x5b
github.com/docker/docker/pkg/ioutils.(*atomicFileWriter).Close(0xc421bcef90, 0x0, 0x0)
	/go/src/github.com/docker/docker/pkg/ioutils/fswriters.go:68 +0x81
github.com/docker/docker/pkg/ioutils.AtomicWriteFile(0xc42134cb00, 0x71, 0xc421090000, 0xe57, 0xe57, 0xc400000180, 0xc420f24990, 0x2b899b0)
	/go/src/github.com/docker/docker/pkg/ioutils/fswriters.go:41 +0xcb
github.com/docker/docker/image.(*fs).Set(0xc420477e30, 0xc421090000, 0xe57, 0xe57, 0x0, 0x0, 0x0, 0x0)
	/go/src/github.com/docker/docker/image/fs.go:121 +0x122
github.com/docker/docker/image.(*store).Create(0xc42022f1a0, 0xc421090000, 0xe57, 0xe57, 0x0, 0x0, 0x0, 0x0)
	/go/src/github.com/docker/docker/image/store.go:146 +0x2c8
github.com/docker/docker/daemon.(*Daemon).Commit(0xc4203c3200, 0xc421c28580, 0x40, 0xc421b39d80, 0x0, 0x0, 0x0, 0x0)
	/go/src/github.com/docker/docker/daemon/commit.go:207 +0x696
github.com/docker/docker/builder/dockerfile.(*Builder).commitContainer(0xc4203eb4a0, 0xc4213c66c0, 0xc421c28580, 0x40, 0xc42123cc80, 0x2b9cba0, 0xc4217ddf60)
	/go/src/github.com/docker/docker/builder/dockerfile/internals.go:117 +0x117
github.com/docker/docker/builder/dockerfile.dispatchRun(0xc4213c66c0, 0xc420addd04, 0xc4203eb4a0, 0x2bafae0, 0xc421af1260, 0xc420feafc0, 0xc42048ff00, 0xc420feafc0, 0xc4217aed00)
	/go/src/github.com/docker/docker/builder/dockerfile/dispatchers.go:356 +0xa7e
github.com/docker/docker/builder/dockerfile.dispatch(0xc4213c66c0, 0xc420addd04, 0xc4203eb4a0, 0x2bafae0, 0xc421af1260, 0xc420feafc0, 0x7fa8b459a008, 0xc42048ff00, 0x0, 0x0)
	/go/src/github.com/docker/docker/builder/dockerfile/evaluator.go:81 +0x457
github.com/docker/docker/builder/dockerfile.(*Builder).dispatchDockerfileWithCancellation(0xc4203eb4a0, 0xc421a77d10, 0x1, 0x1, 0x0, 0x0, 0x0, 0x5c, 0x2bafae0, 0xc421af1260, ...)
	/go/src/github.com/docker/docker/builder/dockerfile/builder.go:322 +0x940
github.com/docker/docker/builder/dockerfile.(*Builder).build(0xc4203eb4a0, 0x2bafae0, 0xc421af1260, 0xc42048f240, 0x0, 0x0, 0x0)
	/go/src/github.com/docker/docker/builder/dockerfile/builder.go:240 +0x3bb
github.com/docker/docker/builder/dockerfile.(*BuildManager).Build(0xc4203b87c0, 0x7fa8b679bba0, 0xc42048f500, 0x2bacf20, 0xc420107280, 0x2b9bd60, 0xc4217ddf40, 0x2b9cba0, 0xc4217ddf60, 0x2b9cba0, ...)
	/go/src/github.com/docker/docker/builder/dockerfile/builder.go:125 +0x4aa
github.com/docker/docker/api/server/backend/build.(*Backend).Build(0xc42085eae0, 0x7fa8b679bba0, 0xc420107380, 0x2bacf20, 0xc420107280, 0x2b9bd60, 0xc4217ddf40, 0x2b9cba0, 0xc4217ddf60, 0x2b9cba0, ...)

Restarting it.

@cpuguy83

This comment has been minimized.

Show comment
Hide comment
@cpuguy83

cpuguy83 Nov 17, 2017

Contributor

Same failure:

panic: DockerSuite.TestBuildAddAndCopyFileWithWhitespace test timed out after 5m0s [recovered]

Contributor

cpuguy83 commented Nov 17, 2017

Same failure:

panic: DockerSuite.TestBuildAddAndCopyFileWithWhitespace test timed out after 5m0s [recovered]

@cpuguy83

This comment has been minimized.

Show comment
Hide comment
@cpuguy83

cpuguy83 Nov 17, 2017

Contributor

It looks like there's some other problems as well.
Janky failed on docker-py tests.

powerpc and z failed on TestCommitHardlink

19:43:33 FAIL: docker_cli_commit_test.go:67: DockerSuite.TestCommitHardlink
19:43:33 
19:43:33 docker_cli_commit_test.go:84:
19:43:33     c.Assert(chunks[1], checker.Contains, chunks[0], check.Commentf("Failed to create hardlink in a container. Expected to find %q in %q", inode, chunks[1:]))
19:43:33 ... obtained string = "\x1b[0;0mfile1\x1b[0m  1466359 \x1b[0;0mfile2\x1b[0m"
19:43:33 ... substring string = "1466358 "
19:43:33 ... Failed to create hardlink in a container. Expected to find "1466358" in ["\x1b[0;0mfile1\x1b[0m  1466359 \x1b[0;0mfile2\x1b[0m"]
Contributor

cpuguy83 commented Nov 17, 2017

It looks like there's some other problems as well.
Janky failed on docker-py tests.

powerpc and z failed on TestCommitHardlink

19:43:33 FAIL: docker_cli_commit_test.go:67: DockerSuite.TestCommitHardlink
19:43:33 
19:43:33 docker_cli_commit_test.go:84:
19:43:33     c.Assert(chunks[1], checker.Contains, chunks[0], check.Commentf("Failed to create hardlink in a container. Expected to find %q in %q", inode, chunks[1:]))
19:43:33 ... obtained string = "\x1b[0;0mfile1\x1b[0m  1466359 \x1b[0;0mfile2\x1b[0m"
19:43:33 ... substring string = "1466358 "
19:43:33 ... Failed to create hardlink in a container. Expected to find "1466358" in ["\x1b[0;0mfile1\x1b[0m  1466359 \x1b[0;0mfile2\x1b[0m"]
@cpuguy83

This comment has been minimized.

Show comment
Hide comment
@cpuguy83

cpuguy83 Nov 17, 2017

Contributor

These errors are likely showing up now because CI runs on VFS.

Contributor

cpuguy83 commented Nov 17, 2017

These errors are likely showing up now because CI runs on VFS.

@sargun

This comment has been minimized.

Show comment
Hide comment
@sargun

sargun Nov 20, 2017

Contributor

@cpuguy83 It seems like the hardlink was creating the issue. I can break the hardlink fix into its own separate commit if you want, but otherwise, things look good to me.

Contributor

sargun commented Nov 20, 2017

@cpuguy83 It seems like the hardlink was creating the issue. I can break the hardlink fix into its own separate commit if you want, but otherwise, things look good to me.

@cpuguy83

This comment has been minimized.

Show comment
Hide comment
@cpuguy83

cpuguy83 Nov 21, 2017

Contributor

Separate commit may be good.
Can we add a unit test for hardlinks as well?

Contributor

cpuguy83 commented Nov 21, 2017

Separate commit may be good.
Can we add a unit test for hardlinks as well?

@sargun

This comment has been minimized.

Show comment
Hide comment
@sargun

sargun Nov 21, 2017

Contributor

There is a test for hard links.

Contributor

sargun commented Nov 21, 2017

There is a test for hard links.

@cpuguy83

This comment has been minimized.

Show comment
Hide comment
@cpuguy83

cpuguy83 Nov 21, 2017

Contributor

Looks good.
One more nit (outside of splitting the fix for the dir copy hardlinks) is our use of syscall instead of golang.org/x/sys/unix.

Contributor

cpuguy83 commented Nov 21, 2017

Looks good.
One more nit (outside of splitting the fix for the dir copy hardlinks) is our use of syscall instead of golang.org/x/sys/unix.

@sargun sargun closed this Nov 21, 2017

@sargun

This comment has been minimized.

Show comment
Hide comment
@sargun

sargun Nov 21, 2017

Contributor

@cpuguy83 I made the test use golang.org/x/sys/unix, but the old code in copy.go is still using syscall.

Contributor

sargun commented Nov 21, 2017

@cpuguy83 I made the test use golang.org/x/sys/unix, but the old code in copy.go is still using syscall.

@sargun sargun reopened this Nov 21, 2017

@sargun

This comment has been minimized.

Show comment
Hide comment
@sargun

sargun Nov 21, 2017

Contributor

I don't see the Windows build being scheduled?

Contributor

sargun commented Nov 21, 2017

I don't see the Windows build being scheduled?

@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Nov 21, 2017

Member

Let me restart, there was an issue earlier with Jenkins

Member

thaJeztah commented Nov 21, 2017

Let me restart, there was an issue earlier with Jenkins

@sargun

This comment has been minimized.

Show comment
Hide comment
@sargun

sargun Nov 21, 2017

Contributor

Finally. @cpuguy83 Split the commit. Changed the tests. Should be good to go.

Contributor

sargun commented Nov 21, 2017

Finally. @cpuguy83 Split the commit. Changed the tests. Should be good to go.

@sargun

This comment has been minimized.

Show comment
Hide comment
@sargun

sargun Nov 22, 2017

Contributor

Some benchmarks:

reflinks:
time docker create centos:latest echo
3dd564d5823994ed54f8e2d646210b4aebcc7622fb6e35d353483057f9d4dd23

real    0m1.154s
user    0m0.008s
sys     0m0.004s

copy_file_range, no reflinks:
time docker create centos:latest echo
737917a226317df9a2f4f7c46d9ed1e83e32a16f4f827eee5e5730f29e61e441

real    0m1.147s
user    0m0.016s
sys     0m0.004s

legacy:
time docker create centos:latest echo
bd925697d2067a6cde175d6a13b4c25f8c23e926422d32303f06e5084647b045

real    0m1.685s
user    0m0.012s
sys     0m0.008s
Contributor

sargun commented Nov 22, 2017

Some benchmarks:

reflinks:
time docker create centos:latest echo
3dd564d5823994ed54f8e2d646210b4aebcc7622fb6e35d353483057f9d4dd23

real    0m1.154s
user    0m0.008s
sys     0m0.004s

copy_file_range, no reflinks:
time docker create centos:latest echo
737917a226317df9a2f4f7c46d9ed1e83e32a16f4f827eee5e5730f29e61e441

real    0m1.147s
user    0m0.016s
sys     0m0.004s

legacy:
time docker create centos:latest echo
bd925697d2067a6cde175d6a13b4c25f8c23e926422d32303f06e5084647b045

real    0m1.685s
user    0m0.012s
sys     0m0.008s
@sargun

This comment has been minimized.

Show comment
Hide comment
@sargun

sargun Nov 22, 2017

Contributor

7.9GB image:

time docker create $MYREGISTRY:7002/drydock/big_data /bin/echo
d8798e4348d288f1952dedd84543c6e739a023be7aba3c9b9059490fbfa36495

real	0m22.611s
user	0m0.012s
sys	0m0.000s

time docker create $MYREGISTRY:7002/drydock/big_data /bin/echo
94a933208cd6bc7986fbf7f2c6c1842f53b055bef20d553a846084df666fc959

real	0m37.500s
user	0m0.024s
sys	0m0.008s
Contributor

sargun commented Nov 22, 2017

7.9GB image:

time docker create $MYREGISTRY:7002/drydock/big_data /bin/echo
d8798e4348d288f1952dedd84543c6e739a023be7aba3c9b9059490fbfa36495

real	0m22.611s
user	0m0.012s
sys	0m0.000s

time docker create $MYREGISTRY:7002/drydock/big_data /bin/echo
94a933208cd6bc7986fbf7f2c6c1842f53b055bef20d553a846084df666fc959

real	0m37.500s
user	0m0.024s
sys	0m0.008s
@sargun

This comment has been minimized.

Show comment
Hide comment
@sargun

sargun Nov 24, 2017

Contributor

With the new code:

time docker create $MYREGISTRY:7002/drydock/big_data /bin/echo
62852bcf9c39968e1c2572d80eac0c796d0d32a8eb793c458d9f53f8cd1440f6

real	0m9.048s
user	0m0.004s
sys	0m0.008s
Contributor

sargun commented Nov 24, 2017

With the new code:

time docker create $MYREGISTRY:7002/drydock/big_data /bin/echo
62852bcf9c39968e1c2572d80eac0c796d0d32a8eb793c458d9f53f8cd1440f6

real	0m9.048s
user	0m0.004s
sys	0m0.008s
@sargun

This comment has been minimized.

Show comment
Hide comment
@sargun

sargun Nov 24, 2017

Contributor

8 was chosen based on testing the speed of the copies. Less than 8, or greater than 8 potentially slowed things down (on a 16 core machine).

Contributor

sargun commented Nov 24, 2017

8 was chosen based on testing the speed of the copies. Less than 8, or greater than 8 potentially slowed things down (on a 16 core machine).

@sargun

This comment has been minimized.

Show comment
Hide comment
@sargun

sargun Nov 24, 2017

Contributor

For those curious about where time is being spent:

% time     seconds  usecs/call     calls    errors syscall
------ ----------- ----------- --------- --------- ----------------
 21.81    9.262997          18    504414        36 openat
 21.41    9.094976          18    504376           close
 12.95    5.502908          23    236285           copy_file_range
 12.13    5.153699          19    266564           fchmodat
 11.01    4.677073          16    284197           lchown
 10.74    4.561071          17    272652           utimensat
  8.58    3.646321        6420       568           sched_yield
  0.92    0.391925           1    295309           lstat
  0.21    0.089337           3     28528         2 mkdirat
  0.14    0.059951           1     57722           getdents64
  0.06    0.024183           1     17633           symlinkat
  0.02    0.007610           0     17632           readlinkat
  0.02    0.006509          21       304        76 read
  0.00    0.001200          15        82           write
  0.00    0.000059           0       157           mknodat
  0.00    0.000056           1        44           linkat
  0.00    0.000000           0        59        14 stat
  0.00    0.000000           0        70           fstat
  0.00    0.000000           0         1           mmap
  0.00    0.000000           0        16           ioctl
  0.00    0.000000           0         1           getpid
  0.00    0.000000           0         3           getsockname
  0.00    0.000000           0         5           fchown
  0.00    0.000000           0         4         2 prctl
  0.00    0.000000           0         1           sched_getaffinity
  0.00    0.000000           0         2         1 restart_syscall
  0.00    0.000000           0         5           epoll_ctl
  0.00    0.000000           0         2           fchownat
  0.00    0.000000           0        17        13 unlinkat
  0.00    0.000000           0         6         3 accept4
  0.00    0.000000           0         1           getrandom
------ ----------- ----------- --------- --------- ----------------
100.00   42.479875               2486660       147 total
Contributor

sargun commented Nov 24, 2017

For those curious about where time is being spent:

% time     seconds  usecs/call     calls    errors syscall
------ ----------- ----------- --------- --------- ----------------
 21.81    9.262997          18    504414        36 openat
 21.41    9.094976          18    504376           close
 12.95    5.502908          23    236285           copy_file_range
 12.13    5.153699          19    266564           fchmodat
 11.01    4.677073          16    284197           lchown
 10.74    4.561071          17    272652           utimensat
  8.58    3.646321        6420       568           sched_yield
  0.92    0.391925           1    295309           lstat
  0.21    0.089337           3     28528         2 mkdirat
  0.14    0.059951           1     57722           getdents64
  0.06    0.024183           1     17633           symlinkat
  0.02    0.007610           0     17632           readlinkat
  0.02    0.006509          21       304        76 read
  0.00    0.001200          15        82           write
  0.00    0.000059           0       157           mknodat
  0.00    0.000056           1        44           linkat
  0.00    0.000000           0        59        14 stat
  0.00    0.000000           0        70           fstat
  0.00    0.000000           0         1           mmap
  0.00    0.000000           0        16           ioctl
  0.00    0.000000           0         1           getpid
  0.00    0.000000           0         3           getsockname
  0.00    0.000000           0         5           fchown
  0.00    0.000000           0         4         2 prctl
  0.00    0.000000           0         1           sched_getaffinity
  0.00    0.000000           0         2         1 restart_syscall
  0.00    0.000000           0         5           epoll_ctl
  0.00    0.000000           0         2           fchownat
  0.00    0.000000           0        17        13 unlinkat
  0.00    0.000000           0         6         3 accept4
  0.00    0.000000           0         1           getrandom
------ ----------- ----------- --------- --------- ----------------
100.00   42.479875               2486660       147 total
@sargun

This comment has been minimized.

Show comment
Hide comment
@sargun

sargun Nov 25, 2017

Contributor

Also, FS with no reflinks (no concurrent copy code):

time docker create $MYREGISTRY:7002/drydock/big_data /bin/echo
31f5543fbcc9846dfc980aeb191f6412857fbbedc172a23aeb376dfe05306548

real	0m37.493s
user	0m0.008s
sys	0m0.024s

Also, FS with no reflinks (concurrent copy code):

time docker create $MYREGISTRY:7002/drydock/big_data /bin/echo
45ed9b11f48ba15aefb7744907897dac893970b5f7909a15cc6fc1a7b636ea9b

real	0m12.147s
user	0m0.020s
sys	0m0.004s

Contributor

sargun commented Nov 25, 2017

Also, FS with no reflinks (no concurrent copy code):

time docker create $MYREGISTRY:7002/drydock/big_data /bin/echo
31f5543fbcc9846dfc980aeb191f6412857fbbedc172a23aeb376dfe05306548

real	0m37.493s
user	0m0.008s
sys	0m0.024s

Also, FS with no reflinks (concurrent copy code):

time docker create $MYREGISTRY:7002/drydock/big_data /bin/echo
45ed9b11f48ba15aefb7744907897dac893970b5f7909a15cc6fc1a7b636ea9b

real	0m12.147s
user	0m0.020s
sys	0m0.004s

sargun added some commits Nov 21, 2017

Have VFS graphdriver use accelerated in-kernel copy
This change makes the VFS graphdriver use the kernel-accelerated
(copy_file_range) mechanism of copying files, which is able to
leverage reflinks.

Signed-off-by: Sargun Dhillon <sargun@sargun.me>
Fix bug, where copy_file_range was still calling legacy copy
There was a small issue here, where it copied the data using
traditional mechanisms, even when copy_file_range was successful.

Signed-off-by: Sargun Dhillon <sargun@sargun.me>
@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Nov 29, 2017

Member

@dmcgowan PR was updated ptal

Member

thaJeztah commented Nov 29, 2017

@dmcgowan PR was updated ptal

@sargun sargun changed the title from Have VFS graphdriver use accelerated in-kernel copy to Have VFS graphdriver use accelerated in-kernel copy; Make accelerated In kernel copy fast Nov 29, 2017

Show outdated Hide outdated daemon/graphdriver/copy/copy.go Outdated
@sargun

This comment has been minimized.

Show comment
Hide comment
@sargun

sargun Nov 29, 2017

Contributor

This looks "right", right?

        id := fileID{dev: fileToCopy.stat.Dev, ino: fileToCopy.stat.Ino}

        copiedFilesLocker.Lock(id)
        if otherFileName, ok := copiedFiles[id]; ok {
                copiedFilesLocker.Unlock(id)
                // Make the hardlink, and return
                // it could potentially fail, if the other dstFile
                // had issues copying
                return os.Link(otherFileName, fileToCopy.dstPath)
        }

        if err := copyRegular(fileToCopy.srcPath, fileToCopy.dstPath, fileToCopy.fileInfo, copyWithFileRange, copyWithFileClone); err != nil {
                copiedFilesLocker.Unlock(id)
                return err
        }
        copiedFiles[id] = fileToCopy.dstPath
        copiedFilesLocker.Unlock(id)
Contributor

sargun commented Nov 29, 2017

This looks "right", right?

        id := fileID{dev: fileToCopy.stat.Dev, ino: fileToCopy.stat.Ino}

        copiedFilesLocker.Lock(id)
        if otherFileName, ok := copiedFiles[id]; ok {
                copiedFilesLocker.Unlock(id)
                // Make the hardlink, and return
                // it could potentially fail, if the other dstFile
                // had issues copying
                return os.Link(otherFileName, fileToCopy.dstPath)
        }

        if err := copyRegular(fileToCopy.srcPath, fileToCopy.dstPath, fileToCopy.fileInfo, copyWithFileRange, copyWithFileClone); err != nil {
                copiedFilesLocker.Unlock(id)
                return err
        }
        copiedFiles[id] = fileToCopy.dstPath
        copiedFilesLocker.Unlock(id)
@sargun

This comment has been minimized.

Show comment
Hide comment
@sargun

sargun Nov 29, 2017

Contributor

If so, wtf:

fatal error: concurrent map read and map write

goroutine 19 [running]:
runtime.throw(0x6fe106, 0x21)
	/usr/local/go/src/runtime/panic.go:605 +0x95 fp=0xc420039cd8 sp=0xc420039cb8 pc=0x42cf05
runtime.mapaccess2(0x69f660, 0xc420077530, 0xc420039dd8, 0xc420011930, 0x2)
	/usr/local/go/src/runtime/hashmap.go:413 +0x24e fp=0xc420039d20 sp=0xc420039cd8 pc=0x409d5e
github.com/docker/docker/daemon/graphdriver/copy.concurrentCopyHelper(0xc420077530, 0xc42003ee50, 0xa53640, 0xc42063eb60, 0xc42063eb98, 0xc42063ce70, 0x30, 0xc420642780, 0x31, 0x0, ...)
	/home/sargun/go/src/github.com/docker/docker/daemon/graphdriver/copy/copy.go:122 +0xde fp=0xc420039e20 sp=0xc420039d20 pc=0x657e9e
github.com/docker/docker/daemon/graphdriver/copy.concurrentFileCopier(0xc420077530, 0xc42003ee50, 0xc42005c420, 0xc4200748a0, 0xc42005c600, 0xc42018ced0, 0x0)
	/home/sargun/go/src/github.com/docker/docker/daemon/graphdriver/copy/copy.go:171 +0x20d fp=0xc420039fa8 sp=0xc420039e20 pc=0x65864d
runtime.goexit()
	/usr/local/go/src/runtime/asm_amd64.s:2337 +0x1 fp=0xc420039fb0 sp=0xc420039fa8 pc=0x45d2e1
created by github.com/docker/docker/daemon/graphdriver/copy.DirCopy
	/home/sargun/go/src/github.com/docker/docker/daemon/graphdriver/copy/copy.go:216 +0x25b

goroutine 1 [chan receive]:
testing.(*T).Run(0xc420104000, 0x6f6ce4, 0xb, 0x708ac0, 0x479f01)
	/usr/local/go/src/testing/testing.go:790 +0x2fc
testing.runTests.func1(0xc420104000)
	/usr/local/go/src/testing/testing.go:1004 +0x64
testing.tRunner(0xc420104000, 0xc42004dde0)
	/usr/local/go/src/testing/testing.go:746 +0xd0
testing.runTests(0xc4200e8480, 0xa496e0, 0x5, 0x5, 0xc42003ed00)
	/usr/local/go/src/testing/testing.go:1002 +0x2d8
testing.(*M).Run(0xc42004df18, 0xc42004df70)
	/usr/local/go/src/testing/testing.go:921 +0x111
main.main()
	github.com/docker/docker/daemon/graphdriver/copy/_test/_testmain.go:52 +0xdb

goroutine 8 [runnable]:
syscall.Syscall(0x102, 0xffffffffffffff9c, 0xc420642c40, 0x1cc, 0x0, 0x1cc, 0x0)
	/usr/local/go/src/syscall/asm_linux_amd64.s:18 +0x5
syscall.Mkdirat(0xffffffffffffff9c, 0xc42063d3e0, 0x30, 0x1cc, 0xc42063d3e0, 0x30)
	/usr/local/go/src/syscall/zsyscall_linux_amd64.go:675 +0x8f
syscall.Mkdir(0xc42063d3e0, 0x30, 0x1cc, 0xc42003b8e8, 0x2)
	/usr/local/go/src/syscall/syscall_linux.go:43 +0x46
os.Mkdir(0xc42063d3e0, 0x30, 0xc4800001cc, 0xc42063d3e0, 0x30)
	/usr/local/go/src/os/file.go:210 +0x69
github.com/docker/docker/daemon/graphdriver/copy.DirCopy.func1(0xc42063d380, 0x2f, 0xa53640, 0xc42063f450, 0x0, 0x0, 0x0, 0x0)
	/home/sargun/go/src/github.com/docker/docker/daemon/graphdriver/copy/copy.go:269 +0xc37
path/filepath.walk(0xc42063d380, 0x2f, 0xa53640, 0xc42063f450, 0xc420338360, 0x0, 0x0)
	/usr/local/go/src/path/filepath/path.go:356 +0x81
path/filepath.walk(0xc42063d230, 0x26, 0xa53640, 0xc42063f380, 0xc420338360, 0x0, 0x0)
	/usr/local/go/src/path/filepath/path.go:381 +0x39a
path/filepath.walk(0xc420012580, 0x1d, 0xa53640, 0xc42063e1a0, 0xc420338360, 0x0, 0x0)
	/usr/local/go/src/path/filepath/path.go:381 +0x39a
path/filepath.walk(0xc420012b80, 0x14, 0xa53640, 0xc420084270, 0xc420338360, 0x0, 0x60)
	/usr/local/go/src/path/filepath/path.go:381 +0x39a
path/filepath.Walk(0xc420012b80, 0x14, 0xc420338360, 0xc42003ee50, 0xc42005c420)
	/usr/local/go/src/path/filepath/path.go:403 +0x11d
github.com/docker/docker/daemon/graphdriver/copy.DirCopy(0xc420012b80, 0x14, 0xc420013340, 0x15, 0x0, 0x0, 0x0, 0x0)
	/home/sargun/go/src/github.com/docker/docker/daemon/graphdriver/copy/copy.go:219 +0x3fc
github.com/docker/docker/daemon/graphdriver/copy.TestCopyDir(0xc4201043c0)
	/home/sargun/go/src/github.com/docker/docker/daemon/graphdriver/copy/copy_test.go:62 +0x1d3
testing.tRunner(0xc4201043c0, 0x708ac0)
	/usr/local/go/src/testing/testing.go:746 +0xd0
created by testing.(*T).Run
	/usr/local/go/src/testing/testing.go:789 +0x2de

goroutine 9 [runnable]:
os.Chmod(0xc42063c2a0, 0x28, 0x13d, 0x3e8, 0x0)
	/usr/local/go/src/os/file.go:314 +0x60
github.com/docker/docker/daemon/graphdriver/copy.concurrentCopyHelper(0xc420077530, 0xc42003ee50, 0xa53640, 0xc42062dc70, 0xc42062dca8, 0xc42063c240, 0x27, 0xc42063c2a0, 0x28, 0x0, ...)
	/home/sargun/go/src/github.com/docker/docker/daemon/graphdriver/copy/copy.go:147 +0x2bb
github.com/docker/docker/daemon/graphdriver/copy.concurrentFileCopier(0xc420077530, 0xc42003ee50, 0xc42005c420, 0xc4200748a0, 0xc42005c480, 0xc42018ced0, 0x0)
	/home/sargun/go/src/github.com/docker/docker/daemon/graphdriver/copy/copy.go:171 +0x20d
created by github.com/docker/docker/daemon/graphdriver/copy.DirCopy
	/home/sargun/go/src/github.com/docker/docker/daemon/graphdriver/copy/copy.go:216 +0x25b

goroutine 10 [select]:
github.com/docker/docker/daemon/graphdriver/copy.concurrentFileCopier(0xc420077530, 0xc42003ee50, 0xc42005c420, 0xc4200748a0, 0xc42005c4e0, 0xc42018ced0, 0x0)
	/home/sargun/go/src/github.com/docker/docker/daemon/graphdriver/copy/copy.go:164 +0x14a
created by github.com/docker/docker/daemon/graphdriver/copy.DirCopy
	/home/sargun/go/src/github.com/docker/docker/daemon/graphdriver/copy/copy.go:216 +0x25b

goroutine 11 [select]:
github.com/docker/docker/daemon/graphdriver/copy.concurrentFileCopier(0xc420077530, 0xc42003ee50, 0xc42005c420, 0xc4200748a0, 0xc42005c540, 0xc42018ced0, 0x0)
	/home/sargun/go/src/github.com/docker/docker/daemon/graphdriver/copy/copy.go:164 +0x14a
created by github.com/docker/docker/daemon/graphdriver/copy.DirCopy
	/home/sargun/go/src/github.com/docker/docker/daemon/graphdriver/copy/copy.go:216 +0x25b

goroutine 12 [select]:
github.com/docker/docker/daemon/graphdriver/copy.concurrentFileCopier(0xc420077530, 0xc42003ee50, 0xc42005c420, 0xc4200748a0, 0xc42005c5a0, 0xc42018ced0, 0x0)
	/home/sargun/go/src/github.com/docker/docker/daemon/graphdriver/copy/copy.go:164 +0x14a
created by github.com/docker/docker/daemon/graphdriver/copy.DirCopy
	/home/sargun/go/src/github.com/docker/docker/daemon/graphdriver/copy/copy.go:216 +0x25b

goroutine 20 [select]:
github.com/docker/docker/daemon/graphdriver/copy.concurrentFileCopier(0xc420077530, 0xc42003ee50, 0xc42005c420, 0xc4200748a0, 0xc4204580c0, 0xc42018ced0, 0x0)
	/home/sargun/go/src/github.com/docker/docker/daemon/graphdriver/copy/copy.go:164 +0x14a
created by github.com/docker/docker/daemon/graphdriver/copy.DirCopy
	/home/sargun/go/src/github.com/docker/docker/daemon/graphdriver/copy/copy.go:216 +0x25b

goroutine 21 [select]:
github.com/docker/docker/daemon/graphdriver/copy.concurrentFileCopier(0xc420077530, 0xc42003ee50, 0xc42005c420, 0xc4200748a0, 0xc420458120, 0xc42018ced0, 0x0)
	/home/sargun/go/src/github.com/docker/docker/daemon/graphdriver/copy/copy.go:164 +0x14a
created by github.com/docker/docker/daemon/graphdriver/copy.DirCopy
	/home/sargun/go/src/github.com/docker/docker/daemon/graphdriver/copy/copy.go:216 +0x25b

goroutine 15 [select]:
github.com/docker/docker/daemon/graphdriver/copy.concurrentFileCopier(0xc420077530, 0xc42003ee50, 0xc42005c420, 0xc4200748a0, 0xc420338300, 0xc42018ced0, 0x0)
	/home/sargun/go/src/github.com/docker/docker/daemon/graphdriver/copy/copy.go:164 +0x14a
created by github.com/docker/docker/daemon/graphdriver/copy.DirCopy
	/home/sargun/go/src/github.com/docker/docker/daemon/graphdriver/copy/copy.go:216 +0x25b
exit status 2
FAIL	github.com/docker/docker/daemon/graphdriver/copy	0.080s

Contributor

sargun commented Nov 29, 2017

If so, wtf:

fatal error: concurrent map read and map write

goroutine 19 [running]:
runtime.throw(0x6fe106, 0x21)
	/usr/local/go/src/runtime/panic.go:605 +0x95 fp=0xc420039cd8 sp=0xc420039cb8 pc=0x42cf05
runtime.mapaccess2(0x69f660, 0xc420077530, 0xc420039dd8, 0xc420011930, 0x2)
	/usr/local/go/src/runtime/hashmap.go:413 +0x24e fp=0xc420039d20 sp=0xc420039cd8 pc=0x409d5e
github.com/docker/docker/daemon/graphdriver/copy.concurrentCopyHelper(0xc420077530, 0xc42003ee50, 0xa53640, 0xc42063eb60, 0xc42063eb98, 0xc42063ce70, 0x30, 0xc420642780, 0x31, 0x0, ...)
	/home/sargun/go/src/github.com/docker/docker/daemon/graphdriver/copy/copy.go:122 +0xde fp=0xc420039e20 sp=0xc420039d20 pc=0x657e9e
github.com/docker/docker/daemon/graphdriver/copy.concurrentFileCopier(0xc420077530, 0xc42003ee50, 0xc42005c420, 0xc4200748a0, 0xc42005c600, 0xc42018ced0, 0x0)
	/home/sargun/go/src/github.com/docker/docker/daemon/graphdriver/copy/copy.go:171 +0x20d fp=0xc420039fa8 sp=0xc420039e20 pc=0x65864d
runtime.goexit()
	/usr/local/go/src/runtime/asm_amd64.s:2337 +0x1 fp=0xc420039fb0 sp=0xc420039fa8 pc=0x45d2e1
created by github.com/docker/docker/daemon/graphdriver/copy.DirCopy
	/home/sargun/go/src/github.com/docker/docker/daemon/graphdriver/copy/copy.go:216 +0x25b

goroutine 1 [chan receive]:
testing.(*T).Run(0xc420104000, 0x6f6ce4, 0xb, 0x708ac0, 0x479f01)
	/usr/local/go/src/testing/testing.go:790 +0x2fc
testing.runTests.func1(0xc420104000)
	/usr/local/go/src/testing/testing.go:1004 +0x64
testing.tRunner(0xc420104000, 0xc42004dde0)
	/usr/local/go/src/testing/testing.go:746 +0xd0
testing.runTests(0xc4200e8480, 0xa496e0, 0x5, 0x5, 0xc42003ed00)
	/usr/local/go/src/testing/testing.go:1002 +0x2d8
testing.(*M).Run(0xc42004df18, 0xc42004df70)
	/usr/local/go/src/testing/testing.go:921 +0x111
main.main()
	github.com/docker/docker/daemon/graphdriver/copy/_test/_testmain.go:52 +0xdb

goroutine 8 [runnable]:
syscall.Syscall(0x102, 0xffffffffffffff9c, 0xc420642c40, 0x1cc, 0x0, 0x1cc, 0x0)
	/usr/local/go/src/syscall/asm_linux_amd64.s:18 +0x5
syscall.Mkdirat(0xffffffffffffff9c, 0xc42063d3e0, 0x30, 0x1cc, 0xc42063d3e0, 0x30)
	/usr/local/go/src/syscall/zsyscall_linux_amd64.go:675 +0x8f
syscall.Mkdir(0xc42063d3e0, 0x30, 0x1cc, 0xc42003b8e8, 0x2)
	/usr/local/go/src/syscall/syscall_linux.go:43 +0x46
os.Mkdir(0xc42063d3e0, 0x30, 0xc4800001cc, 0xc42063d3e0, 0x30)
	/usr/local/go/src/os/file.go:210 +0x69
github.com/docker/docker/daemon/graphdriver/copy.DirCopy.func1(0xc42063d380, 0x2f, 0xa53640, 0xc42063f450, 0x0, 0x0, 0x0, 0x0)
	/home/sargun/go/src/github.com/docker/docker/daemon/graphdriver/copy/copy.go:269 +0xc37
path/filepath.walk(0xc42063d380, 0x2f, 0xa53640, 0xc42063f450, 0xc420338360, 0x0, 0x0)
	/usr/local/go/src/path/filepath/path.go:356 +0x81
path/filepath.walk(0xc42063d230, 0x26, 0xa53640, 0xc42063f380, 0xc420338360, 0x0, 0x0)
	/usr/local/go/src/path/filepath/path.go:381 +0x39a
path/filepath.walk(0xc420012580, 0x1d, 0xa53640, 0xc42063e1a0, 0xc420338360, 0x0, 0x0)
	/usr/local/go/src/path/filepath/path.go:381 +0x39a
path/filepath.walk(0xc420012b80, 0x14, 0xa53640, 0xc420084270, 0xc420338360, 0x0, 0x60)
	/usr/local/go/src/path/filepath/path.go:381 +0x39a
path/filepath.Walk(0xc420012b80, 0x14, 0xc420338360, 0xc42003ee50, 0xc42005c420)
	/usr/local/go/src/path/filepath/path.go:403 +0x11d
github.com/docker/docker/daemon/graphdriver/copy.DirCopy(0xc420012b80, 0x14, 0xc420013340, 0x15, 0x0, 0x0, 0x0, 0x0)
	/home/sargun/go/src/github.com/docker/docker/daemon/graphdriver/copy/copy.go:219 +0x3fc
github.com/docker/docker/daemon/graphdriver/copy.TestCopyDir(0xc4201043c0)
	/home/sargun/go/src/github.com/docker/docker/daemon/graphdriver/copy/copy_test.go:62 +0x1d3
testing.tRunner(0xc4201043c0, 0x708ac0)
	/usr/local/go/src/testing/testing.go:746 +0xd0
created by testing.(*T).Run
	/usr/local/go/src/testing/testing.go:789 +0x2de

goroutine 9 [runnable]:
os.Chmod(0xc42063c2a0, 0x28, 0x13d, 0x3e8, 0x0)
	/usr/local/go/src/os/file.go:314 +0x60
github.com/docker/docker/daemon/graphdriver/copy.concurrentCopyHelper(0xc420077530, 0xc42003ee50, 0xa53640, 0xc42062dc70, 0xc42062dca8, 0xc42063c240, 0x27, 0xc42063c2a0, 0x28, 0x0, ...)
	/home/sargun/go/src/github.com/docker/docker/daemon/graphdriver/copy/copy.go:147 +0x2bb
github.com/docker/docker/daemon/graphdriver/copy.concurrentFileCopier(0xc420077530, 0xc42003ee50, 0xc42005c420, 0xc4200748a0, 0xc42005c480, 0xc42018ced0, 0x0)
	/home/sargun/go/src/github.com/docker/docker/daemon/graphdriver/copy/copy.go:171 +0x20d
created by github.com/docker/docker/daemon/graphdriver/copy.DirCopy
	/home/sargun/go/src/github.com/docker/docker/daemon/graphdriver/copy/copy.go:216 +0x25b

goroutine 10 [select]:
github.com/docker/docker/daemon/graphdriver/copy.concurrentFileCopier(0xc420077530, 0xc42003ee50, 0xc42005c420, 0xc4200748a0, 0xc42005c4e0, 0xc42018ced0, 0x0)
	/home/sargun/go/src/github.com/docker/docker/daemon/graphdriver/copy/copy.go:164 +0x14a
created by github.com/docker/docker/daemon/graphdriver/copy.DirCopy
	/home/sargun/go/src/github.com/docker/docker/daemon/graphdriver/copy/copy.go:216 +0x25b

goroutine 11 [select]:
github.com/docker/docker/daemon/graphdriver/copy.concurrentFileCopier(0xc420077530, 0xc42003ee50, 0xc42005c420, 0xc4200748a0, 0xc42005c540, 0xc42018ced0, 0x0)
	/home/sargun/go/src/github.com/docker/docker/daemon/graphdriver/copy/copy.go:164 +0x14a
created by github.com/docker/docker/daemon/graphdriver/copy.DirCopy
	/home/sargun/go/src/github.com/docker/docker/daemon/graphdriver/copy/copy.go:216 +0x25b

goroutine 12 [select]:
github.com/docker/docker/daemon/graphdriver/copy.concurrentFileCopier(0xc420077530, 0xc42003ee50, 0xc42005c420, 0xc4200748a0, 0xc42005c5a0, 0xc42018ced0, 0x0)
	/home/sargun/go/src/github.com/docker/docker/daemon/graphdriver/copy/copy.go:164 +0x14a
created by github.com/docker/docker/daemon/graphdriver/copy.DirCopy
	/home/sargun/go/src/github.com/docker/docker/daemon/graphdriver/copy/copy.go:216 +0x25b

goroutine 20 [select]:
github.com/docker/docker/daemon/graphdriver/copy.concurrentFileCopier(0xc420077530, 0xc42003ee50, 0xc42005c420, 0xc4200748a0, 0xc4204580c0, 0xc42018ced0, 0x0)
	/home/sargun/go/src/github.com/docker/docker/daemon/graphdriver/copy/copy.go:164 +0x14a
created by github.com/docker/docker/daemon/graphdriver/copy.DirCopy
	/home/sargun/go/src/github.com/docker/docker/daemon/graphdriver/copy/copy.go:216 +0x25b

goroutine 21 [select]:
github.com/docker/docker/daemon/graphdriver/copy.concurrentFileCopier(0xc420077530, 0xc42003ee50, 0xc42005c420, 0xc4200748a0, 0xc420458120, 0xc42018ced0, 0x0)
	/home/sargun/go/src/github.com/docker/docker/daemon/graphdriver/copy/copy.go:164 +0x14a
created by github.com/docker/docker/daemon/graphdriver/copy.DirCopy
	/home/sargun/go/src/github.com/docker/docker/daemon/graphdriver/copy/copy.go:216 +0x25b

goroutine 15 [select]:
github.com/docker/docker/daemon/graphdriver/copy.concurrentFileCopier(0xc420077530, 0xc42003ee50, 0xc42005c420, 0xc4200748a0, 0xc420338300, 0xc42018ced0, 0x0)
	/home/sargun/go/src/github.com/docker/docker/daemon/graphdriver/copy/copy.go:164 +0x14a
created by github.com/docker/docker/daemon/graphdriver/copy.DirCopy
	/home/sargun/go/src/github.com/docker/docker/daemon/graphdriver/copy/copy.go:216 +0x25b
exit status 2
FAIL	github.com/docker/docker/daemon/graphdriver/copy	0.080s

@sargun

This comment has been minimized.

Show comment
Hide comment
@sargun

sargun Dec 1, 2017

Contributor

It seems like users are no longer able to restart their own Jenkins builds. Is this intentional?

Contributor

sargun commented Dec 1, 2017

It seems like users are no longer able to restart their own Jenkins builds. Is this intentional?

@sargun

This comment has been minimized.

Show comment
Hide comment
@sargun

sargun Dec 1, 2017

Contributor

If it helps, we could merge the non-lolspeed version in, and then I can do another PR with the concurrency stuffs.

Contributor

sargun commented Dec 1, 2017

If it helps, we could merge the non-lolspeed version in, and then I can do another PR with the concurrency stuffs.

@cpuguy83

Right now there's a lot of complexity that seems like we could simplify.

There are currently 3 different concurrency primitives in use.
If we can change this to use at most two concurrency primitives (e.g. chan + a waitgroup to simplify chan handling) that will be a good start.

Not sure what to do with the linked list... it's probably ok but don't often see it in go code. Can we use a slice instead?

@sargun

This comment has been minimized.

Show comment
Hide comment
@sargun

sargun Dec 1, 2017

Contributor

The linked list seems like a better approach than a slice to me because the slice will require constant resizes / reallocations. I don't see any benefit of using a slice, other than potential perf cost.

RE: Concurrency primitives --
Really, there's the waitgroup + chans coordinating the divide and conquer. We could replace some of that by a golang.org/x/sync/errgroup, (https://godoc.org/golang.org/x/sync/errgroup), but I'm not sure how much better that is?

I don't see a better way to deal with the inode references.

Contributor

sargun commented Dec 1, 2017

The linked list seems like a better approach than a slice to me because the slice will require constant resizes / reallocations. I don't see any benefit of using a slice, other than potential perf cost.

RE: Concurrency primitives --
Really, there's the waitgroup + chans coordinating the divide and conquer. We could replace some of that by a golang.org/x/sync/errgroup, (https://godoc.org/golang.org/x/sync/errgroup), but I'm not sure how much better that is?

I don't see a better way to deal with the inode references.

@sargun

This comment has been minimized.

Show comment
Hide comment
@sargun

sargun Dec 1, 2017

Contributor

To me, errgroup would be yet another set of primitives.

Contributor

sargun commented Dec 1, 2017

To me, errgroup would be yet another set of primitives.

@cpuguy83

This comment has been minimized.

Show comment
Hide comment
@cpuguy83

cpuguy83 Dec 1, 2017

Contributor

There's (multiple) mutex + (multiple) chan + waitgroup.

Contributor

cpuguy83 commented Dec 1, 2017

There's (multiple) mutex + (multiple) chan + waitgroup.

@sargun

This comment has been minimized.

Show comment
Hide comment
@sargun

sargun Dec 1, 2017

Contributor

I mean, I can also break out the concurrency code into a separate module, if that helps? I don't see a particularly good way to actually remove many of these unless we start using interface{} in places, or overloading?

Contributor

sargun commented Dec 1, 2017

I mean, I can also break out the concurrency code into a separate module, if that helps? I don't see a particularly good way to actually remove many of these unless we start using interface{} in places, or overloading?

Fix setting mtimes on directories
Previously, the code would set the mtime on the directories before
creating files in the directory itself. This was problematic
because it resulted in the mtimes on the directories being
incorrectly set. This change makes it so that the mtime is
set only _after_ all of the files have been created.

Signed-off-by: Sargun Dhillon <sargun@sargun.me>

@sargun sargun changed the title from Have VFS graphdriver use accelerated in-kernel copy; Make accelerated In kernel copy fast to Have VFS graphdriver use accelerated in-kernel copy Dec 1, 2017

@cpuguy83 cpuguy83 added the rebuild/z label Dec 4, 2017

@cpuguy83

LGTM

@sargun

This comment has been minimized.

Show comment
Hide comment
@sargun

sargun Dec 4, 2017

Contributor

DockerSwarmSuite.TestSwarmLockUnlockCluster failed. Can someone restart janky?

Contributor

sargun commented Dec 4, 2017

DockerSwarmSuite.TestSwarmLockUnlockCluster failed. Can someone restart janky?

@yongtang yongtang merged commit 4047ced into moby:master Dec 5, 2017

6 checks passed

dco-signed All commits are signed
experimental Jenkins build Docker-PRs-experimental 38133 has succeeded
Details
janky Jenkins build Docker-PRs 46894 has succeeded
Details
powerpc Jenkins build Docker-PRs-powerpc 7256 has succeeded
Details
windowsRS1 Jenkins build Docker-PRs-WoW-RS1 18401 has succeeded
Details
z Jenkins build Docker-PRs-s390x 7104 has succeeded
Details

@sargun sargun deleted the sargun:vfs-use-copy_file_range branch Jan 25, 2018

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