md_ops: cache next MD results

Instead of hitting the server every single time we need to verify a
revoked key.  For example, when we have to verify lots of MDs written
by the same revoked key when doing garbage collection.

Issue: KBFS-3581
Issue: #1888

libkbfs/errors.go

@@ -1210,3 +1210,16 @@ func (e FolderNotResetOnServer) Error() string {
 	return fmt.Sprintf("Folder %s is not yet reset on the server; "+
 		"contact Keybase for help", e.h.GetCanonicalPath())
 }
+
+// NextMDNotCachedError indicates we haven't cached the next MD after
+// the given Merkle seqno.
+type NextMDNotCachedError struct {
+	TlfID     tlf.ID
+	RootSeqno keybase1.Seqno
+}
+
+// Error implements the Error interface for NextMDNotCachedError.
+func (e NextMDNotCachedError) Error() string {
+	return fmt.Sprintf("The MD following %d for folder %s is not cached",
+		e.RootSeqno, e.TlfID)
+}

libkbfs/interfaces.go

@@ -1035,6 +1035,16 @@ type MDCache interface {
 	// ChangeHandleForID moves an ID to be under a new handle, if the
 	// ID is cached already.
 	ChangeHandleForID(oldHandle *TlfHandle, newHandle *TlfHandle)
+	// GetNextMD returns a cached view of the next MD following the
+	// given global Merkle root.
+	GetNextMD(tlfID tlf.ID, rootSeqno keybase1.Seqno) (
+		nextKbfsRoot *kbfsmd.MerkleRoot, nextMerkleNodes [][]byte,
+		nextRootSeqno keybase1.Seqno, err error)
+	// PutNextMD caches a view of the next MD following the given
+	// global Merkle root.
+	PutNextMD(tlfID tlf.ID, rootSeqno keybase1.Seqno,
+		nextKbfsRoot *kbfsmd.MerkleRoot, nextMerkleNodes [][]byte,
+		nextRootSeqno keybase1.Seqno) error
 }
 
 // KeyCache handles caching for both TLFCryptKeys and BlockCryptKeys.

libkbfs/md_ops.go

@@ -303,11 +303,23 @@ func (md *MDOpsStandard) checkRevisionCameBeforeMerkle(
 	ctx = context.WithValue(ctx, ctxMDOpsSkipKeyVerification, struct{}{})
 
 	kbfsRoot, merkleNodes, rootSeqno, err :=
-		md.config.MDServer().FindNextMD(ctx, rmds.MD.TlfID(),
-			root.Seqno)
-	if err != nil {
+		md.config.MDCache().GetNextMD(rmds.MD.TlfID(), root.Seqno)
+	switch errors.Cause(err).(type) {
+	case nil:
+	case NextMDNotCachedError:
+		md.log.CDebugf(ctx, "Finding next MD for TLF %s after global root %d",
+			rmds.MD.TlfID(), root.Seqno)
+		kbfsRoot, merkleNodes, rootSeqno, err =
+			md.config.MDServer().FindNextMD(ctx, rmds.MD.TlfID(), root.Seqno)
+		if err != nil {
+			return err
+		}
+		err = md.config.MDCache().PutNextMD(
+			rmds.MD.TlfID(), root.Seqno, kbfsRoot, merkleNodes, rootSeqno)
+	default:
 		return err
 	}
+
 	if len(merkleNodes) == 0 {
 		// This can happen legitimately if we are still inside the
 		// error window and no new merkle trees have been made yet, or

libkbfs/md_ops_test.go

@@ -1300,13 +1300,15 @@ func testMDOpsVerifyRevokedDeviceWrite(t *testing.T, ver kbfsmd.MetadataVer) {
 	require.True(t, cacheable)
 
 	t.Log("Make the server return no information, but outside the max gap")
+	config.MDCache().(*MDCacheStandard).nextMDLRU.Purge()
 	clock.Add(maxAllowedMerkleGap) // already added one minute above
 	_, err = mdOps.verifyKey(
 		ctx, allRMDSs[0], allRMDSs[0].MD.GetLastModifyingUser(),
 		allRMDSs[0].SigInfo.VerifyingKey, irmd)
 	require.Error(t, err)
 
 	t.Log("Make the server return a root, but which is outside the max gap")
+	config.MDCache().(*MDCacheStandard).nextMDLRU.Purge()
 	root.Timestamp = clock.Now().Unix()
 	mdServer.nextMerkleRoot = root
 	mdServer.nextMerkleNodes = [][]byte{leafBytes}

libkbfs/mdcache.go

@@ -8,6 +8,7 @@ import (
 	"sync"
 
 	lru "github.com/hashicorp/golang-lru"
+	"github.com/keybase/client/go/protocol/keybase1"
 	"github.com/keybase/kbfs/kbfsmd"
 	"github.com/keybase/kbfs/tlf"
 	"github.com/pkg/errors"
@@ -21,6 +22,8 @@ type MDCacheStandard struct {
 	lock  sync.RWMutex
 	lru   *lru.Cache
 	idLRU *lru.Cache
+
+	nextMDLRU *lru.Cache
 }
 
 type mdCacheKey struct {
@@ -29,7 +32,10 @@ type mdCacheKey struct {
 	bid kbfsmd.BranchID
 }
 
-const defaultMDCacheCapacity = 5000
+const (
+	defaultMDCacheCapacity     = 5000
+	defaultNextMDCacheCapacity = 100
+)
 
 // NewMDCacheStandard constructs a new MDCacheStandard using the given
 // cache capacity.
@@ -42,7 +48,13 @@ func NewMDCacheStandard(capacity int) *MDCacheStandard {
 	if err != nil {
 		return nil
 	}
-	return &MDCacheStandard{lru: mdLRU, idLRU: idLRU}
+	// Hard-code the nextMD cache size to something small, since only
+	// one entry is used for each revoked device we need to verify.
+	nextMDLRU, err := lru.New(defaultNextMDCacheCapacity)
+	if err != nil {
+		return nil
+	}
+	return &MDCacheStandard{lru: mdLRU, idLRU: idLRU, nextMDLRU: nextMDLRU}
 }
 
 // Get implements the MDCache interface for MDCacheStandard.
@@ -161,3 +173,41 @@ func (md *MDCacheStandard) ChangeHandleForID(
 	md.idLRU.Add(newKey, tmp)
 	return
 }
+
+type mdcacheNextMDKey struct {
+	tlfID     tlf.ID
+	rootSeqno keybase1.Seqno
+}
+
+type mdcacheNextMDVal struct {
+	nextKbfsRoot    *kbfsmd.MerkleRoot
+	nextMerkleNodes [][]byte
+	nextRootSeqno   keybase1.Seqno
+}
+
+// GetNextMD implements the MDCache interface for MDCacheStandard.
+func (md *MDCacheStandard) GetNextMD(tlfID tlf.ID, rootSeqno keybase1.Seqno) (
+	nextKbfsRoot *kbfsmd.MerkleRoot, nextMerkleNodes [][]byte,
+	nextRootSeqno keybase1.Seqno, err error) {
+	key := mdcacheNextMDKey{tlfID, rootSeqno}
+	tmp, ok := md.nextMDLRU.Get(key)
+	if !ok {
+		return nil, nil, 0, NextMDNotCachedError{tlfID, rootSeqno}
+	}
+	val, ok := tmp.(mdcacheNextMDVal)
+	if !ok {
+		return nil, nil, 0, errors.Errorf(
+			"Bad next MD for %s, seqno=%d", tlfID, rootSeqno)
+	}
+	return val.nextKbfsRoot, val.nextMerkleNodes, val.nextRootSeqno, nil
+}
+
+// PutNextMD implements the MDCache interface for MDCacheStandard.
+func (md *MDCacheStandard) PutNextMD(
+	tlfID tlf.ID, rootSeqno keybase1.Seqno, nextKbfsRoot *kbfsmd.MerkleRoot,
+	nextMerkleNodes [][]byte, nextRootSeqno keybase1.Seqno) error {
+	key := mdcacheNextMDKey{tlfID, rootSeqno}
+	val := mdcacheNextMDVal{nextKbfsRoot, nextMerkleNodes, nextRootSeqno}
+	md.nextMDLRU.Add(key, val)
+	return nil
+}