Skip to content
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

Add copy-on-write support for Linux BTRFS filesystem #952

Merged
merged 2 commits into from Feb 4, 2016

Conversation

bozaro
Copy link
Contributor

@bozaro bozaro commented Jan 26, 2016

Use btrfs copy-on-write feature for copy lfs files from .git/lfs/objects to working copy (git lfs checkout).
Copy-on-write operation is look like hardlink, but change in one file is not visible in another file.

This command works like: cp --reflink=auto src.txt dst.txt
For this change I use https://github.com/wertarbyte/coreutils/blob/master/src/copy.c as reference.

@technoweenie
Copy link
Contributor

This looks good. However, as someone that doesn't use linux much and has 0 experience with the BTRFS, I'll have to rely on you to support this. Based on our previous interactions, I don't think that'll be a problem, but I wanted to make that clear :)

This doesn't work when I build the linux binaries:

Building for linux/amd64
# github.com/github/git-lfs/lfs
lfs/util.go:48: undefined: CloneFile

Probably because I'm building cross platform binaries without cgo. Should be easy to fix with another file with a build flag like +build linux,!cgo or something. Though, I imagine most linux devs install from the packages, which build git-lfs from source. Part of me wonders if I should even bother with the cross compiled tarballs then?

What will this do on a linux system without btrfs? Will that syscall just return an error, causing CopyWithCallback() to copy the file manually?

@bozaro
Copy link
Contributor Author

bozaro commented Jan 29, 2016

I write wrong build condition:
//+build !linux,cgo ((not linux) and cgo)

It's also means: you build without cgo

What will this do on a linux system without btrfs? Will that syscall just return an error, causing CopyWithCallback() to copy the file manually?

Btrfs is supported on linux a long time (for example Ubuntu 11.04 supports btrfs filesystem). In situation with unknown syscall it simply return error and failback to block-by-block copy. Failback to block-by-block copy also will be in any situations when copy-on-write is inpossible (files on different filesystems, files on ext4, too old btrfs implementation without copy-on-write and etc).

In benefits btrfs users have:

  • Cloning files from .git/lfs/objects to working directory is extremely fast;
  • Unmodified LFS files in working directory don't use aditional disk space.

All other user have penalty by one redundant syscall per file.

I checked this code locally on Ubuntu 14.04 ext4 and btrfs.

Personally, I'm most upset me, I do not know how to implement a similar function under Windows :(

@technoweenie
Copy link
Contributor

Cool, thanks for fixing the build flag. This should make it into a new release next week. I want to make sure my linux and windows vm tests all pass first.

Thanks!

@technoweenie
Copy link
Contributor

This fails in the centos 5 docker build.

$ ./docker/run_dockers.bsh centos_5
centos_5: Pulling from andyneff/git-lfs_dockers
Digest: sha256:e75cba3fb67d5b2a3490ea800bd50d15543e7529358367f1def9bf2e68cc18d3
Status: Image is up to date for andyneff/git-lfs_dockers:centos_5
Compiling LFS in docker image centos_5
...
+ ./script/bootstrap
lfs/util_linux.go
# github.com/github/git-lfs/lfs
/usr/bin/ld: unrecognized option '--build-id=none'
/usr/bin/ld: use the --help option for usage information
collect2: ld returned 1 exit status

It builds successfully in all of the other docker files:

$ ./docker/run_dockers.bsh centos_{6,7} debian_{7,8}

It looks like we can pass custom build flags. Would it be worth trying to pass flags like lfs_centos_5? Then we can set the build flags for the following files:

# this contains the btrfs code
# lfs/util_linux.go
// +build linux,cgo
// +build !lfs_centos_5

# lfs/util_generic.go
// +build !linux !cgo lfs_centos_5

Though, I'm not even sure that would fix that ld error above.

@andyneff
Copy link
Contributor

andyneff commented Feb 3, 2016

If I remember correctly, golang doesn't like having a build ids in the debug symbols. This is basically a checksum of a portion of the binary data (Not all of it so that if you compile the same code twice you get the same checksum). This is important in debugging in Fedora operating systems because this is how debugging symbols are matched with binary files after being turned into RPMs, where the debug symbols and binary files are split into two. In the end, I gave up and just disabled the debug symbol handling in the RPMs.

I'm guessing something in the // flags triggers go to add the --build-id flag, which Centos 5 doesn't support.

Since go doesn't really support Centos 5... This might be a good excuse to drop the Centos 5 building... ;)

@bozaro
Copy link
Contributor Author

bozaro commented Feb 3, 2016

About this build issue
I think, this error will occurs on any cgo usage. So, simplest way is disable cgo on CentOS 5 by environment variable:

export CGO_ENABLED=0

About CentOS 5
https://golang.org/doc/install#requirements

CentOS/RHEL 5.x not supported

https://wiki.centos.org/FAQ/CentOS5#head-a2fd35a09ebf776661ab0f1288762329bb64209c

We intend to support CentOS 5 until Mar 31st, 2017 The current plan is this:

  • Full Updates (including hardware updates): Currently to Q4, 2012
  • Updates ( including minor hardware updates): Up to Q1 of 2014
  • Maintenance Updates Q1, 2011 - Mar 31st, 2017

@andyneff
Copy link
Contributor

andyneff commented Feb 3, 2016

Is cgo not needed for this BTRFS feature?

@bozaro
Copy link
Contributor Author

bozaro commented Feb 3, 2016

I use cgo for calculate correct BTRFS_IOC_CLONE value (I'm not sure that _IOW macro is platform-independent).
Disabling cgo also turned off the change of this PR.

@technoweenie
Copy link
Contributor

I'm in favor of disabling CGO on centos 5. If it's worth just dropping all support, that works too :) Once that happens, we can merge this PR.

andyneff added a commit to andyneff/git-lfs that referenced this pull request Feb 4, 2016
technoweenie added a commit to technoweenie/git-lfs_dockers that referenced this pull request Feb 4, 2016
technoweenie added a commit that referenced this pull request Feb 4, 2016
Disable CGO for Centos 5 to assist #952
@technoweenie technoweenie mentioned this pull request Feb 4, 2016
15 tasks
@technoweenie
Copy link
Contributor

We just ended up disabling CGO in centos 5:

We'll drop centos 5 support once it's no longer under "maintenance support".

Thanks again for the patch!

technoweenie added a commit that referenced this pull request Feb 4, 2016
Add copy-on-write support for Linux BTRFS filesystem
@technoweenie technoweenie merged commit 289c273 into git-lfs:master Feb 4, 2016
@bozaro bozaro deleted the btrfs-clone branch March 10, 2017 19:31
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.

None yet

3 participants