Skip to content

Commit

Permalink
Add batch size config value and use it everywhere
Browse files Browse the repository at this point in the history
Fix review comments. Update the documentation for this feature, remove
unnesessary usages, fix naming and codestyle, create a test for feature.
  • Loading branch information
bogomolets-owl committed Oct 8, 2024
1 parent f5416d5 commit 3161b34
Show file tree
Hide file tree
Showing 9 changed files with 118 additions and 13 deletions.
4 changes: 1 addition & 3 deletions commands/command_fetch.go
Original file line number Diff line number Diff line change
Expand Up @@ -329,9 +329,7 @@ func fetchAndReportToChan(allpointers []*lfs.WrappedPointer, filter *filepathfil
ready, pointers, meter := readyAndMissingPointers(allpointers, filter)
q := newDownloadQueue(
getTransferManifestOperationRemote("download", cfg.Remote()),
cfg.Remote(),
tq.WithProgress(meter),
tq.WithBatchSize(cfg.BatchSize()),
cfg.Remote(), tq.WithProgress(meter),
)

if out != nil {
Expand Down
2 changes: 1 addition & 1 deletion commands/command_filter_process.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ func filterCommand(cmd *cobra.Command, args []string) {
getTransferManifestOperationRemote("download", cfg.Remote()),
cfg.Remote(),
tq.RemoteRef(currentRemoteRef()),
tq.WithBatchSize(cfg.BatchSize()),
tq.WithBatchSize(cfg.TransferBatchSize()),
)
go infiniteTransferBuffer(q, available)
}
Expand Down
2 changes: 1 addition & 1 deletion commands/command_pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func pull(filter *filepathfilter.Filter) {
logger.Enqueue(meter)
remote := cfg.Remote()
singleCheckout := newSingleCheckout(cfg.Git, remote)
q := newDownloadQueue(singleCheckout.Manifest(), remote, tq.WithProgress(meter), tq.WithBatchSize(cfg.BatchSize()))
q := newDownloadQueue(singleCheckout.Manifest(), remote, tq.WithProgress(meter))
gitscanner := lfs.NewGitScanner(cfg, func(p *lfs.WrappedPointer, err error) {
if err != nil {
LoggedError(err, tr.Tr.Get("Scanner error: %s", err))
Expand Down
2 changes: 1 addition & 1 deletion commands/commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ func newDownloadCheckQueue(manifest tq.Manifest, remote string, options ...tq.Op
func newDownloadQueue(manifest tq.Manifest, remote string, options ...tq.Option) *tq.TransferQueue {
return tq.NewTransferQueue(tq.Download, manifest, remote, append(options,
tq.RemoteRef(currentRemoteRef()),
tq.WithBatchSize(cfg.BatchSize()),
tq.WithBatchSize(cfg.TransferBatchSize()),
)...)
}

Expand Down
2 changes: 1 addition & 1 deletion commands/uploader.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ func (c *uploadContext) NewQueue(options ...tq.Option) *tq.TransferQueue {
return tq.NewTransferQueue(tq.Upload, c.Manifest, c.Remote, append(options,
tq.DryRun(c.DryRun),
tq.WithProgress(c.meter),
tq.WithBatchSize(cfg.BatchSize()),
tq.WithBatchSize(cfg.TransferBatchSize()),
)...)
}

Expand Down
8 changes: 4 additions & 4 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,10 @@ func (c *Configuration) TusTransfersAllowed() bool {
return c.Git.Bool("lfs.tustransfers", false)
}

func (c *Configuration) TransferBatchSize() int {
return c.Git.Int("lfs.transfer.batchSize", 0)
}

func (c *Configuration) FetchIncludePaths() []string {
patterns, _ := c.Git.Get("lfs.fetchinclude")
return tools.CleanPaths(patterns, ",")
Expand Down Expand Up @@ -339,10 +343,6 @@ func (c *Configuration) ForceProgress() bool {
return c.Os.Bool("GIT_LFS_FORCE_PROGRESS", false) || c.Git.Bool("lfs.forceprogress", false)
}

func (c *Configuration) BatchSize() int {
return c.Git.Int("lfs.batchsize", 100)
}

// HookDir returns the location of the hooks owned by this repository. If the
// core.hooksPath configuration variable is supported, we prefer that and expand
// paths appropriately.
Expand Down
10 changes: 10 additions & 0 deletions docs/man/git-lfs-config.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,16 @@ to simple HTTP.
If set to true, this enables resumable uploads of LFS objects through
the tus.io API. Once this feature is finalized, this setting will be
removed, and tus.io uploads will be available for all clients.
* `lfs.transfer.batchSize`
+
The number of objects to download/upload sent in a single batch request
to the LFS server. Default is 100.
+
This value should be changed with caution, as it can have a significant
impact on the performance of the LFS server and the server is free to
return an HTTP 413 status code if this value is too high as the Batch
API specification states.

* `lfs.standalonetransferagent`
+
Allows the specified custom transfer agent to be used directly for
Expand Down
4 changes: 2 additions & 2 deletions lfs/gitfilter_smudge.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ func (f *GitFilter) downloadFile(writer io.Writer, ptr *Pointer, workingfile, me
q := tq.NewTransferQueue(tq.Download, manifest, f.cfg.Remote(),
tq.WithProgressCallback(cb),
tq.RemoteRef(f.RemoteRef()),
tq.WithBatchSize(f.cfg.BatchSize()),
tq.WithBatchSize(f.cfg.TransferBatchSize()),
)
q.Add(filepath.Base(workingfile), mediafile, ptr.Oid, ptr.Size, false, nil)
q.Wait()
Expand Down Expand Up @@ -149,7 +149,7 @@ func (f *GitFilter) downloadFileFallBack(writer io.Writer, ptr *Pointer, working
q := tq.NewTransferQueue(tq.Download, manifest, remote,
tq.WithProgressCallback(cb),
tq.RemoteRef(f.RemoteRef()),
tq.WithBatchSize(f.cfg.BatchSize()),
tq.WithBatchSize(f.cfg.TransferBatchSize()),
)
q.Add(filepath.Base(workingfile), mediafile, ptr.Oid, ptr.Size, false, nil)
q.Wait()
Expand Down
97 changes: 97 additions & 0 deletions t/t-batch-transfer-size.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
#!/usr/bin/env bash

. "$(dirname "$0")/testlib.sh"

begin_test "batch storage upload with small batch size"
(
set -e

reponame="batch-storage-upload-small-batch"
setup_remote_repo "$reponame"
clone_repo "$reponame" batch-storage-repo-upload

contents1="storage-upload-batch-1"
contents2="storage-upload-batch-2"
contents3="storage-upload-batch-3"
oid1="$(calc_oid "$contents1")"
oid2="$(calc_oid "$contents2")"
oid3="$(calc_oid "$contents3")"
printf "%s" "$contents1" > a.dat
printf "%s" "$contents2" > b.dat
printf "%s" "$contents3" > c.dat

git lfs track "*.dat"
git add .gitattributes a.dat b.dat c.dat
git commit -m "initial commit"

git config --local lfs.transfer.batchSize 1

GIT_TRACE=1 git push origin main 2>&1 | tee push.log
if [ "0" -ne "${PIPESTATUS[0]}" ]; then
echo >&2 "fatal: expected \`git push origin main\` to succeed ..."
exit 1
fi

actual_count="$(grep -c "tq: sending batch of size 1" push.log)"
[ "3" = "$actual_count" ]

assert_server_object "$reponame" "$oid1"
assert_server_object "$reponame" "$oid2"
assert_server_object "$reponame" "$oid3"
)
end_test

begin_test "batch storage download with small batch size"
(
set -e

reponame="batch-storage-download-small-batch"
setup_remote_repo "$reponame"
clone_repo "$reponame" batch-storage-repo-download

contents1="storage-download-batch-1"
contents2="storage-download-batch-2"
contents3="storage-download-batch-3"
oid1="$(calc_oid "$contents1")"
oid2="$(calc_oid "$contents2")"
oid3="$(calc_oid "$contents3")"
printf "%s" "$contents1" > a.dat
printf "%s" "$contents2" > b.dat
printf "%s" "$contents3" > c.dat

git lfs track "*.dat"
git add .gitattributes a.dat b.dat c.dat
git commit -m "initial commit"

git push origin main
assert_server_object "$reponame" "$oid1"
assert_server_object "$reponame" "$oid2"
assert_server_object "$reponame" "$oid3"

pushd ..
git \
-c "filter.lfs.process=" \
-c "filter.lfs.smudge=cat" \
-c "filter.lfs.required=false" \
clone "$GITSERVER/$reponame" "$reponame-assert"

cd "$reponame-assert"

git config credential.helper lfstest
git config --local lfs.transfer.batchSize 1

GIT_TRACE=1 git lfs pull origin main 2>&1 | tee pull.log
if [ "0" -ne "${PIPESTATUS[0]}" ]; then
echo >&2 "fatal: expected \`git lfs pull origin main\` to succeed ..."
exit 1
fi

actual_count="$(grep -c "tq: sending batch of size 1" pull.log)"
[ "3" = "$actual_count" ]

assert_local_object "$oid1" "${#contents1}"
assert_local_object "$oid2" "${#contents2}"
assert_local_object "$oid3" "${#contents3}"
popd
)
end_test

0 comments on commit 3161b34

Please sign in to comment.