Hey, thank you very much for this PR and the proposed implementation of an improved transfer protocol!

I think a PR of this size and import deserves a really careful review, so I apologize in advance if it takes me a bit of extra time to read through and try to think through some of the implications.

One initial thought I had, after a first reading of the proposal, is that if I understand correctly it's essentially an expansion of the existing custom transfer protocol in order to permit parallel object transfers. As such, my first inclination would be to try to find a way to expand that protocol directly so that the client and server could negotiate whether they both supported parallel (i.e., "bulk") transfers, and if so use that technique, and otherwise the client would fall back to performing sequential transfers.

For instance, the initiation event from the client might start with something like:

{ "event": "init", "version": 2.0, "operation": "download", "remote": "origin", "concurrent": true, "concurrenttransfers": 3, "paralleltransfers": true }

If the server just responds according to the original custom transfer protocol with an empty JSON structure, then the Git LFS client should assume parallel transfers are not accepted and just use sequential transfers per the original protocol specification.

And if the server responds with an error message, then the client should try again with a "basic" initiation message without the extra version and paralleltransfer fields, to see if the server accepts that and just didn't like the unexpected additional fields.

But if the server responds with, say, { "version": 2.0, "paralleltransfers": true } then the client can begin to make parallel object transfers using an updated and expanded version of the protocol.

That's my first thought, anyway. One thing which I think would be beneficial about this type of scheme is that it's in line with how other client/server communication protocols often work, with clients trying more advanced versions of the protocol first and then falling back to earlier versions (so long as they aren't insecure).

The other benefit I see offhand is that it would mean we probably could avoid introducing a full set of new Git LFS configuration options, since the existing lfs.customTransfer.* settings would largely suffice. I think that's a real advantage, as we should avoid having multiple variations of closely related options if we possibly can.

As a potential next step, how would you feel about starting with just the proposal document (either in this PR or a new one) and only working on that at first? For instance, in PRs like #3290 and #6021, we've worked with the interested parties to develop an "RFC" (Request for Comments) before starting on any code changes. Of course, once there's general agreement on the design and the proposal PR is merged, it usually changes a little as the initial implementation progresses, because inevitably there are details we can't foresee in advance, but those are typically small corrections to the accepted proposal rather than wholesale revisions.

Thank you again so much for taking the time to put together this PR, and I very much look forward to collaborating on the design and development of this new feature!

Hey Cris! Thank you very much for your detailed feedback!

This pull request is indeed to be considered primarily as a proposal.
I added the code to demonstrate a basic MVP implementation and to seek for potential issues to consider in the final implementation.
Also, I am not a strong Go developer at the moment, so I needed to understand Go better and play with it at least for a while. I intentionally moved this demo code aside from the main functionality to simplify the task for the demo.

This functionality should indeed extend custom transfers in the final implementation.
Initialization messages seem like an appropriate place to clarify the supported version of the protocol.
This will avoid the introduction of a duplicate set of configuration options and avoid strange configurations possible in the proposed version (there is a configuration for both transfer protocols).
The only alternative way approaching this i can think of is to limit the supported protocol version via config option to manually enable the new protocol support. This option doesn't provide any improvements i can see.

Some observations I saw during this implementation that I want to highlight:

1. Currently, the Add method of the transfer adapter interface does not provide information that there are no more items in the transfer queue to be processed at this time.
   Since the bulk size is fixed - the bulk adapter implementation starts a timer and monitors idle time, assuming that idle time for a certain period is a sign of the end of the queue and the last bulk should be flushed even if it isnt full sized yet. Timer seems like an unreliable mechanism that may depend on the performance of the machine on which the code is running. It seems that on slow disks or with antivirus intervention, such a timer will either require manual tuning or will trigger prematurely, leading to protocol violations.
2. The existing version of the custom transfer protocol does not support the retries mechanism for custom transfers.
   Although the retry mechanism is implemented at the built-in transfer level, external transfers cannot use it, as it is not included into the protocol.
   This is not such a significant problem, since retry can be implemented within the transfer process itself. However, studying the existing code it seems that implementation on the lfs side will relieve the burden from transfer authors and avoid code duplication in each transfer and this should be included in the protocol.

Regarding the implementation - two possible ways come to mind:

1. Implement support for two versions of the protocol in the existing custom transfer adapter. This is a more straightforward option, but it seems less preferable for now, as it mixes two protocol versions in one adapter.
2. Entrust the custom transfer adapter only with the responsibility of choosing the protocol, and move the implementation of specific protocols to separate nested adapters. This option seems more preferable for now, as it allows moving in iterations and use fragments of already written code.

I will write a full proposal for extending the custom transfer protocol in a separate document.
When we agree on the design - it can be taken out into a separate PR and moved further.

Best regards, Alexander Bogomolets.

This functionality should indeed extend custom transfers in the final implementation.
Initialization messages seem like an appropriate place to clarify the supported version of the protocol.

Excellent, I'm glad you agree! I think it's a good overall design principle for systems to try to use the most efficient/secure/fastest options available, and then step down as necessary if those aren't supported. We can also also provide a configuration option to force the use of the original sequential-transfer design, as some users may want to do that for testing or other purposes.

Regarding some of your useful notes:

the bulk adapter implementation starts a timer and monitors idle time, assuming that idle time for a certain period is a sign of the end of the queue and the last bulk should be flushed even if it isnt full sized yet. Timer seems like an unreliable mechanism that may depend on the performance of the machine on which the code is running.

I'd agree that seems less than ideal. While we certainly need (configurable) idle timeouts for any kind of network connection, we don't want to use them to measure whether objects remain to be transferred.

the retry mechanism is implemented at the built-in transfer level, external transfers cannot use it, as it is not included into the protocol.
it seems that implementation on the lfs side will relieve the burden from transfer authors and avoid code duplication in each transfer and this should be included in the protocol.

I think we'd be interested to see a proposal for this, certainly, and if it can be separated out from the protocol extensions to support parallel transfers, so much the better!

Regarding the implementation - two possible ways come to mind:

1. Implement support for two versions of the protocol in the existing custom transfer adapter. This is a more straightforward option, but it seems less preferable for now, as it mixes two protocol versions in one adapter.
2. Entrust the custom transfer adapter only with the responsibility of choosing the protocol, and move the implementation of specific protocols to separate nested adapters. This option seems more preferable for now, as it allows moving in iterations and use fragments of already written code.

I confess that when I first replied to your PR, I only had something like your option #​1 in mind, with most of the existing sequential transfer process being effectively implemented just as parallel transfers with a maximum parallelism of one. But if it's easier to work incrementally in a series of refactoring steps to get there, using two internal implementations along the way, that's totally understandable.

I will write a full proposal for extending the custom transfer protocol in a separate document.
When we agree on the design - it can be taken out into a separate PR and moved further.

Fantastic, thank you so much!

Hey @chrisd8088 !

Thank you for your carefull review!

I submited a new version for the proposal, that address the review comments.
Though while working on this version i tried to align all the therms into the git lfs space and might overdo a bit.

To keep it simple for you to review the fresh version here is the main outlines:

1. I shapeshifted the therms a bit. Instead of using "bulk" transfers i came up with the idea: essentialy this PR is about the new concurrency model. And the actual change i propose is actually to allow custom transfers to process batches of objects as a unit of work instead of single artifatcs. Renaming bulks to batches align bulkSize configuration parameter to lfs.transfer.batchSize and makes it simpler for custom transfer authors to maintain context. Proposal text is olso updated to support this idea. We indeed do not wish to introduce separate parallel running transfer queues (exceipt maybe for unit testing purposes) and the whole idea is to process the batch of objects as a single unit of work. To do so concrete transfer will definetly require a server side to implement this payload which actually moves to the next point.
2. I added the batch API modification to include some new values to keep server aware about what client is intend to do and be able to override client behaviuor. This part seems incomplete for now and requires carefull review. The basic idea is to confirm the server is aware of the new feature and supports it. For now this is a precaution measure to keep existing lfs server implementations backwards compatible and actually aware that something is changing inside the git lfs API.
3. I introduced custom transfer protocol version as you proposed. The protocol version gets a separate configuration option that makes maintaining backwards compatibility an easier task. The default protocol version is meant to be 1 (even while unspecified) so we do not intriduce any behaviour changes untill user explicitly tells so. Although it seems fine for custom tranfers - it seems that we need to also introduce the new batch API endpoints if previous point is approved during the review cause we have the same backward compatibility issues with existing server implementations.
4. I updated all the message descriptions to be consistent with current git lfs therms where oid is always object identifier, size is always object size in bytes so all new protocol messages use new fields that correspond to this naming schema: bid for batch identifier, totalSize for a summ of all object sizes in bytes and objectsCount for the numer of objects inside the batch.

Not all the comments are addressed for now, i will leave a comments to keep it simple for review.

Best regards, Alexander Bogomoletz.