Skip to content

Commit

Permalink
file_block_map: use plaintext strings in the in-memory maps
Browse files Browse the repository at this point in the history
Because the `PathPartString`s might not always have the same
obfuscators, and so the puts won't necessarily match the gets.  But we
have to keep the `PathPartString`s around when we return the list of
all child names.

Issue: HOTPOT-2418
  • Loading branch information
strib committed Apr 11, 2020
1 parent 0158172 commit 7ecaff6
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 19 deletions.
6 changes: 3 additions & 3 deletions go/kbfs/libkbfs/conflict_resolver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1496,11 +1496,11 @@ func TestCRDoActionsWriteConflict(t *testing.T) {
mergedRootPath.TailPointer())
} else if len(blocks) != 1 {
t.Errorf("Unexpected number of blocks")
} else if fblock, ok := blocks[data.NewPathPartString(mergedName, nil)]; !ok {
} else if info, ok := blocks[mergedName]; !ok {
t.Errorf("No block for name %s", mergedName)
} else if fblock.IsInd {
} else if info.block.IsInd {
t.Errorf("Unexpected indirect block")
} else if g, e := fblock.Contents, unmergedData; !reflect.DeepEqual(g, e) {
} else if g, e := info.block.Contents, unmergedData; !reflect.DeepEqual(g, e) {
t.Errorf("Unexpected block contents: %v vs %v", g, e)
}

Expand Down
21 changes: 13 additions & 8 deletions go/kbfs/libkbfs/file_block_map_disk.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,17 @@ import (
"github.com/pkg/errors"
)

type fileBlockMapDiskInfo struct {
pps data.PathPartString
ptr data.BlockPointer
}

// fileBlockMapDisk tracks block info while making a revision, by
// using a disk-based block cache.
type fileBlockMapDisk struct {
dirtyBcache *DirtyBlockCacheDisk
kmd libkey.KeyMetadata
ptrs map[data.BlockPointer]map[data.PathPartString]data.BlockPointer
ptrs map[data.BlockPointer]map[string]fileBlockMapDiskInfo
}

var _ fileBlockMap = (*fileBlockMapDisk)(nil)
Expand All @@ -28,7 +33,7 @@ func newFileBlockMapDisk(
return &fileBlockMapDisk{
dirtyBcache: dirtyBcache,
kmd: kmd,
ptrs: make(map[data.BlockPointer]map[data.PathPartString]data.BlockPointer),
ptrs: make(map[data.BlockPointer]map[string]fileBlockMapDiskInfo),
}
}

Expand All @@ -51,11 +56,11 @@ func (fbmd *fileBlockMapDisk) putTopBlock(

ptrMap, ok := fbmd.ptrs[parentPtr]
if !ok {
ptrMap = make(map[data.PathPartString]data.BlockPointer)
ptrMap = make(map[string]fileBlockMapDiskInfo)
fbmd.ptrs[parentPtr] = ptrMap
}

ptrMap[childName] = ptr
ptrMap[childName.Plaintext()] = fileBlockMapDiskInfo{childName, ptr}
return nil
}

Expand All @@ -66,13 +71,13 @@ func (fbmd *fileBlockMapDisk) GetTopBlock(
if !ok {
return nil, errors.Errorf("No such parent %s", parentPtr)
}
ptr, ok := ptrMap[childName]
info, ok := ptrMap[childName.Plaintext()]
if !ok {
return nil, errors.Errorf(
"No such name %s in parent %s", childName, parentPtr)
}
block, err := fbmd.dirtyBcache.Get(
ctx, fbmd.kmd.TlfID(), ptr, data.MasterBranch)
ctx, fbmd.kmd.TlfID(), info.ptr, data.MasterBranch)
if err != nil {
return nil, err
}
Expand All @@ -92,8 +97,8 @@ func (fbmd *fileBlockMapDisk) getFilenames(
return nil, nil
}
names = make([]data.PathPartString, 0, len(ptrMap))
for name := range ptrMap {
names = append(names, name)
for _, info := range ptrMap {
names = append(names, info.pps)
}
return names, nil
}
21 changes: 13 additions & 8 deletions go/kbfs/libkbfs/file_block_map_memory.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,22 @@ import (
"github.com/pkg/errors"
)

type fileBlockMapMemoryInfo struct {
pps data.PathPartString
block *data.FileBlock
}

// fileBlockMapMemory is an internal structure to track file block
// data in memory when putting blocks.
type fileBlockMapMemory struct {
blocks map[data.BlockPointer]map[data.PathPartString]*data.FileBlock
blocks map[data.BlockPointer]map[string]fileBlockMapMemoryInfo
}

var _ fileBlockMap = (*fileBlockMapMemory)(nil)

func newFileBlockMapMemory() *fileBlockMapMemory {
return &fileBlockMapMemory{
blocks: make(map[data.BlockPointer]map[data.PathPartString]*data.FileBlock),
blocks: make(map[data.BlockPointer]map[string]fileBlockMapMemoryInfo),
}
}

Expand All @@ -30,10 +35,10 @@ func (fbmm *fileBlockMapMemory) putTopBlock(
childName data.PathPartString, topBlock *data.FileBlock) error {
nameMap, ok := fbmm.blocks[parentPtr]
if !ok {
nameMap = make(map[data.PathPartString]*data.FileBlock)
nameMap = make(map[string]fileBlockMapMemoryInfo)
fbmm.blocks[parentPtr] = nameMap
}
nameMap[childName] = topBlock
nameMap[childName.Plaintext()] = fileBlockMapMemoryInfo{childName, topBlock}
return nil
}

Expand All @@ -44,12 +49,12 @@ func (fbmm *fileBlockMapMemory) GetTopBlock(
if !ok {
return nil, errors.Errorf("No such parent %s", parentPtr)
}
block, ok := nameMap[childName]
info, ok := nameMap[childName.Plaintext()]
if !ok {
return nil, errors.Errorf(
"No such name %s in parent %s", childName, parentPtr)
}
return block, nil
return info.block, nil
}

func (fbmm *fileBlockMapMemory) getFilenames(
Expand All @@ -60,8 +65,8 @@ func (fbmm *fileBlockMapMemory) getFilenames(
return nil, nil
}
names = make([]data.PathPartString, 0, len(nameMap))
for name := range nameMap {
names = append(names, name)
for _, info := range nameMap {
names = append(names, info.pps)
}
return names, nil
}

0 comments on commit 7ecaff6

Please sign in to comment.