{"thread":{"id":"46761","subject":"RFC: Design and code of partial clones (now, missing commits and trees OK)","startedAt":"2017-09-15T20:43:52Z","lastAt":"2017-09-29T20:21:35Z","messageCount":21,"participants":["Jonathan Tan","Junio C Hamano","Jeff Hostetler","Michael Haggerty"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"328165","messageId":"20170915134343.3814dc38@twelve2.svl.corp.google.com","threadId":"46761","inReplyTo":null,"subject":"RFC: Design and code of partial clones (now, missing commits and trees OK)","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-09-15T20:43:43Z","receivedAt":"2017-09-15T20:43:52Z","isPatch":false,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"For those interested in partial clones and/or missing objects in repos,\nI've updated my original partialclone patches to not require an explicit\nlist of promises. Fetch/clone still only permits exclusion of blobs, but\nthe infrastructure is there for a local repo to support missing trees\nand commits as well.\n\nThey can be found here:\n\nhttps://github.com/jonathantanmy/git/tree/partialclone2\n\nTo make the number of patches more manageable, I have omitted support\nfor a custom fetching hook (but it can be readded in fetch-object.c),\nand only support promisor packfiles for now (but I don't have any\nobjection to supporting promisor loose objects in the future).\n\nLet me know what you think of the overall approach. In particular, I'm\nstill wondering if there is a better approach than to toggle\n\"fetch_if_missing\" whenever we need lazy fetching (or need to suppress\nit).\n\nAlso, if there any patches that you think might be useful to others, let\nme know and I'll send them to this mailing list for review.\n\nA demo and an overview of the design (also available from that\nrepository's README):\n\nDemo\n====\n\nObtain a repository.\n\n $ make prefix=$HOME/local install\n $ cd $HOME/tmp\n $ git clone https://github.com/git/git\n\nMake it advertise the new feature and allow requests for arbitrary blobs.\n\n $ git -C git config uploadpack.advertiseblobmaxbytes 1\n $ git -C git config uploadpack.allowanysha1inwant 1\n\nPerform the partial clone and check that it is indeed smaller. Specify\n\"file://\" in order to test the partial clone mechanism. (If not, Git will\nperform a local clone, which unselectively copies every object.)\n\n $ git clone --blob-max-bytes=100000 \"file://$(pwd)/git\" git2\n $ git clone \"file://$(pwd)/git\" git3\n $ du -sh git2 git3\n 116M\tgit2\n 129M\tgit3\n\nObserve that the new repo is automatically configured to fetch missing objects\nfrom the original repo. Subsequent fetches will also be partial.\n\n $ cat git2/.git/config\n [core]\n \trepositoryformatversion = 1\n \tfilemode = true\n \tbare = false\n \tlogallrefupdates = true\n [remote \"origin\"]\n \turl = [snip]\n \tfetch = +refs/heads/*:refs/remotes/origin/*\n \tblobmaxbytes = 100000\n [extensions]\n \tpartialclone = origin\n [branch \"master\"]\n \tremote = origin\n \tmerge = refs/heads/master\n\nUnlike in an older version of this code (see the `partialclone` branch), this\nalso works with the HTTP/HTTPS protocols.\n\nDesign\n======\n\nLocal repository layout\n-----------------------\n\nA repository declares its dependence on a *promisor remote* (a remote that\ndeclares that it can serve certain objects when requested) by a repository\nextension \"partialclone\". `extensions.partialclone` must be set to the name of\nthe remote (\"origin\" in the demo above).\n\nA packfile can be annotated as originating from the promisor remote by the\nexistence of a \"(packfile name).promisor\" file with arbitrary contents (similar\nto the \".keep\" file). Whenever a promisor remote sends an object, it declares\nthat it can serve every object directly or indirectly referenced by the sent\nobject.\n\nA promisor packfile is a packfile annotated with the \".promisor\" file. A\npromisor object is an object in a promisor packfile. A promised object is an\nobject directly referenced by a promisor object.\n\n(In the future, we might need to add \".promisor\" support to loose objects.)\n\nConnectivity check and gc\n-------------------------\n\nThe object walk done by the connectivity check (as used by fsck and fetch) stops\nat all promisor objects and promised objects.\n\nThe object walk done by gc also stops at all promisor objects and promised\nobjects. Only non-promisor packfiles are deleted (if pack deletion is\nrequested); promisor packfiles are left alone. This maintains the distinction\nbetween promisor packfiles and non-promisor packfiles. (In the future, we might\nneed to do something more sophisticated with promisor packfiles.)\n\nFetching of promised objects\n----------------------------\n\nWhen `sha1_object_info_extended()` (or similar) is invoked, it will\nautomatically attempt to fetch a missing object from the promisor remote if that\nobject is not in the local repository. For efficiency, no check is made as to\nwhether that object is a promised object or not.\n\nThis automatic fetching can be toggled on and off by the `fetch_if_missing`\nglobal variable, and it is on by default.\n\nThe actual fetch is done through the fetch-pack/upload-pack protocol. Right now,\nthis uses the fact that upload-pack allows blob and tree \"want\"s, and this\nincurs the overhead of the unnecessary ref advertisement. I hope that protocol\nv2 will allow us to declare that blob and tree \"want\"s are allowed, and allow\nthe client to declare that it does not want the ref advertisement. All packfiles\ndownloaded in this way are annotated with \".promisor\".\n\nFetching with `git fetch`\n-------------------------\n\nThe fetch-pack/upload-pack protocol has also been extended to support omission\nof blobs above a certain size. The client only allows this when fetching from\nthe promisor remote, and will annotate any packs received from the promisor\nremote with \".promisor\".\n\n"},{"id":"328346","messageId":"xmqq8thbqlqf.fsf@gitster.mtv.corp.google.com","threadId":"46761","inReplyTo":"20170915134343.3814dc38@twelve2.svl.corp.google.com","subject":"Re: RFC: Design and code of partial clones (now, missing commits and trees OK)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-09-19T05:51:52Z","receivedAt":"2017-09-19T05:52:02Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Tan writes:\n\n> For those interested in partial clones and/or missing objects in repos,\n> I've updated my original partialclone patches to not require an explicit\n> list of promises. Fetch/clone still only permits exclusion of blobs, but\n> the infrastructure is there for a local repo to support missing trees\n> and commits as well.\n> ...\n> Demo\n> ====\n>\n> Obtain a repository.\n>\n> $ make prefix=$HOME/local install\n> $ cd $HOME/tmp\n> $ git clone https://github.com/git/git\n>\n> Make it advertise the new feature and allow requests for arbitrary blobs.\n>\n> $ git -C git config uploadpack.advertiseblobmaxbytes 1\n> $ git -C git config uploadpack.allowanysha1inwant 1\n>\n> Perform the partial clone and check that it is indeed smaller. Specify\n> \"file://\" in order to test the partial clone mechanism. (If not, Git will\n> perform a local clone, which unselectively copies every object.)\n>\n> $ git clone --blob-max-bytes=100000 \"file://$(pwd)/git\" git2\n> $ git clone \"file://$(pwd)/git\" git3\n> $ du -sh git2 git3\n> 116M\tgit2\n> 129M\tgit3\n>\n> Observe that the new repo is automatically configured to fetch missing objects\n> from the original repo. Subsequent fetches will also be partial.\n>\n> $ cat git2/.git/config\n> [core]\n> \trepositoryformatversion = 1\n> \tfilemode = true\n> \tbare = false\n> \tlogallrefupdates = true\n> [remote \"origin\"]\n> \turl = [snip]\n> \tfetch = +refs/heads/*:refs/remotes/origin/*\n> \tblobmaxbytes = 100000\n> [extensions]\n> \tpartialclone = origin\n> [branch \"master\"]\n> \tremote = origin\n> \tmerge = refs/heads/master\n\nThe above sequence of events make quite a lot of sense. And the\nfollowing description of how it is designed (snipped) is clear\nenough (at least to me) to allow me to say that I quite like it.\n\n\n"},{"id":"328582","messageId":"e6259d03-e904-8c57-73b0-2434939fba71@jeffhostetler.com","threadId":"46761","inReplyTo":"20170915134343.3814dc38@twelve2.svl.corp.google.com","subject":"Re: RFC: Design and code of partial clones (now, missing commits and trees OK)","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-09-21T17:57:30Z","receivedAt":"2017-09-21T17:57:39Z","isPatch":false,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"\nThere's a lot in this patch series. I'm still studying it, but here\nare some notes and questions. I'll start with direct responses to\nthe RFC here and follow up in a second email with specific questions\nand comments to keep this from being too long).\n\n\nOn 9/15/2017 4:43 PM, Jonathan Tan wrote:\n> For those interested in partial clones and/or missing objects in repos,\n> I've updated my original partialclone patches to not require an explicit\n> list of promises. Fetch/clone still only permits exclusion of blobs, but\n> the infrastructure is there for a local repo to support missing trees\n> and commits as well.\n> \n> They can be found here:\n> \n> https://github.com/jonathantanmy/git/tree/partialclone2\n> \n> To make the number of patches more manageable, I have omitted support\n> for a custom fetching hook (but it can be readded in fetch-object.c),\n> and only support promisor packfiles for now (but I don't have any\n> objection to supporting promisor loose objects in the future).\n> \n> Let me know what you think of the overall approach. In particular, I'm\n> still wondering if there is a better approach than to toggle\n> \"fetch_if_missing\" whenever we need lazy fetching (or need to suppress\n> it).\n> \n> Also, if there any patches that you think might be useful to others, let\n> me know and I'll send them to this mailing list for review.\n> \n> A demo and an overview of the design (also available from that\n> repository's README):\n> \n> Demo\n> ====\n> \n> Obtain a repository.\n> \n> $ make prefix=$HOME/local install\n> $ cd $HOME/tmp\n> $ git clone https://github.com/git/git\n> \n> Make it advertise the new feature and allow requests for arbitrary blobs.\n> \n> $ git -C git config uploadpack.advertiseblobmaxbytes 1\n> $ git -C git config uploadpack.allowanysha1inwant 1\n> \n> Perform the partial clone and check that it is indeed smaller. Specify\n> \"file://\" in order to test the partial clone mechanism. (If not, Git will\n> perform a local clone, which unselectively copies every object.)\n> \n> $ git clone --blob-max-bytes=100000 \"file://$(pwd)/git\" git2\n> $ git clone \"file://$(pwd)/git\" git3\n> $ du -sh git2 git3\n> 116M\tgit2\n> 129M\tgit3\n> \n> Observe that the new repo is automatically configured to fetch missing objects\n> from the original repo. Subsequent fetches will also be partial.\n> \n> $ cat git2/.git/config\n> [core]\n> \trepositoryformatversion = 1\n> \tfilemode = true\n> \tbare = false\n> \tlogallrefupdates = true\n> [remote \"origin\"]\n> \turl = [snip]\n> \tfetch = +refs/heads/*:refs/remotes/origin/*\n> \tblobmaxbytes = 100000\n> [extensions]\n> \tpartialclone = origin\n> [branch \"master\"]\n> \tremote = origin\n> \tmerge = refs/heads/master\n> \n\nI like that git-clone saves the partial clone settings in the\n.git/config. This should make it easier for subsequent commands to\ndefault to the right settings.\n\nDo we allow a partial-fetch following a regular clone (such that git-fetch\nwould create these config file settings)? This seems like a reasonable\nupgrade path for a user with an existing repo to take advantage of partial\nfetches going forward.\n\nOr do we require that git-fetch only be allowed to do partial-fetches\nafter a partial-clone (so that only clone creates these settings) and\nfetch always does partial-fetches thereafter? It might be useful to\nallow fetch to do a full fetch on top of a partial-clone, but there\nare probably thin-pack issues to work out first.\n\nAlso, there is an assumption here that the user will want to keep\nusing the same filtering settings on subsequent fetches. That's\nprobably fine for now and until we get a chance to try it out for\nreal.\n\n\n> Unlike in an older version of this code (see the `partialclone` branch), this\n> also works with the HTTP/HTTPS protocols.\n> \n> Design\n> ======\n> \n> Local repository layout\n> -----------------------\n> \n> A repository declares its dependence on a *promisor remote* (a remote that\n> declares that it can serve certain objects when requested) by a repository\n> extension \"partialclone\". `extensions.partialclone` must be set to the name of\n> the remote (\"origin\" in the demo above).\n> \n\nDo we allow EXACTLY ONE promisor-remote? That is, can we later add\nanother remote as a promisor-remote? And then do partial-fetches from\neither? Do we need to disallow removing or altering a remote that is\nlisted as a promisor-remote?\n\nI think for now, one promisor-remote is fine. Just asking.\n\nChanging a remote's URL might be fine, but deleting the promisor-remote\nwould put the user in a weird state. We don't need to worry about it\nnow though.\n\n\n> A packfile can be annotated as originating from the promisor remote by the\n> existence of a \"(packfile name).promisor\" file with arbitrary contents (similar\n> to the \".keep\" file). Whenever a promisor remote sends an object, it declares\n> that it can serve every object directly or indirectly referenced by the sent\n> object.\n> \n> A promisor packfile is a packfile annotated with the \".promisor\" file. A\n> promisor object is an object in a promisor packfile. A promised object is an\n> object directly referenced by a promisor object.\n> \n\nI struggled with the terms here a little when looking at the source.\n() A remote responding to a partial-clone is termed a \"promisor-remote\".\n() Packfiles received from a promisor-remote are marked with \".promisor\"\n like \".keep\" names.\n() An object actually contained in such packfiles is called a \"promisor-object\".\n() An object not-present but referenced by one of the above promisor-objects\n is called a \"promised-object\" (aka a \"missing-object\").\n\nI think the similarity of the words \"promisOR\" and \"promisED\" threw me here\nand as I was looking at the code. The code in is_promised() [1] looked like\nit was adding all promisor- and promised-objects to the \"promised\" OIDSET,\nbut I could be mistaken.\n\n[1] https://github.com/jonathantanmy/git/commit/7a9c2d9b6e2fce293817b595dee29a7eede0dddd#diff-5d5d5dc185ef37dc30bb7d9a7ae0c4e8R1960\n\n\nThe contents of the \".promisor\" file are described as arbitrary?\nShould we write the name of the remote (or some other structured data)\ninto this file so that later fetches can find the server? This is more\na question for when we have multiple promisor-remotes and may need to\ndecide who can supply a missing object. Not urgent now though.\n\n\n> (In the future, we might need to add \".promisor\" support to loose objects.)\n>\n\nI assume you mean that a dynamic fetch of a single tree object would be\nunpacked upon receipt (rather than creating a one object packfile) and\nthat we may need to mark it as promisor-object so that missing objects\nthat *IT* references would still follow the promised-object rules.\n\n\n> Connectivity check and gc\n> -------------------------\n> \n> The object walk done by the connectivity check (as used by fsck and fetch) stops\n> at all promisor objects and promised objects.\n>\n\nSo is the assumption that as soon as you touch a promisOR-object you might\nas well stop scanning, because anything it references might be missing?\n\n\nThe code in rev-list.c and revision.c in [2] looks like it will continue\nthru PROMISOR-objects and stop at (still-missing) PROMISED-objects.\n\n[2] https://github.com/jonathantanmy/git/commit/2d7ae2bc780dd056552643e4f5061a0ca7b5b1e5\n\n\n> The object walk done by gc also stops at all promisor objects and promised\n> objects. Only non-promisor packfiles are deleted (if pack deletion is\n> requested); promisor packfiles are left alone. This maintains the distinction\n> between promisor packfiles and non-promisor packfiles. (In the future, we might\n> need to do something more sophisticated with promisor packfiles.)\n>\n\nSeems like we could combine promisor-packfiles -- so, for example, the net\nresult of a complete repack might be one normal- and one promisor-packfile.\n\n\nQuestion: if the initial clone is a partial-clone, we will ONLY have\npromisor-packfiles, right? (Unless they later add a second regular remote.)\nSo fsck and GC will basically stop scanning immediately, right?\nLikewise, repack will basically be disabled until we support repacking\npromisor-packfiles, right? So performance of the resulting repo will\ndeteriorate over time as subsequent partial-fetches pull in more and more\npromisor-packfiles.\n\n\n> Fetching of promised objects\n> ----------------------------\n> \n> When `sha1_object_info_extended()` (or similar) is invoked, it will\n> automatically attempt to fetch a missing object from the promisor remote if that\n> object is not in the local repository. For efficiency, no check is made as to\n> whether that object is a promised object or not.\n>\n\nI don't understand this last statement. A missing-object is assumed to\nbe a promised-object, but we don't check that. By this do you mean that\nis_promised() [1] is only used by commands like fsck and not elsewhere?\n\n\n> This automatic fetching can be toggled on and off by the `fetch_if_missing`\n> global variable, and it is on by default.\n> \n> The actual fetch is done through the fetch-pack/upload-pack protocol. Right now,\n> this uses the fact that upload-pack allows blob and tree \"want\"s, and this\n> incurs the overhead of the unnecessary ref advertisement. I hope that protocol\n> v2 will allow us to declare that blob and tree \"want\"s are allowed, and allow\n> the client to declare that it does not want the ref advertisement. All packfiles\n> downloaded in this way are annotated with \".promisor\".\n>\n\nThe dynamic object fetching in fetch_object() [3] only fetches 1 object from\nthe server. This will spawn a transport process, network connection (and\nauthentication), a packfile download, and index-pack or unpack for each object.\n\nPerhaps this is just preliminary code, but I don't see how you can adapt that\nmodel to be efficient. Even using a long-running background process to fetch\nindividual blobs, it will still be fetching blobs individually. We do need to\nhave dynamic object fetching, but we should try to minimize its use; that is,\nto try have higher-level mechanisms in place to predict and bulk prefetch missing\nobjects.\n\nI guess it depends on how many missing-objects you expect the client to have.\nMy concern here is that we're limiting the design to the \"occasional\" big file\nproblem, rather than the more general scale problem.\n\nAlso, relying on dynamic object fetching also means that operations, like\ncheckout, may now require a network connection. Because we don't know we\nneed an object until we actually need it (and are half-way thru some\noperation that the user thought was safe to do off-line).\n\nI'll talk about this more in my next response to this thread.\n\n[3] https://github.com/jonathantanmy/git/commit/d23be53681549d9ac3c2ecb7b2be9f93d355457b#diff-5c4ccaa3282a2113fc8cafa156b30941R7\n\n\n> Fetching with `git fetch`\n> -------------------------\n> \n> The fetch-pack/upload-pack protocol has also been extended to support omission\n> of blobs above a certain size. The client only allows this when fetching from\n> the promisor remote, and will annotate any packs received from the promisor\n> remote with \".promisor\".\n> \n\nThanks,\nJeff\n"},{"id":"328583","messageId":"5d295ab3-310e-321e-6e88-69484eb9ce8a@jeffhostetler.com","threadId":"46761","inReplyTo":"20170915134343.3814dc38@twelve2.svl.corp.google.com","subject":"Re: RFC: Design and code of partial clones (now, missing commits and trees OK) (part 2/3)","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-09-21T17:59:43Z","receivedAt":"2017-09-21T17:59:48Z","isPatch":false,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"(part 2)\n\nAdditional overall comments on:\nhttps://github.com/jonathantanmy/git/commits/partialclone2\n\n{} I think it would help to split the blob-max-bytes filtering and the\n promisor/promised concepts and discuss them independently.\n\n {} Then we can talk about about the promisor/promised functionality\n independent of any kind of filter. The net-net is that the client\n has missing objects and it doesn't matter what filter criteria\n or mechanism caused that to happened.\n\n {} blob-max-bytes is but one such filter we should have. This might\n be sufficient if the goal is replace LFS (where you rarely ever\n need any given very very large object) and dynamically loading\n them as needed is sufficient and the network round-trip isn't\n too much of a perf penalty.\n\n {} But if we want to do things like a \"sparse-enlistments\" where the\n client only needs a small part of the tree using sparse-checkout.\n For example, only populating 50,000 files from a tree of 3.5M files\n at HEAD, then we need a more general filtering.\n\n {} And as I said above, how we chose to filter should be independent\n of how the client handles promisor/promised objects.\n\n\n{} Also, if we rely strictly on dynamic object fetching to fetch missing\n objects, we are effectively tethered to the server during operations\n (such as checkout) that the user might not think about as requiring\n a network connection. And we are forced to keep the same limitations\n of LFS in that you can't prefetch and go offline (without actually\n checking out to your worktree first). And we can't bulk or parallel\n fetch objects.\n\n\n{} I think it would also help to move the blob-max-bytes calculation out\n of pack-objects.c : add_object_entry() [1]. The current code isolates\n the computation there so that only pack-objects can do the filtering.\n\n Instead, put it in list-objects.c and traverse_commit_list() so that\n pack-objects and rev-list can share it (as Peff suggested [2] in\n response to my first patch series in March).\n\n For example, this would let the client have a pre-checkout hook, use\n rev-list to compute the set of missing objects needed for that commit,\n and pipe that to a command to BULK fetch them from the server BEFORE\n starting the actual checkout. This would allow the savy user to\n manually run a prefetch before going offline.\n\n[1] https://github.com/jonathantanmy/git/commit/68e529484169f4800115c5a32e0904c25ad14bd8#diff-a8d2c9cf879e775d748056cfed48440cR1110\n\n[2] https://public-inbox.org/git/20170309073117.g3br5btsfwntcdpe@sigill.intra.peff.net/\n\n\n{} This also locks us into size-only filtering and makes it more\n difficult to add other filters. In that the add_object_entry()\n code gets called on an object after the traversal has decided\n what to do with it. It would be difficult to add tree-trimming\n at this level, for example.\n\n\n{} An early draft of this type of filtering is here [3]. I hope to push\n up a revised draft of this shortly.\n\n[3] https://public-inbox.org/git/20170713173459.3559-1-git@jeffhostetler.com/\n\n\nThanks,\nJeff\n\n"},{"id":"328584","messageId":"af717446-95bf-c5a5-cd27-aaf20531db0f@jeffhostetler.com","threadId":"46761","inReplyTo":"20170915134343.3814dc38@twelve2.svl.corp.google.com","subject":"Re: RFC: Design and code of partial clones (now, missing commits and trees OK) (part 3)","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-09-21T18:00:40Z","receivedAt":"2017-09-21T18:00:47Z","isPatch":false,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"(part 3)\n\nAdditional overall comments on:\nhttps://github.com/jonathantanmy/git/commits/partialclone2\n\n{} WRT the code in is_promised() [1]\n\n[1] https://github.com/jonathantanmy/git/commit/7a9c2d9b6e2fce293817b595dee29a7eede0dddd#diff-5d5d5dc185ef37dc30bb7d9a7ae0c4e8R1960\n\n {} it looked like it was adding ALL promisor- and promised-objects to\n the \"promised\" OIDSET, rather than just promised-objects, but I\n could be mistaken.\n\n {} Is this iterating over ALL promisor-packfiles?\n\n {} It looked like this was being used by fsck and rev-list. I have\n concerns about how big this OIDSET will get and how it will scale,\n since if we start with a partial-clone all packfiles will be\n promisor-packfiles.\n\n {} When iterating thru a tree object, you add everything that it\n references (everything in that folder). This adds all of the\n child OIDs -- without regard to whether they are new to this\n version of the tree object. (Granted, this is hard to compute.)\n\n My concern is that this will add too many objects to the OIDSET.\n That is, a new tree object (because of a recent change to something\n in that folder) will also have the OIDs of the other *unchanged*\n files which may be present in an earlier non-provisor packfile\n from an earlier commit.\n\n I worry that this will grow the OIDSET to essentially include\n everything. And possibly defeating its own purpose. I could be\n wrong here, but that's my concern.\n\n\n{} I'm not opposed to the .promisor file concept, but I have concerns\n that in practice all packfiles after a partial clone will be\n promisor-packfiles and therefore short-cut during fsck, so fsck\n still won't gain anything.\n\n It would help if there are also non-promisor packfiles present,\n but only for objects referenced by non-promisor packfiles.\n\n But then I also have to wonder whether we can even support non-promisor\n packfiles after starting with a partial clone -- because of the\n properties of received thin-packs on a non-partial fetch.\n \n\nThanks,\nJeff\n\n"},{"id":"328596","messageId":"20170921154214.0d2ac45f@twelve2.svl.corp.google.com","threadId":"46761","inReplyTo":"e6259d03-e904-8c57-73b0-2434939fba71@jeffhostetler.com","subject":"Re: RFC: Design and code of partial clones (now, missing commits and trees OK)","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-09-21T22:42:14Z","receivedAt":"2017-09-21T22:42:26Z","isPatch":false,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"On Thu, 21 Sep 2017 13:57:30 -0400\nJeff Hostetler wrote:\n\n> There's a lot in this patch series. I'm still studying it, but here\n> are some notes and questions. I'll start with direct responses to\n> the RFC here and follow up in a second email with specific questions\n> and comments to keep this from being too long).\n\nOK - thanks for your detailed comments.\n\n> I like that git-clone saves the partial clone settings in the\n> .git/config. This should make it easier for subsequent commands to\n> default to the right settings.\n> \n> Do we allow a partial-fetch following a regular clone (such that\n> git-fetch would create these config file settings)? This seems like\n> a reasonable upgrade path for a user with an existing repo to take\n> advantage of partial fetches going forward.\n\nA \"git-fetch ...options... --save-options\" does not sound unreasonable,\nalthough I would think that (i) partial fetches/clones are useful on\nvery large repositories, and (ii) the fact that you had already cloned\nthis large repository shows that you can handle the disk and network\nload, so partial fetch after non-partial clone doesn't seem very useful.\n\nBut if there is a use case, I think it could work. Although note that\nthe GC in my patch set stops at promisor objects, so if an object\noriginally cloned cannot be reached through a walk (with non-promisor\nintermediate objects only), it might end up GC-ed (which might be fine).\n\n> Or do we require that git-fetch only be allowed to do partial-fetches\n> after a partial-clone (so that only clone creates these settings) and\n> fetch always does partial-fetches thereafter? It might be useful to\n> allow fetch to do a full fetch on top of a partial-clone, but there\n> are probably thin-pack issues to work out first.\n\nAbout the thin-pack issue, I wonder if it is sufficient to turn on\nfetch_if_missing while index-pack is trying to fix the thin pack.\n\nIf the thin-pack issues are worked out, then non-partial fetch after\npartial clone seems doable (all commits from our tip to their tip are\nfetched, as well as all new trees and all new blobs; any trees and blobs\nstill missing are already promised).\n\nThanks for these questions - I am concentrating on repos in which both\nclone and fetch are partial, but it is good to discuss what happens if\nthe user does something else.\n\n> Also, there is an assumption here that the user will want to keep\n> using the same filtering settings on subsequent fetches. That's\n> probably fine for now and until we get a chance to try it out for\n> real.\n\nYes. Having said that, the fetching of missing objects does not take\ninto account the filter at all, so the filter can be easily changed.\n\n> Do we allow EXACTLY ONE promisor-remote? That is, can we later add\n> another remote as a promisor-remote? And then do partial-fetches from\n> either?\n\nYes, exactly one (because we need to know which remote to fetch missing\nobjects from, and which remote to allow partial fetches from). But the\npromisor remote can be switched to another.\n\n> Do we need to disallow removing or altering a remote that is\n> listed as a promisor-remote?\n\nPerhaps, although I think that right now configs are checked when we run\nthe command using the config, not when we run \"git config\".\n\n> I think for now, one promisor-remote is fine. Just asking.\n> \n> Changing a remote's URL might be fine, but deleting the\n> promisor-remote would put the user in a weird state. We don't need\n> to worry about it now though.\n\nAgreed.\n\n> I struggled with the terms here a little when looking at the source.\n> () A remote responding to a partial-clone is termed a\n> \"promisor-remote\". () Packfiles received from a promisor-remote are\n> marked with \".promisor\" like \".keep\" names.\n> () An object actually contained in such packfiles is called a\n> \"promisor-object\". () An object not-present but referenced by one of\n> the above promisor-objects is called a \"promised-object\" (aka a\n> \"missing-object\").\n> \n> I think the similarity of the words \"promisOR\" and \"promisED\" threw\n> me here and as I was looking at the code. The code in is_promised()\n> [1] looked like it was adding all promisor- and promised-objects to\n> the \"promised\" OIDSET, but I could be mistaken.\n> \n> [1]\n> https://github.com/jonathantanmy/git/commit/7a9c2d9b6e2fce293817b595dee29a7eede0dddd#diff-5d5d5dc185ef37dc30bb7d9a7ae0c4e8R1960\n\nI was struggling a bit with the terminology, true.\n\nRight now I'm thinking of:\n - promisor remote (as you defined)\n - promisor packfile (as you defined)\n - promisor object is an object known to belong to the promisor (whether\n because we have it in a promisor packfile or because it is referenced\n by an object in a promisor packfile)\n\nThis might eliminate \"promise(d)\", and thus eliminate the confusion\nbetween \"promised\" and \"promisor\", but I haven't done an exhaustive\nsearch.\n\n> The contents of the \".promisor\" file are described as arbitrary?\n> Should we write the name of the remote (or some other structured data)\n> into this file so that later fetches can find the server? This is\n> more a question for when we have multiple promisor-remotes and may\n> need to decide who can supply a missing object. Not urgent now\n> though.\n\nYeah, I don't have a use for it right now. Although it might be a good\nidea to make it future-proof, by say, declaring it to be lines of\nkey-value pairs (with comments)...but then again, we might need to\nforbid earlier tools from working on such modern repositories, so we\nwould need a repository extension (or something similar), and we can\nupgrade the format then.\n\n> I assume you mean that a dynamic fetch of a single tree object would\n> be unpacked upon receipt (rather than creating a one object packfile)\n> and that we may need to mark it as promisor-object so that missing\n> objects that *IT* references would still follow the promised-object\n> rules.\n\nYes, that's right. (And that it would not participate in GC.)\n\n> So is the assumption that as soon as you touch a promisOR-object you\n> might as well stop scanning, because anything it references might be\n> missing?\n> \n> \n> The code in rev-list.c and revision.c in [2] looks like it will\n> continue thru PROMISOR-objects and stop at (still-missing)\n> PROMISED-objects.\n> \n> [2]\n> https://github.com/jonathantanmy/git/commit/2d7ae2bc780dd056552643e4f5061a0ca7b5b1e5\n\nThe invocation of for_each_packed_object() in prepare_revision_walk()\nshould mark all objects in promisor packs as uninteresting, so the\ntraversal should stop there (unless I'm misunderstanding something).\n\n> > The object walk done by gc also stops at all promisor objects and\n> > promised objects. Only non-promisor packfiles are deleted (if pack\n> > deletion is requested); promisor packfiles are left alone. This\n> > maintains the distinction between promisor packfiles and\n> > non-promisor packfiles. (In the future, we might need to do\n> > something more sophisticated with promisor packfiles.)\n> >\n> \n> Seems like we could combine promisor-packfiles -- so, for example,\n> the net result of a complete repack might be one normal- and one\n> promisor-packfile.\n\nYes, that is a good next step. (I'm aiming for a minimum viable product\nhere so I didn't include that.)\n\n> Question: if the initial clone is a partial-clone, we will ONLY have\n> promisor-packfiles, right? (Unless they later add a second regular\n> remote.) So fsck and GC will basically stop scanning immediately,\n> right? Likewise, repack will basically be disabled until we support\n> repacking promisor-packfiles, right? So performance of the resulting\n> repo will deteriorate over time as subsequent partial-fetches pull in\n> more and more promisor-packfiles.\n\nFiles locally created are non-promisor objects, and will become\nnon-promisor packfiles when GC/repack is run.\n\nAs for performance, you are right. We'll need to make a way to combine\npromisor packfiles after this.\n\n> > Fetching of promised objects\n> > ----------------------------\n> > \n> > When `sha1_object_info_extended()` (or similar) is invoked, it will\n> > automatically attempt to fetch a missing object from the promisor\n> > remote if that object is not in the local repository. For\n> > efficiency, no check is made as to whether that object is a\n> > promised object or not.\n> >\n> \n> I don't understand this last statement. A missing-object is assumed\n> to be a promised-object, but we don't check that. By this do you\n> mean that is_promised() [1] is only used by commands like fsck and\n> not elsewhere?\n\nSorry, I meant not checked in sha1_object_info_extended(). Yes, fsck and\ngc will check them.\n\n> The dynamic object fetching in fetch_object() [3] only fetches 1\n> object from the server. This will spawn a transport process, network\n> connection (and authentication), a packfile download, and index-pack\n> or unpack for each object.\n> \n> Perhaps this is just preliminary code, but I don't see how you can\n> adapt that model to be efficient. Even using a long-running\n> background process to fetch individual blobs, it will still be\n> fetching blobs individually. We do need to have dynamic object\n> fetching, but we should try to minimize its use; that is, to try have\n> higher-level mechanisms in place to predict and bulk prefetch missing\n> objects.\n\nMy plan is to have batch fetching. We would need to extend this command\nby command, for example, my patch for batched checkout here:\n\nhttps://github.com/jonathantanmy/git/commit/14f07d3f06bc3a1a2c9bca85adc8c42b230b9143\n\n(I didn't update this for my newest patch set because I didn't figure\nout a good way to deal with the possibility of missing trees, but\nsomething like this could probably be done - for example, by only\nbatching blobs, and whenever we need to fetch a tree, automatically\nfetch all objects to a certain depth, for example.)\n\nServer incompatibility is not a problem because the server already\nsupports multiple objects per request. (Well, except the fetching trees\nto a certain depth part.)\n\n> I guess it depends on how many missing-objects you expect the client\n> to have. My concern here is that we're limiting the design to the\n> \"occasional\" big file problem, rather than the more general scale\n> problem.\n\nDo you have a specific situation in mind?\n\n> Also, relying on dynamic object fetching also means that operations,\n> like checkout, may now require a network connection. Because we\n> don't know we need an object until we actually need it (and are\n> half-way thru some operation that the user thought was safe to do\n> off-line).\n\nYes, but I think this is expected when we're dealing with partial\nclones.\n\n> Thanks,\n> Jeff\n\nThanks.\n"},{"id":"328597","messageId":"20170921155150.1d57d89e@twelve2.svl.corp.google.com","threadId":"46761","inReplyTo":"5d295ab3-310e-321e-6e88-69484eb9ce8a@jeffhostetler.com","subject":"Re: RFC: Design and code of partial clones (now, missing commits and trees OK) (part 2/3)","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-09-21T22:51:50Z","receivedAt":"2017-09-21T22:51:59Z","isPatch":false,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"On Thu, 21 Sep 2017 13:59:43 -0400\nJeff Hostetler wrote:\n\n> (part 2)\n> \n> Additional overall comments on:\n> https://github.com/jonathantanmy/git/commits/partialclone2\n> \n> {} I think it would help to split the blob-max-bytes filtering and the\n> promisor/promised concepts and discuss them independently.\n> \n> {} Then we can talk about about the promisor/promised\n> functionality independent of any kind of filter. The net-net is that\n> the client has missing objects and it doesn't matter what filter\n> criteria or mechanism caused that to happened.\n> \n> {} blob-max-bytes is but one such filter we should have. This\n> might be sufficient if the goal is replace LFS (where you rarely ever\n> need any given very very large object) and dynamically loading\n> them as needed is sufficient and the network round-trip isn't\n> too much of a perf penalty.\n> \n> {} But if we want to do things like a \"sparse-enlistments\" where\n> the client only needs a small part of the tree using sparse-checkout.\n> For example, only populating 50,000 files from a tree of 3.5M\n> files at HEAD, then we need a more general filtering.\n> \n> {} And as I said above, how we chose to filter should be\n> independent of how the client handles promisor/promised objects.\n\nI agree that they are independent. (I put everything together so that\npeople could see how they work together, but they can be changed\nindependently of each other.)\n\n> {} Also, if we rely strictly on dynamic object fetching to fetch\n> missing objects, we are effectively tethered to the server during\n> operations (such as checkout) that the user might not think about as\n> requiring a network connection. And we are forced to keep the same\n> limitations of LFS in that you can't prefetch and go offline (without\n> actually checking out to your worktree first). And we can't bulk or\n> parallel fetch objects.\n\nI don't think dynamic object fetching precludes any other more optimized\nway of fetching or prefetching - I implemented dynamic object fetching\nfirst so that we would have a fallback, but the others definitely can be\nimplemented too.\n\n> {} I think it would also help to move the blob-max-bytes calculation\n> out of pack-objects.c : add_object_entry() [1]. The current code\n> isolates the computation there so that only pack-objects can do the\n> filtering.\n> \n> Instead, put it in list-objects.c and traverse_commit_list() so\n> that pack-objects and rev-list can share it (as Peff suggested [2] in\n> response to my first patch series in March).\n> \n> For example, this would let the client have a pre-checkout hook,\n> use rev-list to compute the set of missing objects needed for that\n> commit, and pipe that to a command to BULK fetch them from the server\n> BEFORE starting the actual checkout. This would allow the savy user\n> to manually run a prefetch before going offline.\n> \n> [1]\n> https://github.com/jonathantanmy/git/commit/68e529484169f4800115c5a32e0904c25ad14bd8#diff-a8d2c9cf879e775d748056cfed48440cR1110\n> \n> [2]\n> https://public-inbox.org/git/20170309073117.g3br5btsfwntcdpe@sigill.intra.peff.net/\n\nIn your specific example, how would rev-list know, on the client, to\ninclude (or exclude) a large blob in its output if it does not have it,\nand thus does not know its size?\n\nMy reason for including it in pack-objects.c is because I only needed it\nthere and it is much simpler, but I agree that if it can be used\nelsewhere, we can put it in a more general place.\n\n> {} This also locks us into size-only filtering and makes it more\n> difficult to add other filters. In that the add_object_entry()\n> code gets called on an object after the traversal has decided\n> what to do with it. It would be difficult to add tree-trimming\n> at this level, for example.\n\nThat is true.\n\n> {} An early draft of this type of filtering is here [3]. I hope to\n> push up a revised draft of this shortly.\n> \n> [3]\n> https://public-inbox.org/git/20170713173459.3559-1-git@jeffhostetler.com/\n\nOK - I'll take a look when that is done (I think I commented on an\nearlier version on that).\n"},{"id":"328598","messageId":"20170921160416.1c4c6e2c@twelve2.svl.corp.google.com","threadId":"46761","inReplyTo":"af717446-95bf-c5a5-cd27-aaf20531db0f@jeffhostetler.com","subject":"Re: RFC: Design and code of partial clones (now, missing commits and trees OK) (part 3)","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-09-21T23:04:16Z","receivedAt":"2017-09-21T23:04:27Z","isPatch":false,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"On Thu, 21 Sep 2017 14:00:40 -0400\nJeff Hostetler wrote:\n\n> (part 3)\n> \n> Additional overall comments on:\n> https://github.com/jonathantanmy/git/commits/partialclone2\n> \n> {} WRT the code in is_promised() [1]\n> \n> [1]\n> https://github.com/jonathantanmy/git/commit/7a9c2d9b6e2fce293817b595dee29a7eede0dddd#diff-5d5d5dc185ef37dc30bb7d9a7ae0c4e8R1960\n> \n> {} it looked like it was adding ALL promisor- and\n> promised-objects to the \"promised\" OIDSET, rather than just\n> promised-objects, but I could be mistaken.\n\nAs far as I can tell, it is just adding the promised objects (some of\nwhich may also be promisor objects). If you're saying that you expected\nit to add the promisor objects as well, that might be a reasonable\nexpectation...I'm thinking of doing that.\n\n> {} Is this iterating over ALL promisor-packfiles?\n\nYes.\n\n> {} It looked like this was being used by fsck and rev-list. I\n> have concerns about how big this OIDSET will get and how it will\n> scale, since if we start with a partial-clone all packfiles will be\n> promisor-packfiles.\n\nIt's true that scaling is an issue. I'm not sure if omitting the oidset\nwill solve anything, though - as it is, Git maintains an object hash and\nadds to it quite liberally.\n\nOne thing that might help is some sort of flushing of objects in\npromisor packfiles from the local repository - that way, the oidset\nwon't be so large.\n\n> \n> {} When iterating thru a tree object, you add everything that it\n> references (everything in that folder). This adds all of the\n> child OIDs -- without regard to whether they are new to this\n> version of the tree object. (Granted, this is hard to compute.)\n\nThe oidset will deduplicate OIDs.\n\n> My concern is that this will add too many objects to the\n> OIDSET. That is, a new tree object (because of a recent change to\n> something in that folder) will also have the OIDs of the other\n> *unchanged* files which may be present in an earlier non-provisor\n> packfile from an earlier commit.\n> \n> I worry that this will grow the OIDSET to essentially include\n> everything. And possibly defeating its own purpose. I could\n> be wrong here, but that's my concern.\n\nSame answer as above (about flushing of objects in promisor packfiles).\n\n> {} I'm not opposed to the .promisor file concept, but I have concerns\n> that in practice all packfiles after a partial clone will be\n> promisor-packfiles and therefore short-cut during fsck, so fsck\n> still won't gain anything.\n> \n> It would help if there are also non-promisor packfiles present,\n> but only for objects referenced by non-promisor packfiles.\n> \n> But then I also have to wonder whether we can even support\n> non-promisor packfiles after starting with a partial clone -- because\n> of the properties of received thin-packs on a non-partial fetch.\n\nSame reply as to your other e-mail - locally created objects are not in\npromisor packfiles. (Or were you thinking of a situation where locally\ncreated objects are immediately uploaded to the promisor remote, thus\nmaking them promisor objects too?)\n"},{"id":"328685","messageId":"af084ada-4607-53e8-c6bb-f5bf13bdc153@jeffhostetler.com","threadId":"46761","inReplyTo":"20170921154214.0d2ac45f@twelve2.svl.corp.google.com","subject":"Re: RFC: Design and code of partial clones (now, missing commits and trees OK)","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-09-22T21:02:11Z","receivedAt":"2017-09-22T21:02:33Z","isPatch":false,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"\n\nOn 9/21/2017 6:42 PM, Jonathan Tan wrote:\n> On Thu, 21 Sep 2017 13:57:30 -0400\n> Jeff Hostetler wrote:\n> \n>> There's a lot in this patch series. I'm still studying it, but here\n>> are some notes and questions. I'll start with direct responses to\n>> the RFC here and follow up in a second email with specific questions\n>> and comments to keep this from being too long).\n> \n> OK - thanks for your detailed comments.\n> \n>> I like that git-clone saves the partial clone settings in the\n>> .git/config. This should make it easier for subsequent commands to\n>> default to the right settings.\n>>\n>> Do we allow a partial-fetch following a regular clone (such that\n>> git-fetch would create these config file settings)? This seems like\n>> a reasonable upgrade path for a user with an existing repo to take\n>> advantage of partial fetches going forward.\n> \n> A \"git-fetch ...options... --save-options\" does not sound unreasonable,\n> although I would think that (i) partial fetches/clones are useful on\n> very large repositories, and (ii) the fact that you had already cloned\n> this large repository shows that you can handle the disk and network\n> load, so partial fetch after non-partial clone doesn't seem very useful.\n> \n> But if there is a use case, I think it could work. Although note that\n> the GC in my patch set stops at promisor objects, so if an object\n> originally cloned cannot be reached through a walk (with non-promisor\n> intermediate objects only), it might end up GC-ed (which might be fine).\n> \n>> Or do we require that git-fetch only be allowed to do partial-fetches\n>> after a partial-clone (so that only clone creates these settings) and\n>> fetch always does partial-fetches thereafter? It might be useful to\n>> allow fetch to do a full fetch on top of a partial-clone, but there\n>> are probably thin-pack issues to work out first.\n> \n> About the thin-pack issue, I wonder if it is sufficient to turn on\n> fetch_if_missing while index-pack is trying to fix the thin pack.\n> \n> If the thin-pack issues are worked out, then non-partial fetch after\n> partial clone seems doable (all commits from our tip to their tip are\n> fetched, as well as all new trees and all new blobs; any trees and blobs\n> still missing are already promised).\n\nAgreed. If we get a thin-pack and there are missing objects from the\ncommits in the edge set, we wouldn't be able to fix the newly received\nobjects without demand loading the object they are relative to. Perhaps\nwe can use my filter/prefetch concept on the edge set to bulk fetch\nthem. (this is a bit of a SWAG.) turning on the dynamic fetch would\nbe a first step (and may be sufficient).\n\n> \n> Thanks for these questions - I am concentrating on repos in which both\n> clone and fetch are partial, but it is good to discuss what happens if\n> the user does something else.\n> \n>> Also, there is an assumption here that the user will want to keep\n>> using the same filtering settings on subsequent fetches. That's\n>> probably fine for now and until we get a chance to try it out for\n>> real.\n> \n> Yes. Having said that, the fetching of missing objects does not take\n> into account the filter at all, so the filter can be easily changed.\n> \n>> Do we allow EXACTLY ONE promisor-remote? That is, can we later add\n>> another remote as a promisor-remote? And then do partial-fetches from\n>> either?\n> \n> Yes, exactly one (because we need to know which remote to fetch missing\n> objects from, and which remote to allow partial fetches from). But the\n> promisor remote can be switched to another.\n> \n>> Do we need to disallow removing or altering a remote that is\n>> listed as a promisor-remote?\n> \n> Perhaps, although I think that right now configs are checked when we run\n> the command using the config, not when we run \"git config\".\n> \n>> I think for now, one promisor-remote is fine. Just asking.\n>>\n>> Changing a remote's URL might be fine, but deleting the\n>> promisor-remote would put the user in a weird state. We don't need\n>> to worry about it now though.\n> \n> Agreed.\n> \n>> I struggled with the terms here a little when looking at the source.\n>> () A remote responding to a partial-clone is termed a\n>> \"promisor-remote\". () Packfiles received from a promisor-remote are\n>> marked with \".promisor\" like \".keep\" names.\n>> () An object actually contained in such packfiles is called a\n>> \"promisor-object\". () An object not-present but referenced by one of\n>> the above promisor-objects is called a \"promised-object\" (aka a\n>> \"missing-object\").\n>>\n>> I think the similarity of the words \"promisOR\" and \"promisED\" threw\n>> me here and as I was looking at the code. The code in is_promised()\n>> [1] looked like it was adding all promisor- and promised-objects to\n>> the \"promised\" OIDSET, but I could be mistaken.\n>>\n>> [1]\n>> https://github.com/jonathantanmy/git/commit/7a9c2d9b6e2fce293817b595dee29a7eede0dddd#diff-5d5d5dc185ef37dc30bb7d9a7ae0c4e8R1960\n> \n> I was struggling a bit with the terminology, true.\n> \n> Right now I'm thinking of:\n> - promisor remote (as you defined)\n> - promisor packfile (as you defined)\n> - promisor object is an object known to belong to the promisor (whether\n> because we have it in a promisor packfile or because it is referenced\n> by an object in a promisor packfile)\n> \n> This might eliminate \"promise(d)\", and thus eliminate the confusion\n> between \"promised\" and \"promisor\", but I haven't done an exhaustive\n> search.\n> \n\nmaybe just call the \"promised\" ones \"missing\".\n\n>> The contents of the \".promisor\" file are described as arbitrary?\n>> Should we write the name of the remote (or some other structured data)\n>> into this file so that later fetches can find the server? This is\n>> more a question for when we have multiple promisor-remotes and may\n>> need to decide who can supply a missing object. Not urgent now\n>> though.\n> \n> Yeah, I don't have a use for it right now. Although it might be a good\n> idea to make it future-proof, by say, declaring it to be lines of\n> key-value pairs (with comments)...but then again, we might need to\n> forbid earlier tools from working on such modern repositories, so we\n> would need a repository extension (or something similar), and we can\n> upgrade the format then.\n> \n>> I assume you mean that a dynamic fetch of a single tree object would\n>> be unpacked upon receipt (rather than creating a one object packfile)\n>> and that we may need to mark it as promisor-object so that missing\n>> objects that *IT* references would still follow the promised-object\n>> rules.\n> \n> Yes, that's right. (And that it would not participate in GC.)\n> \n>> So is the assumption that as soon as you touch a promisOR-object you\n>> might as well stop scanning, because anything it references might be\n>> missing?\n>>\n>>\n>> The code in rev-list.c and revision.c in [2] looks like it will\n>> continue thru PROMISOR-objects and stop at (still-missing)\n>> PROMISED-objects.\n>>\n>> [2]\n>> https://github.com/jonathantanmy/git/commit/2d7ae2bc780dd056552643e4f5061a0ca7b5b1e5\n> \n> The invocation of for_each_packed_object() in prepare_revision_walk()\n> should mark all objects in promisor packs as uninteresting, so the\n> traversal should stop there (unless I'm misunderstanding something).\n> \n>>> The object walk done by gc also stops at all promisor objects and\n>>> promised objects. Only non-promisor packfiles are deleted (if pack\n>>> deletion is requested); promisor packfiles are left alone. This\n>>> maintains the distinction between promisor packfiles and\n>>> non-promisor packfiles. (In the future, we might need to do\n>>> something more sophisticated with promisor packfiles.)\n>>>\n>>\n>> Seems like we could combine promisor-packfiles -- so, for example,\n>> the net result of a complete repack might be one normal- and one\n>> promisor-packfile.\n> \n> Yes, that is a good next step. (I'm aiming for a minimum viable product\n> here so I didn't include that.)\n> \n>> Question: if the initial clone is a partial-clone, we will ONLY have\n>> promisor-packfiles, right? (Unless they later add a second regular\n>> remote.) So fsck and GC will basically stop scanning immediately,\n>> right? Likewise, repack will basically be disabled until we support\n>> repacking promisor-packfiles, right? So performance of the resulting\n>> repo will deteriorate over time as subsequent partial-fetches pull in\n>> more and more promisor-packfiles.\n> \n> Files locally created are non-promisor objects, and will become\n> non-promisor packfiles when GC/repack is run.\n\ni forgot about locally created objects.\n\n> \n> As for performance, you are right. We'll need to make a way to combine\n> promisor packfiles after this.\n> \n>>> Fetching of promised objects\n>>> ----------------------------\n>>>\n>>> When `sha1_object_info_extended()` (or similar) is invoked, it will\n>>> automatically attempt to fetch a missing object from the promisor\n>>> remote if that object is not in the local repository. For\n>>> efficiency, no check is made as to whether that object is a\n>>> promised object or not.\n>>>\n>>\n>> I don't understand this last statement. A missing-object is assumed\n>> to be a promised-object, but we don't check that. By this do you\n>> mean that is_promised() [1] is only used by commands like fsck and\n>> not elsewhere?\n> \n> Sorry, I meant not checked in sha1_object_info_extended(). Yes, fsck and\n> gc will check them.\n> \n>> The dynamic object fetching in fetch_object() [3] only fetches 1\n>> object from the server. This will spawn a transport process, network\n>> connection (and authentication), a packfile download, and index-pack\n>> or unpack for each object.\n>>\n>> Perhaps this is just preliminary code, but I don't see how you can\n>> adapt that model to be efficient. Even using a long-running\n>> background process to fetch individual blobs, it will still be\n>> fetching blobs individually. We do need to have dynamic object\n>> fetching, but we should try to minimize its use; that is, to try have\n>> higher-level mechanisms in place to predict and bulk prefetch missing\n>> objects.\n> \n> My plan is to have batch fetching. We would need to extend this command\n> by command, for example, my patch for batched checkout here:\n> \n> https://github.com/jonathantanmy/git/commit/14f07d3f06bc3a1a2c9bca85adc8c42b230b9143\n> \n> (I didn't update this for my newest patch set because I didn't figure\n> out a good way to deal with the possibility of missing trees, but\n> something like this could probably be done - for example, by only\n> batching blobs, and whenever we need to fetch a tree, automatically\n> fetch all objects to a certain depth, for example.)\n> \n> Server incompatibility is not a problem because the server already\n> supports multiple objects per request. (Well, except the fetching trees\n> to a certain depth part.)\n> \n\nHere I think we plug in my filter mechanism and hook it to a pre-fetch\ncommand. Then we have a pre-checkout hook or something that drives\nthis. We may need a similar pre-merge hook (to get both tips and the\nmerge base).\n\n>> I guess it depends on how many missing-objects you expect the client\n>> to have. My concern here is that we're limiting the design to the\n>> \"occasional\" big file problem, rather than the more general scale\n>> problem.\n> \n> Do you have a specific situation in mind?\n> \n\nI have would like to be able do sparse-enlistments in the Windows\nsource tree. (3.5M files at HEAD.) Most developers only need a small\nfeature area (a device driver or file system or whatever) and not the\nwhole tree. A typical Windows developer may have only 30-50K files\npopulated. If we can synchronize on a sparse-checkout spec and use\nthat for both the checkout and the clone/fetch, then we can bulk fetch\nthe blobs that they'll actually need. GVFS can hydrate the files as\nthey touch them, but I can use this to pre-fetch the needed blobs in\nbulk, rather than faulting and fetching them one-by-one.\n\nSo, my usage may have >95% of the ODB be missing blobs. Contrast that\nwith the occasional large blob / LFS usage where you may have <5% of\nthe ODB as large objects (by count of OIDs not disk usage).\n\n\n>> Also, relying on dynamic object fetching also means that operations,\n>> like checkout, may now require a network connection. Because we\n>> don't know we need an object until we actually need it (and are\n>> half-way thru some operation that the user thought was safe to do\n>> off-line).\n> \n> Yes, but I think this is expected when we're dealing with partial\n> clones.\n\nRight, but if we could pre-fetch, we could make checkout a little\nless unpredictable.\n\n> \n>> Thanks,\n>> Jeff\n> \n> Thanks.\n> \n\nThanks\nJeff\n"},{"id":"328687","messageId":"5b251b02-fe00-3d99-8ae2-d6df0193ebe0@jeffhostetler.com","threadId":"46761","inReplyTo":"20170921155150.1d57d89e@twelve2.svl.corp.google.com","subject":"Re: RFC: Design and code of partial clones (now, missing commits and trees OK) (part 2/3)","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-09-22T21:19:50Z","receivedAt":"2017-09-22T21:19:56Z","isPatch":false,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"\n\nOn 9/21/2017 6:51 PM, Jonathan Tan wrote:\n> On Thu, 21 Sep 2017 13:59:43 -0400\n> Jeff Hostetler wrote:\n> \n>> (part 2)\n>>\n>> Additional overall comments on:\n>> https://github.com/jonathantanmy/git/commits/partialclone2\n>>\n>> {} I think it would help to split the blob-max-bytes filtering and the\n>> promisor/promised concepts and discuss them independently.\n>>\n>> {} Then we can talk about about the promisor/promised\n>> functionality independent of any kind of filter. The net-net is that\n>> the client has missing objects and it doesn't matter what filter\n>> criteria or mechanism caused that to happened.\n>>\n>> {} blob-max-bytes is but one such filter we should have. This\n>> might be sufficient if the goal is replace LFS (where you rarely ever\n>> need any given very very large object) and dynamically loading\n>> them as needed is sufficient and the network round-trip isn't\n>> too much of a perf penalty.\n>>\n>> {} But if we want to do things like a \"sparse-enlistments\" where\n>> the client only needs a small part of the tree using sparse-checkout.\n>> For example, only populating 50,000 files from a tree of 3.5M\n>> files at HEAD, then we need a more general filtering.\n>>\n>> {} And as I said above, how we chose to filter should be\n>> independent of how the client handles promisor/promised objects.\n> \n> I agree that they are independent. (I put everything together so that\n> people could see how they work together, but they can be changed\n> independently of each other.)\n> \n>> {} Also, if we rely strictly on dynamic object fetching to fetch\n>> missing objects, we are effectively tethered to the server during\n>> operations (such as checkout) that the user might not think about as\n>> requiring a network connection. And we are forced to keep the same\n>> limitations of LFS in that you can't prefetch and go offline (without\n>> actually checking out to your worktree first). And we can't bulk or\n>> parallel fetch objects.\n> \n> I don't think dynamic object fetching precludes any other more optimized\n> way of fetching or prefetching - I implemented dynamic object fetching\n> first so that we would have a fallback, but the others definitely can be\n> implemented too.\n\nyes, we need that as a fallback/default for the odd cases where we\ncan't predict perfectly. Like during a blame or history or a merge.\n\nI didn't mean to say we didn't need it, but rather that we should\ntry to minimize it.\n\n> \n>> {} I think it would also help to move the blob-max-bytes calculation\n>> out of pack-objects.c : add_object_entry() [1]. The current code\n>> isolates the computation there so that only pack-objects can do the\n>> filtering.\n>>\n>> Instead, put it in list-objects.c and traverse_commit_list() so\n>> that pack-objects and rev-list can share it (as Peff suggested [2] in\n>> response to my first patch series in March).\n>>\n>> For example, this would let the client have a pre-checkout hook,\n>> use rev-list to compute the set of missing objects needed for that\n>> commit, and pipe that to a command to BULK fetch them from the server\n>> BEFORE starting the actual checkout. This would allow the savy user\n>> to manually run a prefetch before going offline.\n>>\n>> [1]\n>> https://github.com/jonathantanmy/git/commit/68e529484169f4800115c5a32e0904c25ad14bd8#diff-a8d2c9cf879e775d748056cfed48440cR1110\n>>\n>> [2]\n>> https://public-inbox.org/git/20170309073117.g3br5btsfwntcdpe@sigill.intra.peff.net/\n> \n> In your specific example, how would rev-list know, on the client, to\n> include (or exclude) a large blob in its output if it does not have it,\n> and thus does not know its size?\n> \n\nThe client doesn't have the size. It just knows it is missing and it\nneeds it. It doesn't matter why it is missing. (But I guess the client\ncould assume it is because it is large.)\n\nSo rev-list on the client could filter the objects it has by size.\n\nI added that to rev-list primarily to demonstrate and debug the\nfiltering concept (it's easier than playing with packfiles). But\nit can be used to drive client-side queries and bulk requests.\n\n> My reason for including it in pack-objects.c is because I only needed it\n> there and it is much simpler, but I agree that if it can be used\n> elsewhere, we can put it in a more general place.\n> \n>> {} This also locks us into size-only filtering and makes it more\n>> difficult to add other filters. In that the add_object_entry()\n>> code gets called on an object after the traversal has decided\n>> what to do with it. It would be difficult to add tree-trimming\n>> at this level, for example.\n> \n> That is true.\n> \n>> {} An early draft of this type of filtering is here [3]. I hope to\n>> push up a revised draft of this shortly.\n>>\n>> [3]\n>> https://public-inbox.org/git/20170713173459.3559-1-git@jeffhostetler.com/\n> \n> OK - I'll take a look when that is done (I think I commented on an\n> earlier version on that).\n> \n\nFYI I just posted my RFC this afternoon.\nhttps://public-inbox.org/git/20170922204211.GA24036@google.com/T/\n\n\nThanks\nJeff\n\n"},{"id":"328690","messageId":"7977bab0-09c3-0e43-4d6f-f2bf87a3fd9e@jeffhostetler.com","threadId":"46761","inReplyTo":"20170921160416.1c4c6e2c@twelve2.svl.corp.google.com","subject":"Re: RFC: Design and code of partial clones (now, missing commits and trees OK) (part 3)","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-09-22T21:32:00Z","receivedAt":"2017-09-22T21:32:13Z","isPatch":false,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"\n\nOn 9/21/2017 7:04 PM, Jonathan Tan wrote:\n> On Thu, 21 Sep 2017 14:00:40 -0400\n> Jeff Hostetler wrote:\n> \n>> (part 3)\n>>\n>> Additional overall comments on:\n>> https://github.com/jonathantanmy/git/commits/partialclone2\n>>\n>> {} WRT the code in is_promised() [1]\n>>\n>> [1]\n>> https://github.com/jonathantanmy/git/commit/7a9c2d9b6e2fce293817b595dee29a7eede0dddd#diff-5d5d5dc185ef37dc30bb7d9a7ae0c4e8R1960\n>>\n>> {} it looked like it was adding ALL promisor- and\n>> promised-objects to the \"promised\" OIDSET, rather than just\n>> promised-objects, but I could be mistaken.\n> \n> As far as I can tell, it is just adding the promised objects (some of\n> which may also be promisor objects). If you're saying that you expected\n> it to add the promisor objects as well, that might be a reasonable\n> expectation...I'm thinking of doing that.\n> \n\nIt looked like it was adding both types. I was concerned that that\nit might be doing too much. But I haven't run the code, that was from\nan observation.\n\n>> {} Is this iterating over ALL promisor-packfiles?\n> \n> Yes.\n> \n>> {} It looked like this was being used by fsck and rev-list. I\n>> have concerns about how big this OIDSET will get and how it will\n>> scale, since if we start with a partial-clone all packfiles will be\n>> promisor-packfiles.\n> \n> It's true that scaling is an issue. I'm not sure if omitting the oidset\n> will solve anything, though - as it is, Git maintains an object hash and\n> adds to it quite liberally.\n\nI guess I'm afraid that the first call to is_promised() is going\ncause a very long pause as it loads up a very large hash of objects.\n\nPerhaps you could augment the OID lookup to remember where the object\nwas found (essentially a .promisor bit set). Then you wouldn't need\nto touch them all.\n\n> \n> One thing that might help is some sort of flushing of objects in\n> promisor packfiles from the local repository - that way, the oidset\n> won't be so large.\n> \n>>\n>> {} When iterating thru a tree object, you add everything that it\n>> references (everything in that folder). This adds all of the\n>> child OIDs -- without regard to whether they are new to this\n>> version of the tree object. (Granted, this is hard to compute.)\n> \n> The oidset will deduplicate OIDs.\n\nRight, but you still have an entry for each object. For a repo the\nsize of Windows, you may have 25M+ objects your copy of the ODB.\n\n> \n>> My concern is that this will add too many objects to the\n>> OIDSET. That is, a new tree object (because of a recent change to\n>> something in that folder) will also have the OIDs of the other\n>> *unchanged* files which may be present in an earlier non-provisor\n>> packfile from an earlier commit.\n>>\n>> I worry that this will grow the OIDSET to essentially include\n>> everything. And possibly defeating its own purpose. I could\n>> be wrong here, but that's my concern.\n> \n> Same answer as above (about flushing of objects in promisor packfiles).\n> \n>> {} I'm not opposed to the .promisor file concept, but I have concerns\n>> that in practice all packfiles after a partial clone will be\n>> promisor-packfiles and therefore short-cut during fsck, so fsck\n>> still won't gain anything.\n>>\n>> It would help if there are also non-promisor packfiles present,\n>> but only for objects referenced by non-promisor packfiles.\n>>\n>> But then I also have to wonder whether we can even support\n>> non-promisor packfiles after starting with a partial clone -- because\n>> of the properties of received thin-packs on a non-partial fetch.\n> \n> Same reply as to your other e-mail - locally created objects are not in\n> promisor packfiles. (Or were you thinking of a situation where locally\n> created objects are immediately uploaded to the promisor remote, thus\n> making them promisor objects too?)\n> \n\nThanks,\nJeff\n"},{"id":"328694","messageId":"20170922154946.5052732a061155a3f81ae2a0@google.com","threadId":"46761","inReplyTo":"af084ada-4607-53e8-c6bb-f5bf13bdc153@jeffhostetler.com","subject":"Re: RFC: Design and code of partial clones (now, missing commits and trees OK)","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-09-22T22:49:46Z","receivedAt":"2017-09-22T22:49:54Z","isPatch":false,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"On Fri, 22 Sep 2017 17:02:11 -0400\nJeff Hostetler wrote:\n\n> > I was struggling a bit with the terminology, true.\n> > \n> > Right now I'm thinking of:\n> > - promisor remote (as you defined)\n> > - promisor packfile (as you defined)\n> > - promisor object is an object known to belong to the promisor (whether\n> > because we have it in a promisor packfile or because it is referenced\n> > by an object in a promisor packfile)\n> > \n> > This might eliminate \"promise(d)\", and thus eliminate the confusion\n> > between \"promised\" and \"promisor\", but I haven't done an exhaustive\n> > search.\n> > \n> \n> maybe just call the \"promised\" ones \"missing\".\n\nThey are not the same, though - \"missing\" usually just means that the\nlocal repo does not have it, without regard to whether another repo has\nit.\n\n> >> I guess it depends on how many missing-objects you expect the client\n> >> to have. My concern here is that we're limiting the design to the\n> >> \"occasional\" big file problem, rather than the more general scale\n> >> problem.\n> > \n> > Do you have a specific situation in mind?\n> > \n> \n> I have would like to be able do sparse-enlistments in the Windows\n> source tree. (3.5M files at HEAD.) Most developers only need a small\n> feature area (a device driver or file system or whatever) and not the\n> whole tree. A typical Windows developer may have only 30-50K files\n> populated. If we can synchronize on a sparse-checkout spec and use\n> that for both the checkout and the clone/fetch, then we can bulk fetch\n> the blobs that they'll actually need. GVFS can hydrate the files as\n> they touch them, but I can use this to pre-fetch the needed blobs in\n> bulk, rather than faulting and fetching them one-by-one.\n> \n> So, my usage may have >95% of the ODB be missing blobs. Contrast that\n> with the occasional large blob / LFS usage where you may have <5% of\n> the ODB as large objects (by count of OIDs not disk usage).\n\nI don't think the current design precludes a more intelligent bulk\nfetching (e.g. being allowed to specify a \"want\" tree and several \"have\"\ntrees, although we will have to figure out a design for that, including\nhow to select the \"have\" trees to inform the server about).\n\nIn the meantime, yes, this will be more useful for occasional large blob\nrepos, and (if/when the hook support is added) a GVFS situation where\nthe missing objects are available network-topologically close by.\n"},{"id":"328695","messageId":"20170922155235.7b50de91f0370d9101cc4faa@google.com","threadId":"46761","inReplyTo":"5b251b02-fe00-3d99-8ae2-d6df0193ebe0@jeffhostetler.com","subject":"Re: RFC: Design and code of partial clones (now, missing commits and trees OK) (part 2/3)","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-09-22T22:52:35Z","receivedAt":"2017-09-22T22:52:42Z","isPatch":false,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"On Fri, 22 Sep 2017 17:19:50 -0400\nJeff Hostetler wrote:\n\n> > In your specific example, how would rev-list know, on the client, to\n> > include (or exclude) a large blob in its output if it does not have it,\n> > and thus does not know its size?\n> > \n> \n> The client doesn't have the size. It just knows it is missing and it\n> needs it. It doesn't matter why it is missing. (But I guess the client\n> could assume it is because it is large.)\n\nAh, OK.\n\n> So rev-list on the client could filter the objects it has by size.\n\nMy issue is that if the purpose of this feature in rev-list is to do\nprefetching, the only criterion we need to check for is absence from the\nlocal repo right? (Or is filtering by size on the client useful for\nother reasons?)\n\n> FYI I just posted my RFC this afternoon.\n> https://public-inbox.org/git/20170922204211.GA24036@google.com/T/\n\nThanks, I'll take a look.\n"},{"id":"328697","messageId":"20170922155802.ab79717818578a23cc31f6fe@google.com","threadId":"46761","inReplyTo":"7977bab0-09c3-0e43-4d6f-f2bf87a3fd9e@jeffhostetler.com","subject":"Re: RFC: Design and code of partial clones (now, missing commits and trees OK) (part 3)","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-09-22T22:58:02Z","receivedAt":"2017-09-22T22:58:09Z","isPatch":false,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"On Fri, 22 Sep 2017 17:32:00 -0400\nJeff Hostetler wrote:\n\n> I guess I'm afraid that the first call to is_promised() is going\n> cause a very long pause as it loads up a very large hash of objects.\n\nYes, the first call will cause a long pause. (I think fsck and gc can\ntolerate this, but a better solution is appreciated.)\n\n> Perhaps you could augment the OID lookup to remember where the object\n> was found (essentially a .promisor bit set). Then you wouldn't need\n> to touch them all.\n\nSorry - I don't understand this. Are you saying that missing promisor\nobjects should go into the global object hashtable, so that we can set a\nflag on them?\n\n> > The oidset will deduplicate OIDs.\n> \n> Right, but you still have an entry for each object. For a repo the\n> size of Windows, you may have 25M+ objects your copy of the ODB.\n\nWe have entries only for the \"frontier\" objects (the objects directly\nreferenced by any promisor object). For the Windows repo, for example, I\nforesee that many of the blobs, trees, and commits will be \"hiding\"\nbehind objects that the repository user did not download into their\nrepo.\n"},{"id":"328944","messageId":"4cc2e996-250d-c62c-905d-b75467d7fe09@jeffhostetler.com","threadId":"46761","inReplyTo":"20170922155235.7b50de91f0370d9101cc4faa@google.com","subject":"Re: RFC: Design and code of partial clones (now, missing commits and trees OK) (part 2/3)","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-09-26T14:03:14Z","receivedAt":"2017-09-26T14:03:21Z","isPatch":false,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"\n\nOn 9/22/2017 6:52 PM, Jonathan Tan wrote:\n> On Fri, 22 Sep 2017 17:19:50 -0400\n> Jeff Hostetler wrote:\n> \n>>> In your specific example, how would rev-list know, on the client, to\n>>> include (or exclude) a large blob in its output if it does not have it,\n>>> and thus does not know its size?\n>>>\n>>\n>> The client doesn't have the size. It just knows it is missing and it\n>> needs it. It doesn't matter why it is missing. (But I guess the client\n>> could assume it is because it is large.)\n> \n> Ah, OK.\n> \n>> So rev-list on the client could filter the objects it has by size.\n> \n> My issue is that if the purpose of this feature in rev-list is to do\n> prefetching, the only criterion we need to check for is absence from the\n> local repo right? (Or is filtering by size on the client useful for\n> other reasons?)\n\nThe prefetch before a checkout may want all missing blobs, or it\nwant to work with the sparse-checkout specification and only get\nthe required missing blobs in the subset of the tree. By putting\nthe same filter logic in rev-list, we can do that.\n\nIt also sets the stage for later filtering trees. (My current patch\nonly filters blobs. It would be nice to have a second version of the\nsparse filter that also omits trees, but that may require a recursive\noption in the fetch-objects protocol.)\n\n\n> \n>> FYI I just posted my RFC this afternoon.\n>> https://public-inbox.org/git/20170922204211.GA24036@google.com/T/\n> \n> Thanks, I'll take a look.\n> \n"},{"id":"328945","messageId":"e05a7978-c312-9e26-79a2-fca3ae44f59a@jeffhostetler.com","threadId":"46761","inReplyTo":"20170922155802.ab79717818578a23cc31f6fe@google.com","subject":"Re: RFC: Design and code of partial clones (now, missing commits and trees OK) (part 3)","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-09-26T14:25:16Z","receivedAt":"2017-09-26T14:25:21Z","isPatch":false,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"\n\nOn 9/22/2017 6:58 PM, Jonathan Tan wrote:\n> On Fri, 22 Sep 2017 17:32:00 -0400\n> Jeff Hostetler wrote:\n> \n>> I guess I'm afraid that the first call to is_promised() is going\n>> cause a very long pause as it loads up a very large hash of objects.\n> \n> Yes, the first call will cause a long pause. (I think fsck and gc can\n> tolerate this, but a better solution is appreciated.)\n> \n>> Perhaps you could augment the OID lookup to remember where the object\n>> was found (essentially a .promisor bit set). Then you wouldn't need\n>> to touch them all.\n> \n> Sorry - I don't understand this. Are you saying that missing promisor\n> objects should go into the global object hashtable, so that we can set a\n> flag on them?\n\nI just meant could we add a bit to \"struct object_info\" to indicate\nthat the object was found in a .promisor packfile ? This could\nbe set in sha1_object_info_extended().\n\nThen the is_promised() calls in fsck and gc would just test that bit.\n\nGiven that that bit will be set on promisOR objects (and we won't\nhave object_info for missing objects), you may need to adjust the\niterator in the fsck/gc code slightly.\n\nThis is a bit of a handwave, but could something like that eliminate\nthe need to build this oidset?\n\n\n> \n>>> The oidset will deduplicate OIDs.\n>>\n>> Right, but you still have an entry for each object. For a repo the\n>> size of Windows, you may have 25M+ objects your copy of the ODB.\n> \n> We have entries only for the \"frontier\" objects (the objects directly\n> referenced by any promisor object). For the Windows repo, for example, I\n> foresee that many of the blobs, trees, and commits will be \"hiding\"\n> behind objects that the repository user did not download into their\n> repo.\n> \n"},{"id":"328949","messageId":"c3245fd2-cff4-0266-37e9-35b307e0357b@alum.mit.edu","threadId":"46761","inReplyTo":"20170921154214.0d2ac45f@twelve2.svl.corp.google.com","subject":"Re: RFC: Design and code of partial clones (now, missing commits and trees OK)","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2017-09-26T15:26:33Z","receivedAt":"2017-09-26T15:26:42Z","isPatch":false,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 09/22/2017 12:42 AM, Jonathan Tan wrote:\n> On Thu, 21 Sep 2017 13:57:30 Jeff Hostetler wrote:\n> [...]\n>> I struggled with the terms here a little when looking at the source.\n>> () A remote responding to a partial-clone is termed a\n>> \"promisor-remote\". () Packfiles received from a promisor-remote are\n>> marked with \".promisor\" like \".keep\" names.\n>> () An object actually contained in such packfiles is called a\n>> \"promisor-object\". () An object not-present but referenced by one of\n>> the above promisor-objects is called a \"promised-object\" (aka a\n>> \"missing-object\").\n>>\n>> I think the similarity of the words \"promisOR\" and \"promisED\" threw\n>> me here and as I was looking at the code. The code in is_promised()\n>> [1] looked like it was adding all promisor- and promised-objects to\n>> the \"promised\" OIDSET, but I could be mistaken.\n>>\n>> [1]\n>> https://github.com/jonathantanmy/git/commit/7a9c2d9b6e2fce293817b595dee29a7eede0dddd#diff-5d5d5dc185ef37dc30bb7d9a7ae0c4e8R1960\n> \n> I was struggling a bit with the terminology, true.\n> \n> Right now I'm thinking of:\n> - promisor remote (as you defined)\n> - promisor packfile (as you defined)\n> - promisor object is an object known to belong to the promisor (whether\n> because we have it in a promisor packfile or because it is referenced\n> by an object in a promisor packfile)\n\nMaybe naming has been discussed at length before, and I am jumping into\na long-settled topic. And admittedly this is bikeshedding.\n\nBut I find these names obscure, even as a developer. And terms like this\nwill undoubtedly bleed into the UI and documentation, so it would be\ngood to put some effort into choosing the best names possible.\n\nI suppose that the term \"promisor\" comes from the computer science term\n\"promise\" [1]. In that sense it is apt, because, say, a promisor object\nis something that is known to be obtainable, but we don't have it yet.\n\nBut from the user's point of view, I think this isn't a very\nilluminating term. I think the user's mental model will be that there is\na distinguished remote repository that holds the project's entire\npublished history, and she has to remain connected to it for certain Git\noperations to work [2]. Another interesting aspect of this remote is\nthat it has to be trusted never (well, almost never) to discard any\nobjects [3].\n\nLet me brainstorm about other names or concepts that seem closer to the\nuser's mental model:\n\n* \"backing remote\", \"backing repository\"\n\n* \"lazy remote\", \"live remote\", \"cloud remote\", \"shared remote\",\n \"on-demand remote\"\n\n* \"full remote\", \"deep remote\", \"permanent remote\"\n\n* \"attached remote\", \"bound remote\", \"linked remote\"\n\n* \"trusted remote\", \"authoritative remote\", \"official remote\"\n (these terms probably imply more than we want)\n\n* \"upstream\", \"upstream remote\" (probably too confusing with how\n the term \"upstream\" is currently used, even if in practice they\n will often be the same remote)\n\n* \"object depot\", \"depot remote\", \"depot repository\", \"remote\n object depot\" (I don't like the term \"object\" that much, because\n it is rather remote from the Git user's daily life)\n\n* \"repository of record\", \"remote of record\" (too wordy)\n\n* \"history depot\", \"history warehouse\" (meh)\n\n* (dare I say it?) \"central repository\"\n\n* \"object archive\", \"archival remote\" (not so good because we already\n use \"archive\" for other concepts)\n\n* depository (sounds too much like \"repository\")\n\n* The thesaurus suggests nouns like: store, bank, warehouse, library,\n chronicle, annals, registry, locker, strongbox, attic, bunker\n\nPersonally I think \"lazy remote\" and \"backing remote\" are not too bad.\n\nMichael\n\n[1] https://en.wikipedia.org/wiki/Futures_and_promises\n\n[2] I haven't checked whether the current proposal allows for\n multiple \"promisor remotes\". It's certainly thinkable, if not\n now then in the future. But I suppose that even then, 99% of\n users will configure a single \"promisor remote\" for each\n repository.\n\n[3] For those rare occasions where the server has to discard objects,\n it might make sense for the server to remember the names of the\n objects that were deleted, so that it can tell clients \"no, you're\n not insane. I used to have that object but it has intentionally\n been obliterated\", and possibly even a reason: \"it is now taboo\"\n vs. \"I got tired of carrying it around\".\n"},{"id":"328952","messageId":"20170926103252.8330a266cf450e64dee27500@google.com","threadId":"46761","inReplyTo":"e05a7978-c312-9e26-79a2-fca3ae44f59a@jeffhostetler.com","subject":"Re: RFC: Design and code of partial clones (now, missing commits and trees OK) (part 3)","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-09-26T17:32:52Z","receivedAt":"2017-09-26T17:33:00Z","isPatch":false,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"On Tue, 26 Sep 2017 10:25:16 -0400\nJeff Hostetler wrote:\n\n> >> Perhaps you could augment the OID lookup to remember where the object\n> >> was found (essentially a .promisor bit set). Then you wouldn't need\n> >> to touch them all.\n> > \n> > Sorry - I don't understand this. Are you saying that missing promisor\n> > objects should go into the global object hashtable, so that we can set a\n> > flag on them?\n> \n> I just meant could we add a bit to \"struct object_info\" to indicate\n> that the object was found in a .promisor packfile ? This could\n> be set in sha1_object_info_extended().\n> \n> Then the is_promised() calls in fsck and gc would just test that bit.\n> \n> Given that that bit will be set on promisOR objects (and we won't\n> have object_info for missing objects), you may need to adjust the\n> iterator in the fsck/gc code slightly.\n> \n> This is a bit of a handwave, but could something like that eliminate\n> the need to build this oidset?\n\nThis oidset is meant to contain the missing objects, and is needed as\nthe final check (attempt to read the object, then check this oidset).\nAdmittedly, right now I add objects to it even if they are present in\nthe DB, but that's because I think that it's better for the set to be\nbigger than to incur the repeated existence checks. But even if we only\ninclude truly missing objects in this oidset, we still need the oidset,\nor store information about missing objects in some equivalent data\nstructure.\n\nThe bit that you mention being set on promisOR objects is already being\nset. See the invocation of mark_uninteresting() in rev-list.c.\n"},{"id":"329135","messageId":"20170928175352.9a490564d47172568ea2f416@google.com","threadId":"46761","inReplyTo":"20170915134343.3814dc38@twelve2.svl.corp.google.com","subject":"Re: RFC: Design and code of partial clones (now, missing commits and trees OK)","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-09-29T00:53:52Z","receivedAt":"2017-09-29T00:53:59Z","isPatch":false,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"On Fri, 15 Sep 2017 13:43:43 -0700\nJonathan Tan wrote:\n\n> For those interested in partial clones and/or missing objects in repos,\n> I've updated my original partialclone patches to not require an explicit\n> list of promises. Fetch/clone still only permits exclusion of blobs, but\n> the infrastructure is there for a local repo to support missing trees\n> and commits as well.\n> \n> They can be found here:\n> \n> https://github.com/jonathantanmy/git/tree/partialclone2\n\nI've pushed a new version:\n\nhttps://github.com/jonathantanmy/git/tree/partialclone3\n\nBesides some small changes as requested by comments on the GitHub\nrepository, I've also updated the code to do the following:\n - clarified terminology - in particular, I've tried to avoid\n \"promised\", only using \"promisor object\" to denote objects that the\n local repo knows that the promisor remote has, whether the local repo\n has it or not\n - restored bulk checkout functionality (so now you can clone with\n --blob-max-bytes=0)\n - a fix to fetch-pack to restore a global flag after it uses it, so\n commands like \"git log -S\" still work (but to test this, I used\n --blob-max-bytes=200000 with the Git repository, because batch\n fetching is not implemented for commands like these)\n\nIn its current form, the code is already useful for situations like:\n - a large repository with many blobs in which the client only needs to\n checkout, at most, and does not need to search through history\n locally, and\n - a repository with a few large blobs, where the client still can\n search through history as long as the client is online\n"},{"id":"329138","messageId":"xmqqfub6z2fw.fsf@gitster.mtv.corp.google.com","threadId":"46761","inReplyTo":"20170928175352.9a490564d47172568ea2f416@google.com","subject":"Re: RFC: Design and code of partial clones (now, missing commits and trees OK)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-09-29T02:03:15Z","receivedAt":"2017-09-29T02:03:22Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Tan writes:\n\n> I've pushed a new version:\n>\n> https://github.com/jonathantanmy/git/tree/partialclone3\n\nJust FYI, the reason why I commented only on the first patch in your\nprevious series at GitHub wasn't because I found the others perfect\nand nothing to comment on. It was because I found it extremely\npainful to conduct review and comment in the webform and gave up\nwhile trying to review the series that way just after doing a single\npatch.\n\nI also found it frustrating that it is not even obvious which one of\nthe many patches in the series have been already commented on,\nwithout clicking to each and every commit (and back), even when the\nresult is to find that nobody has commented on them yet.\n"},{"id":"329211","messageId":"20170929132128.3071d469cd8f80a86ce6b680@google.com","threadId":"46761","inReplyTo":"c3245fd2-cff4-0266-37e9-35b307e0357b@alum.mit.edu","subject":"Re: RFC: Design and code of partial clones (now, missing commits and trees OK)","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-09-29T20:21:28Z","receivedAt":"2017-09-29T20:21:35Z","isPatch":false,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"On Tue, 26 Sep 2017 17:26:33 +0200\nMichael Haggerty wrote:\n\n> Maybe naming has been discussed at length before, and I am jumping into\n> a long-settled topic. And admittedly this is bikeshedding.\n> \n> But I find these names obscure, even as a developer. And terms like this\n> will undoubtedly bleed into the UI and documentation, so it would be\n> good to put some effort into choosing the best names possible.\n\nNames are definitely not a long-settled topic. :-)\n\nI agree that naming is important, and thanks for your efforts.\n\n> I suppose that the term \"promisor\" comes from the computer science term\n> \"promise\" [1]. In that sense it is apt, because, say, a promisor object\n> is something that is known to be obtainable, but we don't have it yet.\n> \n> But from the user's point of view, I think this isn't a very\n> illuminating term. I think the user's mental model will be that there is\n> a distinguished remote repository that holds the project's entire\n> published history, and she has to remain connected to it for certain Git\n> operations to work [2]. Another interesting aspect of this remote is\n> that it has to be trusted never (well, almost never) to discard any\n> objects [3].\n\nYes, that is the mental model I have too. I think the ordinary meaning\nof the word \"promise\" works, though - you're not working completely on\nthings you have, but you're working partly based on the guarantees (or\npromises) that this distinguished remote repository gives.\n\n> Personally I think \"lazy remote\" and \"backing remote\" are not too bad.\n\nI think these terms are imprecise. \"Lazy remote\" seems to me to imply\nthat it is the remote that is lazy, not us.\n\n\"Backing remote\" does evoke the concept of a \"backing store\". For me,\nthe ability to transfer objects to the backing store to be stored\npermanently (so that you don't have to store it yourself) is an\nessential part of a backing store, and that is definitely something we\ndon't do here (although, admittedly, such a feature might be useful), so\nI don't agree with that term. But if transferring objects is not\nessential to a backing store, or if adding such a feature is a natural\nfit to the partial clone feature, maybe we could use that.\n\n> [2] I haven't checked whether the current proposal allows for\n> multiple \"promisor remotes\". It's certainly thinkable, if not\n> now then in the future. But I suppose that even then, 99% of\n> users will configure a single \"promisor remote\" for each\n> repository.\n\nIt does not allow for multiple \"promisor remotes\". Support for that\nwould require upgrades in the design (including knowing which remote to\nfetch a missing object from), but otherwise I agree with your\nstatements.\n"}]}