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."

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.

...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.

Can you bring the private-branches branch up-to-date with trunk for me? And test it again?

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.

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.

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.

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

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;
         }

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

The branch has been updated, and re-tested successfully.

... 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?
• 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.)

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.

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.

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.

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.

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.

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.

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.

... "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.

Let's hold off merging this change until after 2.11. I consider this to be an enhancement, not a bug fix.

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!

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:

https://fossil-scm.org/home/timeline?r=sync-improvements