X fixed

go/neo/client/cache.go

@@ -74,8 +74,8 @@ type oidCacheEntry struct {
 
 // revCacheEntry is information about 1 cached oid revision
 type revCacheEntry struct {
-	inLRU  listHead		// in Cache.lru
 	parent *oidCacheEntry	// oidCacheEntry holding us
+	inLRU  listHead		// in Cache.lru; protected by Cache.gcMu
 
 	// we know that loadBefore(oid, .before) will give this .serial:oid.
 	//
@@ -98,7 +98,8 @@ type revCacheEntry struct {
 	serial zodb.Tid
 	err    error
 
-	ready chan struct{} // closed when loading finished
+	ready     chan struct{} // closed when loading finished
+	accounted bool		// whether rce size accounted in cache size; protected by .parent's lock
 }
 
 // lock order: Cache.mu   > oidCacheEntry > (?) revCacheEntry
@@ -325,12 +326,16 @@ func (c *Cache) loadRCE(rce *revCacheEntry, oid zodb.Oid) {
 		rcePrev := oce.rcev[i-1]
 		if rcePrev.loaded() && tryMerge(rcePrev, rce, rce, oid) {
 			rcePrevDropped = rcePrev
-			// XXX not always right - e.g. if rcePrev did not yet took oce lock
-			// XXX ^^^ test for this
-			δsize -= len(rcePrev.data)
+			if rcePrev.accounted {
+				δsize -= len(rcePrev.data)
+			}
 		}
 	}
 
+	if !rceDropped {
+		rce.accounted = true
+	}
+
 	oce.Unlock()
 
 	// update lru & cache size

go/neo/client/cache_test.go

@@ -125,7 +125,7 @@ func TestCache(t *testing.T) {
 				{7, nil, ioerr},
 				{10, world, nil},
 				{16, zz, nil},
-				{18, www, nil},		// XXX +2
+				{20, www, nil},
 			},
 		},
 	}
@@ -355,35 +355,31 @@ func TestCache(t *testing.T) {
 	checkOCE(1, rce1_b4, rce1_b7, rce1_b8, rce1_b10, rce1_b16, rce1_b18)
 	checkMRU(14, rce1_b18, rce1_b16, rce1_b10, rce1_b8, rce1_b7, rce1_b4)
 
-	// XXX temp
-	return
-
-/*
-	// load =17 -> <16 merged with <18
-	checkLoad(xideq(1,17), nil, 0, &zodb.ErrXidMissing{xideq(1,17)})
-	ok1(len(oce1.rcev) == 5)
-	rce1_b18 := oce1.rcev[4]
-	ok1(rce1_b18 != rce1_b16)
-	checkRCE(rce1_b18, 18, 10, world, nil)
-	checkOCE(1,  rce1_b4, rce1_b7, rce1_b8, rce1_b10, rce1_b18)
-	checkMRU(12, rce1_b18, rce1_b10, rce1_b8, rce1_b7, rce1_b4)
-
-	// load =18 -> new <19
-	checkLoad(xideq(1,18), www, 18, nil)
-	ok1(len(oce1.rcev) == 6)
-	rce1_b19 := oce1.rcev[5]
-	checkRCE(rce1_b19, 19, 18, www, nil)
-	checkOCE(1,  rce1_b4, rce1_b7, rce1_b8, rce1_b10, rce1_b18, rce1_b19)
-	checkMRU(15, rce1_b19, rce1_b18, rce1_b10, rce1_b8, rce1_b7, rce1_b4)
-
-	// load =19 -> <19 merged with <20
+	// load =19 -> <18 merged with <20
 	checkLoad(xideq(1,19), nil, 0, &zodb.ErrXidMissing{xideq(1,19)})
 	ok1(len(oce1.rcev) == 6)
 	rce1_b20 := oce1.rcev[5]
-	ok1(rce1_b20 != rce1_b19)
-	checkRCE(rce1_b20, 20, 18, www, nil)
-	checkOCE(1,  rce1_b4, rce1_b7, rce1_b8, rce1_b10, rce1_b18, rce1_b20)
-	checkMRU(15, rce1_b20, rce1_b18, rce1_b10, rce1_b8, rce1_b7, rce1_b4)
+	ok1(rce1_b20 != rce1_b18)
+	checkRCE(rce1_b20, 20, 16, zz, nil)
+	checkOCE(1,  rce1_b4, rce1_b7, rce1_b8, rce1_b10, rce1_b16, rce1_b20)
+	checkMRU(14, rce1_b20, rce1_b16, rce1_b10, rce1_b8, rce1_b7, rce1_b4)
+
+	// load =20 -> new <21
+	checkLoad(xideq(1,20), www, 20, nil)
+	ok1(len(oce1.rcev) == 7)
+	rce1_b21 := oce1.rcev[6]
+	checkRCE(rce1_b21, 21, 20, www, nil)
+	checkOCE(1,  rce1_b4, rce1_b7, rce1_b8, rce1_b10, rce1_b16, rce1_b20, rce1_b21)
+	checkMRU(17, rce1_b21, rce1_b20, rce1_b16, rce1_b10, rce1_b8, rce1_b7, rce1_b4)
+
+	// load =21 -> <21 merged with <22
+	checkLoad(xideq(1,21), nil, 0, &zodb.ErrXidMissing{xideq(1,21)})
+	ok1(len(oce1.rcev) == 7)
+	rce1_b22 := oce1.rcev[6]
+	ok1(rce1_b22 != rce1_b21)
+	checkRCE(rce1_b22, 22, 20, www, nil)
+	checkOCE(1,  rce1_b4, rce1_b7, rce1_b8, rce1_b10, rce1_b16, rce1_b20, rce1_b22)
+	checkMRU(17, rce1_b22, rce1_b20, rce1_b16, rce1_b10, rce1_b8, rce1_b7, rce1_b4)
 
 
 	// ---- verify rce lookup for must be cached entries ----
@@ -409,18 +405,18 @@ func TestCache(t *testing.T) {
 	checkLookup(xideq(1,19), rce1_b20)
 	checkLookup(xidlt(1,19), rce1_b20)
 	checkLookup(xideq(1,18), rce1_b20)
-	checkLookup(xidlt(1,18), rce1_b18)
-	checkLookup(xideq(1,17), rce1_b18)
-	checkLookup(xidlt(1,17), rce1_b18)
-	checkLookup(xideq(1,16), rce1_b18)
-	checkLookup(xidlt(1,16), rce1_b18)
-	checkLookup(xideq(1,15), rce1_b18)
-	checkLookup(xidlt(1,15), rce1_b18)
-	checkLookup(xideq(1,12), rce1_b18)
-	checkLookup(xidlt(1,12), rce1_b18)
-	checkLookup(xideq(1,11), rce1_b18)
-	checkLookup(xidlt(1,11), rce1_b18)
-	checkLookup(xideq(1,10), rce1_b18)
+	checkLookup(xidlt(1,18), rce1_b20)
+	checkLookup(xideq(1,17), rce1_b20)
+	checkLookup(xidlt(1,17), rce1_b20)
+	checkLookup(xideq(1,16), rce1_b20)
+	checkLookup(xidlt(1,16), rce1_b16)
+	checkLookup(xideq(1,15), rce1_b16)
+	checkLookup(xidlt(1,15), rce1_b16)
+	checkLookup(xideq(1,12), rce1_b16)
+	checkLookup(xidlt(1,12), rce1_b16)
+	checkLookup(xideq(1,11), rce1_b16)
+	checkLookup(xidlt(1,11), rce1_b16)
+	checkLookup(xideq(1,10), rce1_b16)
 	checkLookup(xidlt(1,10), rce1_b10)
 
 	// <9 must be separate from <8 and <10 because it is IO error there
@@ -428,8 +424,8 @@ func TestCache(t *testing.T) {
 	ok1(new9)
 	c.loadRCE(rce1_b9, 1)
 	checkRCE(rce1_b9, 9, 0, nil, ioerr)
-	checkOCE(1, rce1_b4, rce1_b7, rce1_b8, rce1_b9, rce1_b10, rce1_b18, rce1_b20)
-	checkMRU(15, rce1_b9, rce1_b20, rce1_b18, rce1_b10, rce1_b8, rce1_b7, rce1_b4)
+	checkOCE(1, rce1_b4, rce1_b7, rce1_b8, rce1_b9, rce1_b10, rce1_b16, rce1_b20, rce1_b22)
+	checkMRU(17, rce1_b9, rce1_b22, rce1_b20, rce1_b16, rce1_b10, rce1_b8, rce1_b7, rce1_b4)
 
 	checkLookup(xideq(1,8), rce1_b9)
 	checkLookup(xidlt(1,8), rce1_b8)
@@ -453,14 +449,13 @@ func TestCache(t *testing.T) {
 	checkLookup(xidlt(1,1), rce1_b4)
 
 	// ---- verify how LRU changes for in-cache loads ----
-	checkMRU(15, rce1_b9, rce1_b20, rce1_b18, rce1_b10, rce1_b8, rce1_b7, rce1_b4)
+	checkMRU(17, rce1_b9, rce1_b22, rce1_b20, rce1_b16, rce1_b10, rce1_b8, rce1_b7, rce1_b4)
 
 	checkLoad(xidlt(1,7), hello, 4, nil)
-	checkMRU(15, rce1_b7, rce1_b9, rce1_b20, rce1_b18, rce1_b10, rce1_b8, rce1_b4)
+	checkMRU(17, rce1_b7, rce1_b9, rce1_b22, rce1_b20, rce1_b16, rce1_b10, rce1_b8, rce1_b4)
 
-	checkLoad(xidlt(1,18), world, 10, nil)
-	checkMRU(15, rce1_b18, rce1_b7, rce1_b9, rce1_b20, rce1_b10, rce1_b8, rce1_b4)
-*/
+	checkLoad(xidlt(1,16), world, 10, nil)
+	checkMRU(17, rce1_b16, rce1_b7, rce1_b9, rce1_b22, rce1_b20, rce1_b10, rce1_b8, rce1_b4)
 
 
 	// XXX verify LRU eviction