replication: ignore bookmarks when computing incremental path

fixes https://github.com/zrepl/zrepl/issues/490
closes https://github.com/zrepl/zrepl/pull/619

Co-authored-by: Christian Schwarz <me@cschwarz.com>

docs/changelog.rst

@@ -29,6 +29,8 @@ Developers should consult the git commit log or GitHub issue tracker.
     The Grafana dashboard in :repomasterlink:`dist/grafana` has been updated.
 
 * |bugfix| transient zrepl status error: ``Post "http://unix/status": EOF``
+* |bugfix| don't treat receive-side bookmarks as a replication conflict.
+  This facilitates chaining of replication jobs. See :issue:`490`.
 
 0.5
 ---

replication/logic/diff/diff.go

@@ -88,8 +88,27 @@ func SortVersionListByCreateTXGThenBookmarkLTSnapshot(fsvslice []*FilesystemVers
 	return sorted
 }
 
+func StripBookmarksFromVersionList(fsvslice []*FilesystemVersion) []*FilesystemVersion {
+	fslice := make([]*FilesystemVersion, 0, len(fsvslice))
+	for _, fv := range fsvslice {
+		if fv.Type != FilesystemVersion_Bookmark {
+			fslice = append(fslice, fv)
+		}
+	}
+	return fslice
+}
+
 func IncrementalPath(receiver, sender []*FilesystemVersion) (incPath []*FilesystemVersion, conflict error) {
 
+	// Receive-side bookmarks can't be used as incremental-from,
+	// and don't cause recv to fail if there is a newer bookmark than incremetal-form on the receiver.
+	// So, simply mask them out.
+	// This will also hide them in the report, but it keeps the code in this function simple,
+	// and a user who complains about them missing in a conflict message will likely require
+	// more education about bookmarks than a slightly more accurate error message. They'll get
+	// that when they open an issue.
+	receiver = StripBookmarksFromVersionList(receiver)
+
 	receiver = SortVersionListByCreateTXGThenBookmarkLTSnapshot(receiver)
 	sender = SortVersionListByCreateTXGThenBookmarkLTSnapshot(sender)
 

replication/logic/diff/diff_test.go

@@ -152,4 +152,18 @@ func TestIncrementalPath_BookmarkSupport(t *testing.T) {
 		assert.Equal(t, l("@a,1", "@b,2"), path)
 	})
 
+	// test that receive-side bookmarks younger than the most recent common ancestor do not cause a conflict
+	doTest(l("@a,1", "#b,2"), l("@a,1", "@c,3"), func(path []*FilesystemVersion, conflict error) {
+		assert.NoError(t, conflict)
+		require.Len(t, path, 2)
+		assert.Equal(t, l("@a,1")[0], path[0])
+		assert.Equal(t, l("@c,3")[0], path[1])
+	})
+	doTest(l("#a,1"), l("@a,1", "@b,2"), func(path []*FilesystemVersion, conflict error) {
+		assert.Nil(t, path)
+		ca, ok := conflict.(*ConflictNoCommonAncestor)
+		require.True(t, ok)
+		assert.Equal(t, l(), ca.SortedReceiverVersions, "See comment in IncrementalPath() on why we don't include the boomkmark here")
+	})
+
 }