X wcfs/xbtree: Fix treediff(a, ø)

treediff(ø, b) is TODO

wcfs/go.mod

@@ -10,7 +10,7 @@ require (
 	github.com/pkg/errors v0.9.1
 	github.com/stretchr/testify v1.7.0
 	lab.nexedi.com/kirr/go123 v0.0.0-20210302025843-863c4602a230
-	lab.nexedi.com/kirr/neo/go v0.0.0-20210503113049-7fba56df234c
+	lab.nexedi.com/kirr/neo/go v0.0.0-20210524152903-d02d65559752
 )
 
 // we use kirr/go-fuse@y/nodefs-cancel

wcfs/go.sum

@@ -193,3 +193,5 @@ lab.nexedi.com/kirr/neo/go v0.0.0-20210326102715-dc35035cdc18 h1:ysqIq6WqSMhCPbS
 lab.nexedi.com/kirr/neo/go v0.0.0-20210326102715-dc35035cdc18/go.mod h1:llI3hcJJMACe+rYuXUfS5dljjwIrlBMfJ1ZeRcey96A=
 lab.nexedi.com/kirr/neo/go v0.0.0-20210503113049-7fba56df234c h1:+M4xtOKZqy7oC6L9CzdXi77PNI9KnohOEnEsHlmAPPE=
 lab.nexedi.com/kirr/neo/go v0.0.0-20210503113049-7fba56df234c/go.mod h1:llI3hcJJMACe+rYuXUfS5dljjwIrlBMfJ1ZeRcey96A=
+lab.nexedi.com/kirr/neo/go v0.0.0-20210524152903-d02d65559752 h1:knRAqs0xLytZrxWHkCccg9xyAbAgzGFnyHE2rdg7onI=
+lab.nexedi.com/kirr/neo/go v0.0.0-20210524152903-d02d65559752/go.mod h1:llI3hcJJMACe+rYuXUfS5dljjwIrlBMfJ1ZeRcey96A=

wcfs/internal/xbtree/pptreesubset.go

@@ -92,11 +92,16 @@ func (S PPTreeSubSet) AddPath(path []zodb.Oid) {
 		panic("empty path")
 	}
 
+	// don't keep track of artificial empty tree
+	if l == 1 && path[0] == zodb.InvalidOid {
+		return
+	}
+
 	// don't explicitly keep track of embedded buckets - they all have
 	// InvalidOid, and thus, if kept in S, e.g. T/B1:a and another
 	// T/B2:b would lead to InvalidOid having multiple parents.
-	if l >= 2 && path[l-1] == zodb.InvalidOid {
-		path = path[:l-1]
+	if l == 2 && path[1] == zodb.InvalidOid {
+		path = path[:1]
 	}
 
 	parent := zodb.InvalidOid
@@ -268,11 +273,16 @@ func (A PPTreeSubSet) fixup(δnchild map[zodb.Oid]int) {
 	A.xfixup(+1, δnchild)
 }
 func (A PPTreeSubSet) xfixup(sign int, δnchild map[zodb.Oid]int) {
+	//fmt.Printf("\nfixup:\n")
+	//fmt.Printf("    ·: %s\n", A)
+	//fmt.Printf("    δ: %v\n", δnchild)
+	//defer fmt.Printf("  ->·: %s\n\n", A)
+
 	gcq := []zodb.Oid{}
 	for oid, δnc := range δnchild {
 		t := A[oid] // t != nil as A is PP-connected
 		t.nchild += sign*δnc
-		if t.nchild == 0 && /* not root node */t.parent != zodb.InvalidOid {
+		if t.nchild == 0 {
 			gcq = append(gcq, oid)
 		}
 	}

wcfs/internal/xbtree/pptreesubset_test.go

+// Copyright (C) 2021  Nexedi SA and Contributors.
+//                     Kirill Smelkov <kirr@nexedi.com>
+//
+// This program is free software: you can Use, Study, Modify and Redistribute
+// it under the terms of the GNU General Public License version 3, or (at your
+// option) any later version, as published by the Free Software Foundation.
+//
+// You can also Link and Combine this program with other software covered by
+// the terms of any of the Free Software licenses or any of the Open Source
+// Initiative approved licenses and Convey the resulting work. Corresponding
+// source of such a combination shall include the source code for all other
+// software used.
+//
+// This program is distributed WITHOUT ANY WARRANTY; without even the implied
+// warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+//
+// See COPYING file for full licensing terms.
+// See https://www.nexedi.com/licensing for rationale and options.
+
+package xbtree
+
+import (
+	"testing"
+
+	"lab.nexedi.com/kirr/neo/go/zodb"
+)
+
+func TestPPTreeSubSetOps(t *testing.T) {
+	const (
+		a zodb.Oid = 0xa + iota
+		b
+		c
+		d
+		ø = zodb.InvalidOid
+	)
+	type  S = PPTreeSubSet
+	type  testEntry struct {
+		A, B       S
+		Union      S
+		Difference S
+	}
+	E := func(A, B, U, D S) testEntry {
+		return testEntry{A, B, U, D}
+	}
+
+	testv := []testEntry{
+		E(
+			S{},	// A
+			S{},	// B
+				S{},	// U
+				S{}),	// D
+
+		E(
+			S{a:{ø,1}, b:{a,0}},	// A
+			S{a:{ø,1}, c:{a,0}},	// B
+				S{a:{ø,2}, b:{a,0}, c:{a,0}},	// U
+				S{a:{ø,1}, b:{a,0}}),		// D
+
+		E(
+			S{a:{ø,1}, b:{a,1}, c:{b,0}},	// A
+			S{a:{ø,1}, b:{a,1}, d:{b,0}},	// B
+				S{a:{ø,1}, b:{a,2}, c:{b,0}, d:{b,0}},	// U
+				S{a:{ø,1}, b:{a,1}, c:{b,0}}),		// D
+
+		E(
+			S{a:{ø,1}, b:{a,0}},	// A
+			S{a:{ø,1}, b:{a,0}},	// B
+				S{a:{ø,1}, b:{a,0}},	// U
+				S{}),			// D
+	}
+
+	for _, tt := range testv {
+		U := tt.A.Clone()
+		U.UnionInplace(tt.B)
+		D := tt.A.Clone()
+		D.DifferenceInplace(tt.B)
+
+		if !U.Equal(tt.Union) {
+			t.Errorf("Union:\n  A:   %s\n  B:   %s\n  ->u: %s\n  okU: %s\n", tt.A, tt.B, U, tt.Union)
+		}
+		if !D.Equal(tt.Difference) {
+			t.Errorf("Difference:\n  A:   %s\n  B:   %s\n  ->d: %s\n  okD: %s\n", tt.A, tt.B, D, tt.Difference)
+		}
+
+		// XXX also verify U/D properties like (A+B)\B + (A+B)\A + (A^B) == (A+B) ?
+	}
+}

wcfs/internal/xbtree/treediff.go

@@ -78,6 +78,8 @@ import (
 
 	"lab.nexedi.com/kirr/go123/xerr"
 	"lab.nexedi.com/kirr/neo/go/zodb"
+
+	"lab.nexedi.com/nexedi/wendelin.core/wcfs/internal/xzodb"
 )
 
 // ΔValue represents change in value.
@@ -352,8 +354,8 @@ func treediff(ctx context.Context, root zodb.Oid, δtops SetOid, δZTC SetOid, t
 	δtrackv := []*ΔPPTreeSubSet{}
 
 	for top := range δtops { // XXX -> sorted?
-		a, err1 := zgetNode(ctx, zconnOld, top)
-		b, err2 := zgetNode(ctx, zconnNew, top)
+		a, err1 := zgetNodeOrNil(ctx, zconnOld, top)
+		b, err2 := zgetNodeOrNil(ctx, zconnNew, top)
 		err := xerr.Merge(err1, err2)
 		if err != nil {
 			return nil, nil, err
@@ -410,7 +412,7 @@ func treediff(ctx context.Context, root zodb.Oid, δtops SetOid, δZTC SetOid, t
 // consistent with b (= a + δ).
 func diffX(ctx context.Context, a, b Node, δZTC SetOid, trackSet PPTreeSubSet, holeIdx treeSetKey) (δ map[Key]ΔValue, δtrack *ΔPPTreeSubSet, err error) {
 	if a==nil && b==nil {
-		panic("BUG: both a & b == nil")
+		panic("BUG: both a & b == nil")	// XXX -> not a bug e.g. for `ø ø T` sequence?
 	}
 
 	var aT, bT *Tree
@@ -462,8 +464,13 @@ func diffT(ctx context.Context, A, B *Tree, δZTC SetOid, trackSet PPTreeSubSet,
 	tracef("  diffT %s %s\n", xidOf(A), xidOf(B))
 	defer xerr.Contextf(&err, "diffT %s %s", xidOf(A), xidOf(B))
 
-	if A == nil { panic("A is nil") }
-	if B == nil { panic("B is nil") }
+	if A == nil { panic("A is nil") }	// XXX -> art. ø tree
+	Bempty := false
+	if B == nil {
+		// artificial empty tree
+		B = zodb.NewPersistent(reflect.TypeOf(Tree{}), /*jar*/nil).(*Tree)
+		Bempty = true
+	}
 
 	δ = map[Key]ΔValue{}
 	δtrack = &ΔPPTreeSubSet{Del: PPTreeSubSet{}, Add: PPTreeSubSet{}, δnchildNonLeafs: map[zodb.Oid]int{}}
@@ -505,7 +512,9 @@ func diffT(ctx context.Context, A, B *Tree, δZTC SetOid, trackSet PPTreeSubSet,
 	// {} oid -> parent for all nodes in Bv: current and previously expanded - up till top B
 	// XXX requires A.oid == B.oid
 	BtrackSet := PPTreeSubSet{}
-	BtrackSet.AddPath(trackSet.Path(B.POid()))
+	if !Bempty {
+		BtrackSet.AddPath(trackSet.Path(B.POid()))
+	}
 
 	// phase 1: expand A top->down driven by δZTC.
 	// by default a node contributes to δ-
@@ -574,14 +583,17 @@ func diffT(ctx context.Context, A, B *Tree, δZTC SetOid, trackSet PPTreeSubSet,
 							if blo != bhi_ {
 								break
 							}
-							_, ok := blo.node.(*Tree)
+							bloT, ok := blo.node.(*Tree)
 							if !ok {
 								break // bucket
 							}
-							err = blo.node.PActivate(ctx);  /*X*/if err != nil { return nil,nil, err }
-							defer blo.node.PDeactivate()
+							err = bloT.PActivate(ctx);  /*X*/if err != nil { return nil,nil, err }
+							defer bloT.PDeactivate()
+
+							if len(bloT.Entryv()) == 0 {
+								break // empty tree
+							}
 
-							// XXX check for empty tree?
 							bchildren := Bv.Expand(blo)
 							for _, bc := range bchildren {
 									bcOid := bc.node.POid()
@@ -841,11 +853,15 @@ func diffB(ctx context.Context, a, b *Bucket) (δ map[Key]ΔValue, err error) {
 	return δ, nil
 }
 
-// zgetNode returns btree node corresponding to zconn.Get(oid) .
-func zgetNode(ctx context.Context, zconn *zodb.Connection, oid zodb.Oid) (_ Node, err error) {
+// zgetNodeOrNil returns btree node corresponding to zconn.Get(oid) .
+// if the node does not exist, (nil, ok) is returned.
+func zgetNodeOrNil(ctx context.Context, zconn *zodb.Connection, oid zodb.Oid) (_ Node, err error) {
 	defer xerr.Contextf(&err, "getnode %s@%s", oid, zconn.At())
 	xnode, err := zconn.Get(ctx, oid)
 	if err != nil {
+		if xzodb.IsErrNoData(err) {
+			err = nil
+		}
 		return nil, err
 	}
 
@@ -854,6 +870,19 @@ func zgetNode(ctx context.Context, zconn *zodb.Connection, oid zodb.Oid) (_ Node
 		return nil, fmt.Errorf("unexpected type: %s", zodb.ClassOf(xnode))
 	}
 
+	// activate the node to find out it really exists
+	// after removal on storage, the object might have stayed in Connection
+	// cache due to e.g. PCachePinObject, and it will be PActivate that
+	// will return "deleted" error.
+	err = node.PActivate(ctx)
+	if err != nil {
+		if xzodb.IsErrNoData(err) {
+			return nil, nil
+		}
+		return nil, err
+	}
+	node.PDeactivate()
+
 	return node, nil
 }
 

wcfs/internal/xbtree/δbtail_test.go

@@ -342,22 +342,26 @@ func (rbs RBucketSet) trackSet(tracked SetKey) PPTreeSubSet {
 				ppoid = p.parent.oid
 			}
 
-			pt, already := trackSet[p.oid]
-			newParent := false
-			if !already {
-				pt = &nodeInTree{parent: ppoid, nchild: 0}
-				trackSet[p.oid] = pt
-				newParent = true
-			}
-			if pt.parent != ppoid {
-				panicf("BUG: %s: T%s -> multiple parents: %s %s", rbs.coverage(), p.oid, pt.parent, ppoid)
-			}
+			// skip ø (non-existing) tree
+			if p.oid != zodb.InvalidOid {
+				newParent := false
+				pt, already := trackSet[p.oid]
+				if !already {
+					pt = &nodeInTree{parent: ppoid, nchild: 0}
+					trackSet[p.oid] = pt
+					newParent = true
+				}
+				if pt.parent != ppoid {
+					panicf("BUG: %s: T%s -> multiple parents: %s %s", rbs.coverage(), p.oid, pt.parent, ppoid)
+				}
 
-			if newNode {
-				pt.nchild++
+				if newNode {
+					pt.nchild++
+				}
+				newNode = newParent
 			}
+
 			p = p.parent
-			newNode = newParent
 		}
 	}
 	return trackSet
@@ -610,10 +614,7 @@ func xverifyΔBTail_Update(t *testing.T, subj string, db *zodb.DB, treeRoot zodb
 	t.Run(fmt.Sprintf("Update/%s→%s", t1.tree, t2.tree), func(t *testing.T) {
 		if t1.tree == DEL {
 			// Track(k) -> Update  ; requires that tree exists at Track time
-			t.Skip("Track(tree1) needs tree1 to exist")
-		}
-		if t2.tree == DEL {
-			t.Fatal("TODO")
+			t.Skip("Track(tree1) needs tree1 to exist")	// XXX just limit trackKeys to ø instead ?
 		}
 
 		allKeys := allTestKeys(t1, t2)
@@ -1320,6 +1321,10 @@ func TestΔBTail(t *testing.T) {
 
 	// test known cases going through tree1 -> tree2 -> ...
 	testv := []interface{} {
+		// start from non-empty tree to verify both ->empty and empty-> transitions
+		"T/B1:a,2:b",
+
+		// empty
 		"T/B:",
 
 		// +1

wcfs/internal/xzodb/xzodb.go

@@ -22,6 +22,7 @@ package xzodb
 
 import (
 	"context"
+	"errors"
 	"fmt"
 
 	"lab.nexedi.com/kirr/go123/xcontext"
@@ -80,3 +81,18 @@ func ZOpen(ctx context.Context, zdb *zodb.DB, zopt *zodb.ConnOptions) (_ *ZConn,
 		TxnCtx:     txnCtx,
 	}, nil
 }
+
+// IsErrNoData returns whether err is due to NoDataError or NoObjectError.
+func IsErrNoData(err error) bool {
+	var eNoData   *zodb.NoDataError
+	var eNoObject *zodb.NoObjectError
+
+	switch {
+	case errors.As(err, &eNoData):
+		return true
+	case errors.As(err, &eNoObject):
+		return true
+	default:
+		return false
+	}
+}