Commit 124688f9 authored by Kirill Smelkov's avatar Kirill Smelkov

X ΔFtail fixes

* t2:
  X ΔFtail.SliceByFileRev: Fix untracked entries to be present uniformly in result
  .
  .
  .
  .
  .
  X test that shows problem of SliceByRootRev where untracked blocks are not added uniformly into whole history
  .
  .
  .
  .
  .
  .
  .
  .
  X Size no longer tracks [0,∞) since we start tracking when zfile is non-empty
  X ΔFtail: `go test -failfast -short -v -run Random -randseed=1626793016249041295` discovered problems
parents 0853cc9f c0b7e4c3
......@@ -66,7 +66,7 @@ type setOid = set.Oid
// .Update(δZ) -> δF - update files δ tail given raw ZODB changes
// .ForgetPast(revCut) - forget changes past revCut
// .SliceByRev(lo, hi) -> []δF - query for all files changes with rev ∈ (lo, hi]
// .SliceByFileRev(file, lo, hi) -> []δfile - query for changes of file with rev ∈ (lo, hi]
// .SliceByFileRev(file, lo, hi) -> []δfile - query for changes of a file with rev ∈ (lo, hi]
// .BlkRevAt(file, #blk, at) - query for what is last revision that changed
// file[#blk] as of @at database state.
//
......@@ -74,7 +74,7 @@ type setOid = set.Oid
//
// δfile:
// .rev↑
// {}blk
// {}blk | EPOCH
//
// XXX concurrent use
//
......@@ -85,7 +85,7 @@ type ΔFtail struct {
fileIdx map[zodb.Oid]setOid // tree-root -> {} ZBigFile<oid> as of @head XXX -> root2file ?
byFile map[zodb.Oid]*_ΔFileTail // file -> vδf tail XXX
// set of files, which are newly tracked and for which vδE was not yet rebuilt
// set of files, which are newly tracked and for which byFile[foid].vδE was not yet rebuilt
trackNew setOid // {}foid
trackSetZBlk map[zodb.Oid]*zblkTrack // zblk -> {} root -> {}blk as of @head
......@@ -157,14 +157,26 @@ func (δFtail *ΔFtail) Tail() zodb.Tid { return δFtail.δBtail.Tail() }
//
// XXX Track adds tree path to tracked set and associates path root with file.
//
// XXX text
//
// XXX objects in path and zblk must be with .PJar().At() == .head
// Objects in path and zblk must be with .PJar().At() == .head
//
// A root can be associated with several files (each provided on different Track call).
func (δFtail *ΔFtail) Track(file *ZBigFile, blk int64, path []btree.LONode, zblk ZBlk) {
// XXX locking
head := δFtail.Head()
fileAt := file.PJar().At()
if fileAt != head {
panicf("file.at (@%s) != δFtail.head (@%s)", fileAt, head)
}
if zblk != nil {
zblkAt := zblk.PJar().At()
if zblkAt != head {
panicf("zblk.at (@%s) != δFtail.head (@%s)", zblkAt, head)
}
}
// path.at == head is verified by ΔBtail.Track
foid := file.POid()
if blk == -1 {
// XXX blk = ∞ from beginning ?
......@@ -186,7 +198,7 @@ func (δFtail *ΔFtail) Track(file *ZBigFile, blk int64, path []btree.LONode, zb
δftail, ok := δFtail.byFile[foid]
if !ok {
δftail = &_ΔFileTail{root: roid, vδE: nil /*will need to be rebuilt till past*/}
δftail = &_ΔFileTail{root: roid, vδE: nil /*will need to be rebuilt to past till tail*/}
δFtail.byFile[foid] = δftail
δFtail.trackNew.Add(foid)
}
......@@ -195,7 +207,7 @@ func (δFtail *ΔFtail) Track(file *ZBigFile, blk int64, path []btree.LONode, zb
}
// associate zblk with file, if it was not hole
// associate zblk with root, if it was not hole
if zblk != nil {
zoid := zblk.POid()
zt, ok := δFtail.trackSetZBlk[zoid]
......@@ -221,11 +233,13 @@ func (δFtail *ΔFtail) rebuildAll() (err error) {
defer xerr.Contextf(&err, "ΔFtail rebuildAll")
// XXX locking
δBtail := δFtail.δBtail
δZtail := δBtail.ΔZtail()
db := δBtail.DB()
for foid := range δFtail.trackNew {
δFtail.trackNew.Del(foid)
δftail := δFtail.byFile[foid]
δBtail := δFtail.δBtail
err := δftail.rebuild1(foid, δBtail.ΔZtail(), δBtail.DB())
err := δftail.rebuild1(foid, δZtail, db)
if err != nil {
return err
}
......@@ -517,9 +531,11 @@ func (δftail *_ΔFileTail) forgetPast(revCut zodb.Tid) {
//
// the caller must not modify returned slice.
//
// XXX only tracked blocks are guaranteed to be present.
//
// Note: contrary to regular go slicing, low is exclusive while high is inclusive.
func (δFtail *ΔFtail) SliceByFileRev(zfile *ZBigFile, lo, hi zodb.Tid) /*readonly*/[]*ΔFile {
//fmt.Printf("\n")
//fmt.Printf("\nslice f<%s> (@%s,@%s]\n", zfile.POid(), lo, hi)
xtail.AssertSlice(δFtail, lo, hi)
// XXX locking
......@@ -552,7 +568,7 @@ func (δFtail *ΔFtail) SliceByFileRev(zfile *ZBigFile, lo, hi zodb.Tid) /*reado
if δfTail.Rev == tail {
return δfTail
}
if !(tail <= δfTail.Rev) {
if !(tail < δfTail.Rev) {
panic("tail not ↓")
}
}
......@@ -579,7 +595,7 @@ func (δFtail *ΔFtail) SliceByFileRev(zfile *ZBigFile, lo, hi zodb.Tid) /*reado
// vδE[ie] is next epoch
// vδE[ie-1] is epoch that covers hi
// loop through all epochs till lo
// loop through all epochs from hi till lo
for lastEpoch := false; !lastEpoch ; {
// current epoch
var epoch zodb.Tid
......@@ -611,7 +627,6 @@ func (δFtail *ΔFtail) SliceByFileRev(zfile *ZBigFile, lo, hi zodb.Tid) /*reado
Zinblk[zblk] = inblk.Clone()
}
}
// XXX ZinblkAt
} else {
δE := vδE[ie+1]
root = δE.oldRoot
......@@ -620,6 +635,7 @@ func (δFtail *ΔFtail) SliceByFileRev(zfile *ZBigFile, lo, hi zodb.Tid) /*reado
Zinblk[zblk] = inblk.Clone()
}
}
//fmt.Printf("Zinblk: %v\n", Zinblk)
// vδT for current epoch
vδT := δFtail.δBtail.SliceByRootRev(root, epoch, head) // NOTE @head, not hi
......@@ -630,11 +646,54 @@ func (δFtail *ΔFtail) SliceByFileRev(zfile *ZBigFile, lo, hi zodb.Tid) /*reado
ZinblkAt = epoch
}
// merge cumulative vδT(epoch,head] update to Zinblk, so that
// changes to blocks that were not explicitly requested to be
// tracked, are present in resulting slice uniformly.
//
// For example on
//
// at1 T/B0:a,1:b,2:c δDø δ{0,1,2}
// at2 δT{0:d,1:e} δD{c} δ{0,1,2}
// at3 δTø δD{c,d,e} δ{0,1,2}
// at4 δTø δD{c,e} δ{ 1,2}
//
// if tracked={0} for (at1,at4] query changes to 1 should be
// also all present @at2, @at3 and @at4 - because @at2 both 0
// and 1 are changed in the same tracked bucket. Note that
// changes to 2 should not be present at all.
ZinblkAdj := map[zodb.Oid]setI64{}
for _, δT := range vδT {
for blk, δzblk := range δT.ΔKV {
if δzblk.Old != xbtree.VDEL {
inblk, ok := ZinblkAdj[δzblk.Old]
if ok {
inblk.Del(blk)
}
}
if δzblk.New != xbtree.VDEL {
inblk, ok := ZinblkAdj[δzblk.New]
if !ok {
inblk = setI64{}
ZinblkAdj[δzblk.New] = inblk
}
inblk.Add(blk)
}
}
}
for zblk, inblkAdj := range ZinblkAdj {
inblk, ok := Zinblk[zblk]
if !ok {
Zinblk[zblk] = inblkAdj
} else {
inblk.Update(inblkAdj)
}
}
// merge vδZ and vδT of current epoch
for ((iz >= 0 && vδZ[iz].Rev >= epoch) || it >= 0) {
for ((iz >= 0 && vδZ[iz].Rev > epoch) || it >= 0) {
// δZ that is covered by current Zinblk
// -> update δf
if iz >= 0 {
if iz >= 0 && vδZ[iz].Rev > epoch {
δZ := vδZ[iz]
if ZinblkAt <= δZ.Rev {
//fmt.Printf("δZ @%s\n", δZ.Rev)
......@@ -653,7 +712,7 @@ func (δFtail *ΔFtail) SliceByFileRev(zfile *ZBigFile, lo, hi zodb.Tid) /*reado
// δT -> adjust Zinblk + update δf
if it >= 0 {
δT := vδT[it]
//fmt.Printf("δT @%s\n", δT.Rev)
//fmt.Printf("δT @%s %v\n", δT.Rev, δT.ΔKV)
for blk, δzblk := range δT.ΔKV {
// apply in reverse as we go ←
if δzblk.New != xbtree.VDEL {
......@@ -797,7 +856,7 @@ func (δFtail *ΔFtail) _BlkRevAt(ctx context.Context, zf *ZBigFile, blk int64,
}
// if δBtail does not have entry that covers root[blk] - get it
// through zconn that has any .at ∈ (tail, head].
// through any zconn with .at ∈ (tail, head].
if !zblkExact {
xblktab, err := zconn.Get(ctx, root)
if err != nil {
......
......@@ -29,7 +29,7 @@ package zdata
//
// Since ΔFtail does not recompute anything by itself when tracking set
// changes, and only merges δBtail and δZtail histories on queries, there is no
// need to exercise many different tracking sets. Once again we assume that
// need to exercise many different tracking sets(*). Once again we assume that
// ΔBtail works correctly and verify δFtail only with track=[-∞,∞).
//
// There are 2 testing approaches:
......@@ -40,6 +40,9 @@ package zdata
// states and feed ΔFtail through created database transactions.
//
// TestΔFtail and TestΔFtailRandom implement approaches "a" and "b" correspondingly.
//
// (*) except one small place in SliceByFileRev which handles tracked vs
// untracked set differences and is verified by TestΔFtailSliceUntrackedUniform.
import (
"context"
......@@ -63,7 +66,7 @@ const ø = "ø"
// ΔFTestEntry represents one entry in ΔFtail tests.
type ΔFTestEntry struct {
δblkTab map[int64]string // changes in tree part {} #blk -> ZBlk<name>
δblkTab map[int64]string // changes in tree part {} #blk -> ZBlk<name>
δdataTab setStr // changes to ZBlk objects
}
......@@ -112,6 +115,12 @@ func TestΔFtail(t *testing.T) {
{δT{1:a,3:d,6:j}, δD(b,c,d,f,g,h,i,j)},
{δT{0:i,1:f,4:e,5:e,7:d,8:h}, δD(d,j)},
{δT{}, δD(a,b,c,e,f,g,h,i,j)},
// XXX
{nil, nil},
{δT{0:a}, δD()},
{δT{2:i,3:c,5:d,9:c}, δD(a,b,c,d,e,f,g,h,i)},
{δT{0:j,1:d,2:h,5:g,6:h,7:c,9:h}, δD(d,e,f,h,j)},
}
testq := make(chan ΔFTestEntry)
......@@ -170,19 +179,20 @@ func TestΔFtailRandom(t *testing.T) {
testΔFtail(t, testq)
}
// testΔFtail verifies ΔFtail on sequence on testcases coming from testq.
// testΔFtail verifies ΔFtail on sequence of testcases coming from testq.
func testΔFtail(t_ *testing.T, testq chan ΔFTestEntry) {
t := xbtreetest.NewT(t_)
X := exc.Raiseif
xat := map[zodb.Tid]string{} // tid > "at<i>"
xat := map[zodb.Tid]string{} // tid -> "at<i>"
// start δFtail when zfile does not yet exists
// this way we'll verify how ΔFtail rebuilds vδE for started-to-be-tracked file
t0 := t.CommitTree("øf")
xat[t0.At] = "at0"
t.Logf("# @at0 (%s)", t0.At)
δFtail := NewΔFtail(t.Head().At, t.DB)
// data built via applying changes from testv
vδf := []*ΔFile{} // (rev↑, {}blk)
vδE := []_ΔFileEpoch{} // (rev↑, EPOCH)
blkTab := map[int64]string{} // #blk -> ZBlk<name>
Zinblk := map[string]setI64{} // ZBlk<name> -> which #blk refer to it
blkRevAt := map[zodb.Tid]map[int64]zodb.Tid{} // {} at -> {} #blk -> rev
epochv := []zodb.Tid{} // XXX vs vδE ?
// load dataTab
dataTab := map[string]string{} // ZBlk<name> -> data
......@@ -190,10 +200,24 @@ func testΔFtail(t_ *testing.T, testq chan ΔFTestEntry) {
dataTab[zblki.Name] = zblki.Data
}
// start δFtail when zfile does not yet exists
// this way we'll verify how ΔFtail rebuilds vδE for started-to-be-tracked file
t0 := t.CommitTree("øf")
xat[t0.At] = "at0"
t.Logf("# @at0 (%s)", t0.At)
epochv = append(epochv, t0.At)
δFtail := NewΔFtail(t.Head().At, t.DB)
// create zfile, but do not track it yet
t1 := t.CommitTree(fmt.Sprintf("t0:a D%s", dataTabTxt(dataTab)))
// vδf + friends will be updated after "load zfile"
δt1 := map[int64]string{0:"a"}
t1 := t.CommitTree(fmt.Sprintf("t%s D%s", xbtreetest.KVTxt(δt1), dataTabTxt(dataTab)))
xat[t1.At] = "at1"
t.Logf("# → @at1 (%s) %s\t; not-yet-tracked", t1.At, t1.Tree)
δblk1 := setI64{}
for blk := range δt1 {
δblk1.Add(blk)
}
t.Logf("# → @at1 (%s) δT%s δD{} ; %s\tδ%s *not-yet-tracked", t1.At, xbtreetest.KVTxt(δt1), t1.Tree, δblk1)
δF, err := δFtail.Update(t1.ΔZ); X(err)
if !(δF.Rev == t1.At && len(δF.ByFile) == 0) {
t.Errorf("wrong δF:\nhave {%s, %v}\nwant: {%s, ø}", δF.Rev, δF.ByFile, t1.At)
......@@ -201,6 +225,9 @@ func testΔFtail(t_ *testing.T, testq chan ΔFTestEntry) {
// load zfile via root['treegen/file']
txn, ctx := transaction.New(context.Background())
defer func() {
txn.Abort()
}()
zconn, err := t.DB.Open(ctx, &zodb.ConnOptions{At: t.Head().At, NoPool: true}); X(err)
xzroot, err := zconn.Get(ctx, 0); X(err)
zroot := xzroot.(*zodb.Map)
......@@ -216,42 +243,39 @@ func testΔFtail(t_ *testing.T, testq chan ΔFTestEntry) {
}
zfile.PDeactivate()
// start track zfile[0,∞) from the beginning
// this should make ΔFtail to see all zfile changes
size, path, err := zfile.Size(ctx); X(err)
δFtail.Track(zfile, /*blk*/-1, path, /*zblk*/nil)
if sizeOK := 1*blksize; size != sizeOK { // NOTE maches t1 commit
t.Fatalf("BUG: zfile size: have %d ; want %d", size, sizeOK)
// update vδf + co for t1
vδf = append(vδf, &ΔFile{Rev: t1.At, Epoch: true})
vδE = append(vδE, _ΔFileEpoch{
Rev: t1.At,
oldRoot: zodb.InvalidOid,
newRoot: blktabOid,
newBlkSize: blksize,
oldTrackSetZBlk: nil,
})
epochv = append(epochv, t1.At)
for blk, zblk := range δt1 {
blkTab[blk] = zblk
inblk, ok := Zinblk[zblk]
if !ok {
inblk = setI64{}
Zinblk[zblk] = inblk
}
inblk.Add(blk)
}
// data built via applying changes from testv
vδf := []*ΔFile{ // (rev↑, {}blk)
{Rev: t1.At, Epoch: true},
}
vδE := []_ΔFileEpoch{ // (rev↑, EPOCH)
{
Rev: t1.At,
oldRoot: zodb.InvalidOid,
newRoot: blktabOid,
newBlkSize: blksize,
oldTrackSetZBlk: nil,
},
}
blkTab := map[int64]string{0:"a"} // #blk -> ZBlk<name>
Zinblk := map[string]setI64{} // ZBlk<name> -> which #blk refer to it
blkRevAt := map[zodb.Tid]map[int64]zodb.Tid{} // {} at -> {} #blk -> rev
// retrack should be called after new epoch to track zfile[-∞,∞) again
// start tracking zfile[-∞,∞) from the beginning
// this should make ΔFtail to see all zfile changes
// ( later retrack should be called after new epoch to track zfile[-∞,∞) again )
retrack := func() {
for blk := range blkTab {
_, path, zblk, _, err := zfile.LoadBlk(ctx, blk); X(err)
δFtail.Track(zfile, blk, path, zblk)
}
}
retrack()
epochv := []zodb.Tid{t0.At, t1.At}
// δfstr/vδfstr converts δf/vδf to string taking xat into account
δfstr := func(δf *ΔFile) string {
s := fmt.Sprintf("@%s·%s", xat[δf.Rev], δf.Blocks)
......@@ -544,7 +568,6 @@ func testΔFtail(t_ *testing.T, testq chan ΔFTestEntry) {
// BlkRevAt
blkv := []int64{} // all blocks
if l := len(vδf); l > 0 {
for blk := range blkRevAt[vδf[l-1].Rev] {
......@@ -584,6 +607,134 @@ func testΔFtail(t_ *testing.T, testq chan ΔFTestEntry) {
}
}
// TestΔFtailSliceUntrackedUniform verifies that untracked blocks, if present, are present uniformly in returned slice.
//
// Some changes to untracked blocks, might be seen by ΔFtail, because those
// changes occur in the same BTree bucket that covers another change to a
// tracked block.
//
// Here we verify that if some change to such untracked block is ever present,
// SliceByFileRev returns all changes to that untracked block. In other words
// we verify that no change to untracked block is missed, if any change to that
// block is ever present in returned slice.
func TestΔFtailSliceUntrackedUniform(t_ *testing.T) {
t := xbtreetest.NewT(t_)
X := exc.Raiseif
xat := map[zodb.Tid]string{}
at0 := t.Head().At
xat[at0] = "at0"
δFtail := NewΔFtail(at0, t.DB)
// commit t1. all 0, 1 and 2 are in the same bucket.
t1 := t.CommitTree("T/B0:a,1:b,2:c")
xat[t1.At] = "at1"
δF, err := δFtail.Update(t1.ΔZ); X(err)
// XXX assert δF == ø
_ = δF
t2 := t.CommitTree("t0:d,1:e,2:c Da:a,b:b,c:c2,d:d,e:e") // 0:-a+d 1:-b+e δc₂
xat[t2.At] = "at2"
δF, err = δFtail.Update(t2.ΔZ); X(err)
// XXX assert δF
t3 := t.CommitTree("t0:d,1:e,2:c Da:a,b:b,c:c3,d:d3,e:e3") // δc₃ δd₃ δe₃
xat[t3.At] = "at3"
δF, err = δFtail.Update(t3.ΔZ); X(err)
// XXX assert δF
t4 := t.CommitTree("t0:d,1:e,2:c Da:a,b:b,c:c4,d:d3,e:e4") // δc₄ δe₄
xat[t4.At] = "at4"
δF, err = δFtail.Update(t4.ΔZ); X(err)
// XXX assert δF
// load zfile via root['treegen/file']
// XXX dup
txn, ctx := transaction.New(context.Background())
defer func() {
txn.Abort()
}()
zconn, err := t.DB.Open(ctx, &zodb.ConnOptions{At: t.Head().At, NoPool: true}); X(err)
xzroot, err := zconn.Get(ctx, 0); X(err)
zroot := xzroot.(*zodb.Map)
err = zroot.PActivate(ctx); X(err)
zfile := zroot.Data["treegen/file"].(*ZBigFile)
zroot.PDeactivate()
// foid := zfile.POid()
err = zfile.PActivate(ctx); X(err)
// blksize := zfile.blksize
blktabOid := zfile.blktab.POid()
if blktabOid != t.Root() {
t.Fatalf("BUG: zfile.blktab (%s) != treeroot (%s)", blktabOid, t.Root())
}
zfile.PDeactivate()
// XXX dup
xtrackBlk := func(blk int64) {
_, path, zblk, _, err := zfile.LoadBlk(ctx, blk); X(err)
δFtail.Track(zfile, blk, path, zblk)
}
// δfstr/vδfstr converts δf/vδf to string taking xat into account
// XXX dup
δfstr := func(δf *ΔFile) string {
s := fmt.Sprintf("@%s·%s", xat[δf.Rev], δf.Blocks)
if δf.Epoch {
s += "E"
}
if δf.Size {
s += "S"
}
return s
}
vδfstr := func(vδf []*ΔFile) string {
var s []string
for _, δf := range vδf {
s = append(s, δfstr(δf))
}
return fmt.Sprintf("%s", s)
}
// track 0, but do not track 1 and 2.
// blktab[1] becomes noticed by δBtail because both 0 and 1 are in the same bucket and both are changed @at2.
// blktab[2] remains unnoticed because it is not changed past at1.
xtrackBlk(0)
// (at1, at4] -> changes to both 0 and 1, because they both are changed in the same bucket @at2
lo := t1.At
hi := t4.At
vδf := δFtail.SliceByFileRev(zfile, lo, hi)
vδf_ok := []*ΔFile{
&ΔFile{Rev: t2.At, Blocks: b(0,1), Size: true},
&ΔFile{Rev: t3.At, Blocks: b(0,1), Size: false},
&ΔFile{Rev: t4.At, Blocks: b( 1), Size: false},
}
if !reflect.DeepEqual(vδf, vδf_ok) {
t.Errorf("slice (@%s,@%s]:\nhave: %v\nwant: %v", xat[lo], xat[hi], vδfstr(vδf), vδfstr(vδf_ok))
}
// (at2, at4] -> changes to only 0, because there is no change to 2 via blktab
lo = t2.At
vδf = δFtail.SliceByFileRev(zfile, lo, hi)
vδf_ok = []*ΔFile{
&ΔFile{Rev: t3.At, Blocks: b(0), Size: false},
}
if !reflect.DeepEqual(vδf, vδf_ok) {
t.Errorf("slice (@%s,@%s]:\nhave: %v\nwant: %v", xat[lo], xat[hi], vδfstr(vδf), vδfstr(vδf_ok))
}
// (at3, at4] -> changes to only 0, ----/----
lo = t3.At
vδf = δFtail.SliceByFileRev(zfile, lo, hi)
vδf_ok = []*ΔFile(nil)
if !reflect.DeepEqual(vδf, vδf_ok) {
t.Errorf("slice (@%s,@%s]:\nhave: %v\nwant: %v", xat[lo], xat[hi], vδfstr(vδf), vδfstr(vδf_ok))
}
}
// dataTabTxt returns string representation of {} dataTab.
func dataTabTxt(dataTab map[string]string) string {
......@@ -607,3 +758,13 @@ func dataTabTxt(dataTab map[string]string) string {
return strings.Join(sv, ",")
}
// b is shorthand to create setI64(blocks).
func b(blocks ...int64) setI64 {
s := setI64{}
for _, blk := range blocks {
s.Add(blk)
}
return s
}
......@@ -632,6 +632,7 @@ type BigFile struct {
// blocks that were ever read-accessed (head/ only) XXX locking by bfdir.δFmu ?
// XXX = δFtail.Tracked(f) ?
// XXX goes away if δFtail query returns only tracked blocks
accessed setI64
// inflight loadings of ZBigFile from ZODB.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment