Skip to content

Commit

Permalink
dedup leafidentityhash values ahead of SQL lookup of existing leaves (#…
Browse files Browse the repository at this point in the history
…3607)

Signed-off-by: Bob Callaway <[email protected]>
  • Loading branch information
bobcallaway authored Aug 27, 2024
1 parent 685353d commit 19d061d
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 2 deletions.
11 changes: 9 additions & 2 deletions storage/mysql/log_storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -476,11 +476,18 @@ func (t *logTreeTX) QueueLeaves(ctx context.Context, leaves []*trillian.LogLeaf,
return existingLeaves, nil
}

// For existing leaves, we need to retrieve the contents. First collate the desired LeafIdentityHash values.
// For existing leaves, we need to retrieve the contents. First collate the desired LeafIdentityHash values
// We deduplicate the hashes to address https://github.com/google/trillian/issues/3603 but will be mapped
// back to the existingLeaves slice below
uniqueLeafMap := make(map[string]struct{}, len(existingLeaves))
var toRetrieve [][]byte
for _, existing := range existingLeaves {
if existing != nil {
toRetrieve = append(toRetrieve, existing.LeafIdentityHash)
key := string(existing.LeafIdentityHash)
if _, ok := uniqueLeafMap[key]; !ok {
uniqueLeafMap[key] = struct{}{}
toRetrieve = append(toRetrieve, existing.LeafIdentityHash)
}
}
}
results, err := t.getLeafDataByIdentityHash(ctx, toRetrieve)
Expand Down
7 changes: 7 additions & 0 deletions storage/mysql/log_storage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ func TestQueueDuplicateLeaf(t *testing.T) {
leaves := createTestLeaves(int64(count), 10)
leaves2 := createTestLeaves(int64(count), 12)
leaves3 := createTestLeaves(3, 100)
leaves4 := createTestLeaves(3, 105)

// Note that tests accumulate queued leaves on top of each other.
tests := []struct {
Expand All @@ -161,6 +162,12 @@ func TestQueueDuplicateLeaf(t *testing.T) {
leaves: []*trillian.LogLeaf{leaves[0], leaves3[0], leaves[1], leaves3[1], leaves[2]},
want: []*trillian.LogLeaf{leaves[0], nil, leaves[1], nil, leaves[2]},
},
{
// we explictly reuse tests that have already been integrated to test issue 3603
desc: "[100, 100, 106, 101, 107]",
leaves: []*trillian.LogLeaf{leaves3[0], leaves3[0], leaves4[1], leaves3[1], leaves4[2]},
want: []*trillian.LogLeaf{leaves3[0], leaves3[0], leaves4[1], leaves3[1], leaves4[2]},
},
}

for _, test := range tests {
Expand Down

0 comments on commit 19d061d

Please sign in to comment.