Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: reduce allocations in the Time Window Block Selector #4255

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: reduce allocations in the Time Window Block Selector #4255

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 67 additions & 10 deletions tempodb/compaction_block_selector.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
package tempodb

import (
"fmt"
"sort"
"strconv"
"strings"
"time"

"github.com/grafana/tempo/tempodb/backend"
Expand Down Expand Up @@ -57,6 +58,9 @@ func newTimeWindowBlockSelector(blocklist []*backend.BlockMeta, maxCompactionRan
now := time.Now()
currWindow := twbs.windowForTime(now)
activeWindow := twbs.windowForTime(now.Add(-activeWindowDuration))
var builder strings.Builder
// Preallocate
twbs.entries = make([]timeWindowBlockEntry, 0, len(blocklist))

for _, b := range blocklist {
w := twbs.windowForBlock(b)
Expand All @@ -74,29 +78,82 @@ func newTimeWindowBlockSelector(blocklist []*backend.BlockMeta, maxCompactionRan
}

age := currWindow - w
builder.Reset()

if activeWindow <= w {
// Grow size: 2+20+1+16+1+20
builder.Grow(60)
Copy link
Member

@joe-elliott joe-elliott Oct 31, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a very tiny %age of all compactor allocations and the previous implementation had a lot of clarity on what the group/order/hash strings looked like. the current implementation loses this a bit

i'm fine with moving forward with this, but we need the new implementation to read as cleanly as the old given that the value gained is so small

builder.WriteString("A-")
builder.WriteString(strconv.FormatUint(uint64(b.CompactionLevel), 10))
builder.WriteByte('-')
builder.WriteString(strconv.FormatInt(age, 16))
builder.WriteByte('-')
builder.WriteString(strconv.FormatUint(uint64(b.ReplicationFactor), 10))
// inside active window.
// Group by compaction level and window.
// Choose lowest compaction level and most recent windows first.
entry.group = fmt.Sprintf("A-%v-%016X-%v", b.CompactionLevel, age, b.ReplicationFactor)

entry.group = builder.String()

builder.Reset()
// Grow size: 16+1+version+1+16
builder.Grow(34 + len(entry.meta.Version))
builder.WriteString((strconv.FormatInt(entry.meta.TotalObjects, 16)))
builder.WriteByte('-')
builder.WriteString(entry.meta.Version)
builder.WriteByte('-')
builder.WriteString(strconv.FormatUint(entry.meta.DedicatedColumnsHash(), 16))
// Within group choose smallest blocks first.
// update after parquet: we want to make sure blocks of the same version end up together
// update afert vParquet3: we want to make sure blocks of the same dedicated columns end up together
entry.order = fmt.Sprintf("%016X-%v-%016X", entry.meta.TotalObjects, entry.meta.Version, entry.meta.DedicatedColumnsHash())
entry.order = builder.String()

builder.Reset()
// Grow size: 36+1+20+1+19+1+20
builder.Grow(98)
builder.WriteString(b.TenantID)
builder.WriteByte('-')
builder.WriteString(strconv.FormatUint(uint64(b.CompactionLevel), 10))
builder.WriteByte('-')
builder.WriteString(strconv.FormatInt(w, 10))
builder.WriteByte('-')
builder.WriteString(strconv.FormatUint(uint64(b.ReplicationFactor), 10))
entry.hash = builder.String()

entry.hash = fmt.Sprintf("%v-%v-%v-%v", b.TenantID, b.CompactionLevel, w, b.ReplicationFactor)
} else {
// Grow size: 2+16+1+20
builder.Grow(40)
builder.WriteString("B-")
builder.WriteString(strconv.FormatInt(age, 16))
builder.WriteByte('-')
builder.WriteString(strconv.FormatUint(uint64(b.ReplicationFactor), 10))
// outside active window.
// Group by window only. Choose most recent windows first.
entry.group = fmt.Sprintf("B-%016X-%v", age, b.ReplicationFactor)

entry.group = builder.String()

builder.Reset()
// Grow size: 20+1+16+1+version+1+16
builder.Grow(55 + len(entry.meta.Version))
builder.WriteString((strconv.FormatUint(uint64(b.CompactionLevel), 10)))
builder.WriteByte('-')
builder.WriteString((strconv.FormatInt(entry.meta.TotalObjects, 16)))
builder.WriteByte('-')
builder.WriteString(entry.meta.Version)
builder.WriteByte('-')
builder.WriteString(strconv.FormatUint(entry.meta.DedicatedColumnsHash(), 16))
// Within group chose lowest compaction lvl and smallest blocks first.
// update after parquet: we want to make sure blocks of the same version end up together
// update afert vParquet3: we want to make sure blocks of the same dedicated columns end up together
entry.order = fmt.Sprintf("%v-%016X-%v-%016X", b.CompactionLevel, entry.meta.TotalObjects, entry.meta.Version, entry.meta.DedicatedColumnsHash())

entry.hash = fmt.Sprintf("%v-%v-%v", b.TenantID, w, b.ReplicationFactor)
entry.order = builder.String()

builder.Reset()
// Grow size: 36+1+19+1+20
builder.Grow(77)
builder.WriteString(b.TenantID)
builder.WriteByte('-')
builder.WriteString((strconv.FormatInt(w, 10)))
builder.WriteByte('-')
builder.WriteString(strconv.FormatUint(uint64(b.ReplicationFactor), 10))
entry.hash = builder.String()
}

twbs.entries = append(twbs.entries, entry)
Expand Down
24 changes: 24 additions & 0 deletions tempodb/compaction_block_selector_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -846,3 +846,27 @@ func TestTimeWindowBlockSelectorBlocksToCompact(t *testing.T) {
})
}
}

func BenchmarkBlockSelector(b *testing.B) {
blockMetas := make([]*backend.BlockMeta, 100000)
for i := range blockMetas {
blockMetas[i] = &backend.BlockMeta{
Size_: 1000,
TotalRecords: 100,
BlockID: backend.MustParse("00000000-0000-0000-0000-000000000000"),
DataEncoding: "json",
Encoding: backend.EncGZIP,
IndexPageSize: 13,
Version: "glarg",
TotalObjects: 540,
DedicatedColumns: backend.DedicatedColumns{
{Scope: "span", Name: "foo", Type: "int"},
},
}
}
b.ResetTimer()

for i := 0; i < b.N; i++ {
_ = newTimeWindowBlockSelector(blockMetas, time.Second, 100, uint64(1024*1024), defaultMaxInputBlocks, defaultMinInputBlocks)
}
}
Loading