Beta test request
(1) By Richard Hipp (drh) on 2020-04-13 13:14:11 [link] [source]
I have made some enhancements to the sync protocol that should reduce bandwidth in cases where one side of the sync is holding many private branches that have been merged into public branches. These changes should be fully backwards and forward compatible. Older clients should work with newer servers and newer clients should work with older servers. Nevertheless, there is the possibility that I have overlooked something, so please be on the lookout for unusual sync behavior. Report any problems to this thread. Thanks.
Low-level details of the enhancement for the curious:
If a developer uses a private branch to work on some feature in their on local repository, then later merges the private branch onto a public branch (say "trunk") and syncs, the manifest for the merge-to-trunk check-in will record the fact that it was a merge of the private branch. The hash of the last check-in of the private branch is recorded in the P card of the merge-to-trunk check-in.
When the merge is received on the server, the server parses the manifest and sees the last check-in of the private branch on in the P card. It does not have the content of that private check-in, so it creates what we call a "phantom" entry in the BLOB table. A phantom is an artifact for which the hash is known but the content is unknown.
Whenever syncs happen, both parties to the sync look at their phantom list and ask the other side "please send me the content for these artifacts". In other words, Fossil is always trying to resolve phantoms.
But if the developer never publishes their private branch, that means that these phantoms will hang out forever, and every sync message will contain a long list of "gimme" cards requesting the content of phantoms that will never arrive. Over time this list gets longer and longer.
The enhancement to the sync protocol essentially gives Fossil the ability to say "artifact X is private and I am not going to tell you its content, so stop asking". After this enhancement (assuming that it works) the next time the developer with the private branch syncs, the server will ask for some of those private check-ins, and the developer's local repository will answer with "I'm not going to tell you about those so stop asking for them."
(2) By Florian Balmer (florian.balmer) on 2020-04-13 14:30:04 in reply to 1 [link] [source]
It seems the phantoms are due to the +close
tag added by the fossil merge --integrate
command, as reported here.
To my understanding, merging a private branch into a public branch does not track the reference to the private branch in the P card of the resulting merge check-in, probably for the very reason to avoid phantoms.
A possible fix is to disallow the --integrate
flag when merging private branches, with a hint to close the private branch separately by fossil amend --close
(the generated control artifact referring solely to the private branch would then also remain private).
This solution, together with a few other changes, is on the private-branches branch.
(5) By Richard Hipp (drh) on 2020-04-13 16:20:23 in reply to 2 [link] [source]
Can you bring the private-branches branch up-to-date with trunk for me? And test it again?
(6) By Florian Balmer (florian.balmer) on 2020-04-13 16:52:24 in reply to 5 [link] [source]
I was able to merge with trunk without conflicts today, but 1 or 2 commits behind the current tip. I'm off my dev machine for 1or 2 days, but then I can find the time for an up-to-date merging and testing.
I was using the branch without problems, so far, but usually I don't sync private commits.
(12) By Florian Balmer (florian.balmer) on 2020-04-15 12:18:23 in reply to 6 [link] [source]
The branch has been updated, and re-tested successfully.
(7) By Richard Hipp (drh) on 2020-04-13 20:06:18 in reply to 2 [link] [source]
It seems the phantoms are due to the +close tag added by the fossil merge --integrate command, as reported here.
Confirmed. You are also correct that when a private branch is merged into a public branch, the private branch is omitted from the P card. But I'm wonder: Is that correct? Should we not keep of record of where the merge came from, even if that source is unavailable? By omitting the private branch from the P card, that is making the merge work like rebase.
Preventing unnecessary leakage of private artifact names into the public block-chain is good. But Fossil still ought to be able to robustly handle missing artifacts regardless. It does a better job of that now, but it could be made better still. I'll continue thinking about how to make the sync protocol even more efficient and more robust.
It is good to prevent unnecessary leakage of private artifact hashes into the public block-chain. But some leakage might be necessary. The private +close tags caused by merging a private branch into a public branch with --integrate is truly unnecessary, as the same could be handled by a separate tag artifact that is itself private.
But what if we do change things so that private merge parents do appear on the P-cards of public check-ins? That too would count as private hash leaking to the public block-chain. I would call that a necessary leak, however. I propose to deal with the necessary leaks separately, both in the timeline display logic and in the sync protocol.
(8) By Richard Hipp (drh) on 2020-04-13 20:28:32 in reply to 7 [link] [source]
Should we not keep of record of where the merge came from, even if that source is unavailable?
There are two check-ins in the Fossil source tree that do this already. You can see them both, on the /bloblist page. So there is precedent for including private merge parents on the P-card. It does not appear to break anything. I thinking we need to make that behavior standard policy.
Returning to the main topic of this thread (optimizing sync): The three non-private phantoms currently in the Fossil public block chain cause three "gimme" records to be sent in both directions on every round-trip of a sync.
gimme ee2d352b3e23c80eaf4bfa5465be546b12db6e1b
gimme ab58aacfbe23fc2fe849eff53306daa9199a7f7f
gimme edf1bee5f2a3577138c6df087565fe5462d99926
I need to come up with a better way for Fossil to realize that it isn't going to (easily) acquire these artifacts, and to stop asking for them on every single sync message.
(13.1) By Florian Balmer (florian.balmer) on 2020-04-15 12:22:03 edited from 13.0 in reply to 8 [link] [source]
... including private merge parents on the P-card ... thinking we need to make that behavior standard policy.
I tend to use private branches like an extended "stash" feature, and therefore I don't need (and don't want) any stale references to private artifacts on non-local peers -- hence my efforts to get rid of the phantoms due to the +close
tags. The less clutter (and phantoms) I have in my "master" repositories on my web server, the better is my feeling that everything is alright with them.
But I can see your point, too, and there were other votes to include private merge parents in P-cards, especially with regard to later publishing of initially private branches.
I will be able to work with whatever you decide! But I can already imagine myself playing tricks ;-) like fossil co --keep SOME-PUBLIC-BRANCH
to switch from an (up-to-date) private to a public branch, to transfer the whole snapshot of the private branch to the public branch, and then commit the changes on the public branch, without generating any stale references to private artifacts. Somewhat similar to rebasing, but snapshot-based instead of diff-based.
In case the private-branches branch is a merge candidate for trunk, and the decision is made to allow private merge parents in P-cards, then [8268c5dafb] could be backed out, and [86ecdeefc8] could be adapted to keep only the paragraphs (1-2) on publishing private branches, and delete the last paragraph (3) to explain why --integrate
is disallowed when merging private branches.
I think it's good to have consistent behavior, and if private merge parents are allowed in P-cards, they can also be allowed in +close
tags, even more so because the tag would just reference the same artifact already referenced by the P-card.
Further observations:
- Currently, for cherry-pick and backout merges, private parents are recorded in the
Q+/-
cards, something I didn't address in my changes, however the new phantoms ui page does not seem to list them for peer repositories lacking the private parent.- If the decision is made not to allow private merge parents in P-cards, maybe they should also be disallowed for
Q+/-
cards?
- If the decision is made not to allow private merge parents in P-cards, maybe they should also be disallowed for
- Should new features from the phantoms ui page (i.e. the more detailed descriptions for unknown artifacts) be ported to the test-missing and/or test-orphans commands?
(Edit: fixed nested list layout.)
(14) By Richard Hipp (drh) on 2020-04-15 13:23:19 in reply to 13.1 [link] [source]
The "private-branches" branch has been merge at check-in b4beadb5078a0342. I also included an associated wiki page on that check-in that tries to encapsulate the issues we are working on and suggests a few new improvement ideas.
That associated wiki page also includes a question about whether or not the backlink detection will work for Markdown pages. I found that it does not. This is something that should be investigated and possibly fixed.
(15) By Florian Balmer (florian.balmer) on 2020-04-15 13:46:04 in reply to 14 [link] [source]
Wow, that's quite some brain jogging going on here ...
Regarding auto-running fossil amend --close
instead of adding +close
tags, this may work with a bag to collect RIDs (integers) instead of UUIDs (strings) during the check-in process, and then invoke the amend
command directly via its handler function for each bag entry with arguments like rid:%d(RID)
, and let the amend
handler find out the UUIDs, again.
(17) By Richard Hipp (drh) on 2020-04-15 14:16:26 in reply to 15 [link] [source]
quite some brain jogging going on here
Yeah, it gets complicated. Which is why I would really like to be able to attach Forum discussions to a check-in or branch. That way, when somebody looks at the code five years from now and asks "what where we/they thinking?" the discussion would be right there in front of them.
There really wants to be two forums: This separate forum for discussion of end-user related issues ("How do I set up my server?", "What does this error message mean?", and so forth) and then a developer-oriented forum in the same repository with the source code and that is dedicated to discussions about how the code is or ought to be implemented and that can only be posted by people with check-in privilege.
Regarding auto-running fossil amend --close...
Rather than actually invoking the "fossil amend" code, I was thinking that the "fossil commit" could just simply create the appropriate tag artifacts and add them to the blockchain together with the check-in manifest and the various file artifacts. It should be as simple as a subroutine call. I don't know if that subroutine exists yet or not, but it seems like it would be relatively easy to add.
(20) By ddevienne on 2020-04-15 15:43:16 in reply to 17 [link] [source]
a developer-oriented forum in the same repository with the source code and
that is dedicated to discussions about how the code is or ought to be implemented
Sounds like almost like a CodeReview tool, no?
Having discussions around specific codes of lines, at a
given point in time is very much CodeReview-like to me.
and that can only be posted by people with check-in privilege.
Why? Most of your users are developers too, after all, no?
Like the recent SQLite thread about providing the length of some SQL
with pieces of source codes. Being able to start a thread off of the
code itself would be useful. Not super common, but if commiters can
do it, why not other people too, modulo moderation of course.
(21) By Florian Balmer (florian.balmer) on 2020-04-22 12:03:18 in reply to 17 [link] [source]
... "fossil commit" could just simply create the appropriate tag artifacts and add them to the blockchain together with the check-in manifest ...
I'm sorry I'm a little late, so maybe this goes after the 2.11 release (that is, if it turns out to be useful)? After some time spent on an abandoned approach, I think I've found a better solution now to implement the outlined behavior, see recent private branches check-ins.
The commit code now generates separate private control artifacts to close leaves on private branches that were merged with the --integrate
flag.
Each control artifact has a timestamp delta of +1 second relative to the check-in time, or the previous control artifact, respectively.
The output of fossil commit --dry-run
is now a bit more complicated, as there's additional dumps of control artifacts after the "Closed:" entries for private branches, and they can be intermingled with "Closed:" entries for public branches without additional control artifacts. However, I've taken care to keep the overall output as stable as possible, so as not to confuse possible scripts parsing the output.
Maybe that one single control artifact holding multiple +close
entries for multiple private branches would also work, but I haven't tested this. And I'm not sure if the timestamp delta of +1 second is okay, but I think it's better to keep the events in chronological order.
(22.1) By Richard Hipp (drh) on 2020-04-22 12:18:56 edited from 22.0 in reply to 21 [link] [source]
Let's hold off merging this change until after 2.11. I consider this to be an enhancement, not a bug fix.
(23) By Florian Balmer (florian.balmer) on 2020-04-22 13:15:55 in reply to 22.1 [link] [source]
That's fine. I'll give it another round of code review and detailed testing, and any potential corrections from my part should be committed and synced within 1 or 2 days. Thanks for your interest!
I really love diving into the Fossil source code. At first sight, things often look fairly complex, but then they turn out to be simple and elegant. Really one great code base!
And I'm happy this isn't C++: similar to Perl, C++ has turned into a write-only (i.e. unreadable) language for me. For example, just recently, I learned about "pip(e)able functions" (which have obviously been around for a few years, already): they overloaded the logical OR operator to act like a pipe in shell scripts, i.e. to use output of functions as input to chained functions. I can hardly imagine it's possible to fully understand the resulting code. Oh dear, how much I love plain old C!
(16) By Florian Balmer (florian.balmer) on 2020-04-15 14:07:13 in reply to 14 [link] [source]
But, perhaps there should also be a separate (public) tag artifact that designates the private merge parent as being private, so that there will be an artifact in the public blockchain that prevents the P-card reference from generating a persistent phantom.
Wouldn't that re-introduce the same problem that was solved by changing the tagging of private contents from +private
tags to the RID→PRIVATE table, i.e. that private contents can't be published? Once a peer has the new tag artifact to designate the private merge parent as private, then it couldn't be published anymore on this server, or at least it would be unpublished after a rebuild? This would require yet another tag to cancel the new "this is a private phantom" tag, and then things will be more even complicated than before the introduction of the RID→PRIVATE table.
(18) By Florian Balmer (florian.balmer) on 2020-04-15 14:22:45 in reply to 16 [link] [source]
Or, the new tag could be valid only for phantoms, and could even be dropped on rebuild, if the target artifact is no longer a phantom. Still, this sounds tricky.
(19) By Richard Hipp (drh) on 2020-04-15 14:24:19 in reply to 16 [link] [source]
The tag artifact itself is public and goes into the public blockchain. But the tag references the private artifact and marks that artifact as being private. Thus, there is a public declaration saying: "artifact abcdef0123456... is private".
So someone (or some-algorithm) that only has access to the public blockchain sees the hash of the private artifact and knows it exists. So the private artifact is a phantom. But we also see the content of the separate public tag and hence know that the artifact is private and that we should not be sending "gimme" cards to anyone and everyone in an effort to find the artifact's content.
(3) By anonymous on 2020-04-13 15:01:39 in reply to 1 [link] [source]
...so please be on the lookout for unusual sync behavior. Report any problems to this thread.
I'm using private branches in my workflows. How these changes should manifest themselves in sync use or it's meant to be transparent?
I understand that private branches are not pushed by default, so it's reasonable to assume the merged checkins will remain private forever. How about the cases when the at some point the whole private branch is being pushed, would this be still supported?
I can test this scenario, provided I understand what to expect.
(4) By Richard Hipp (drh) on 2020-04-13 16:18:25 in reply to 3 [source]
Everything should "just work" without you noticing any change in behavior and certainly without any malfunctions. The only difference will be that fewer bytes need to cross the network when you sync.
For now, please consider downloading, compiling, and using the very latest trunk check-ins for Fossil, and let me know (on this forum) if you see anything that looks like it isn't working as it should.
For example: At one point, I had "fossil sync" command stuck in a loop where the client and server were exchanging messages over and over and not making any progress. (That problem was fixed before I checked in, so it won't affect you.) That's the kind of trouble I'm looking for. Other possible anomalies include a failure to sync all content. (If you get into a situation where two repos are not syncing correctly, you can always try "fossil sync --verily" to try to recover.)
(9) By PF (peter) on 2020-04-14 00:35:34 in reply to 4 [link] [source]
Since I build by latest fossil based on [ec93507424] I can't pull from fossil main repo any more with my pull script I am using for last few years. That is I can't pull anything after [ec93507424].
Adding --verily
does not help pull latest changes as well.
I can't rule out my changes in fossil break something of sync logic but I am not aware of any of those.
I suppose I can bring my fossilclone.fossil
repo up to date with some official fossil build. For now I just report
this.
fossil version
This is fossil version 2.11 [298fed1011] 2020-04-13 16:52:08 UTC
fossil pull https://www.fossil-scm.org/home -R fossilclone.fossil --verbose
Bytes Cards Artifacts Deltas
Sent: 456 7 0 0
Received: 3576 52 0 0
Pull done, sent: 474 received: 2166 ip: 45.33.6.223
fossil pull https://www.fossil-scm.org/home -R fossilclone.fossil --verbose --verily
Bytes Cards Artifacts Deltas
Sent: 476 8 0 0
Received: 2280884 45174 0 0
Pull done, sent: 488 received: 1227616 ip: 45.33.6.223
(10) By Richard Hipp (drh) on 2020-04-14 02:28:27 in reply to 9 [link] [source]
Thanks for the report. The problem is now fixed on trunk (I think - correct me if you discover otherwise).
To fix your local setup you can apply the patch (also reproduced below)
and rebuild Fossil, then sync to the latest trunk and rebuild again. Or
you can back up using "fossil update 10c90175a702dcc4
", rebuild, then
do the sync all the way to the latest trunk, and rebuild again.
Index: src/xfer.c
==================================================================
--- src/xfer.c
+++ src/xfer.c
@@ -2258,11 +2258,11 @@
if( isPriv ){
content_make_private(rid);
}else{
content_make_public(rid);
}
- }else if( !g.perm.Private ){
+ }else if( isPriv && !g.perm.Private ){
/* ignore private files */
}else if( (syncFlags & (SYNC_PULL|SYNC_CLONE))!=0 ){
rid = content_new(blob_str(&xfer.aToken[1]), isPriv);
if( rid ) newPhantom = 1;
}
(11) By PF (peter) on 2020-04-14 11:56:06 in reply to 10 [link] [source]
Thank you for the patch/hint.
Finally I went by "fossil update 10c90175a702dcc4" route as provided patch was not compilable.
Patch including mkversion.c changes did compile but I did not manage it to work.
Version with latest-greatest trunk it is working fine again. Thanks
(24) By Stephan Beal (stephan) on 2020-04-30 06:13:09 in reply to 1 [link] [source]
I have made some enhancements to the sync protocol that should reduce bandwidth
Just to point out, in off case it was unintentional: this branch is still opened after merging into trunk: