Skip to content

Commit

Permalink
Address minor items from the review
Browse files Browse the repository at this point in the history
  • Loading branch information
pmm-sumo committed Jun 13, 2022
1 parent 471528b commit 614d1cd
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 11 deletions.
19 changes: 9 additions & 10 deletions extension/storage/filestorage/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ const (
elapsedKey = "elapsed"
directoryKey = "directory"
tempDirectoryKey = "tempDirectory"

oneMiB = 1048576
)

type fileStorageClient struct {
Expand Down Expand Up @@ -257,32 +259,29 @@ func (c *fileStorageClient) shouldCompact() bool {
return false
}

totalSize, dataSize, err := c.getDbSize()
totalSizeBytes, dataSizeBytes, err := c.getDbSize()
if err != nil {
c.logger.Error("failed to get db size", zap.Error(err))
return false
}

c.logger.Debug("shouldCompact check",
zap.Int64("totalSize", totalSize),
zap.Int64("dataSize", dataSize))
zap.Int64("totalSizeBytes", totalSizeBytes),
zap.Int64("dataSizeBytes", dataSizeBytes))

if dataSize > c.compactionCfg.ReboundNeededThresholdMiB*1048576 ||
totalSize < c.compactionCfg.ReboundTriggerThresholdMiB*1048576 {
if dataSizeBytes > c.compactionCfg.ReboundNeededThresholdMiB*oneMiB ||
totalSizeBytes < c.compactionCfg.ReboundTriggerThresholdMiB*oneMiB {
return false
}

c.logger.Debug("shouldCompact returns true",
zap.Int64("totalSize", totalSize),
zap.Int64("dataSize", dataSize))
zap.Int64("totalSizeBytes", totalSizeBytes),
zap.Int64("dataSizeBytes", dataSizeBytes))

return true
}

func (c *fileStorageClient) getDbSize() (totalSizeResult int64, dataSizeResult int64, errResult error) {
c.compactionMutex.Lock()
defer c.compactionMutex.Unlock()

var totalSize int64

err := c.db.View(func(tx *bbolt.Tx) error {
Expand Down
14 changes: 13 additions & 1 deletion extension/storage/filestorage/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"go.etcd.io/bbolt"
"go.opentelemetry.io/collector/extension/experimental/storage"
"go.uber.org/zap"
"go.uber.org/zap/zaptest/observer"
)

func TestClientOperations(t *testing.T) {
Expand Down Expand Up @@ -272,6 +273,11 @@ func TestClientReboundCompaction(t *testing.T) {

require.Eventually(t,
func() bool {
// The check is performed while the database might be compacted, hence we're reusing the mutex here
// (getDbSize is not called from outside the compaction loop otherwise)
client.compactionMutex.Lock()
defer client.compactionMutex.Unlock()

totalSize, realSize, dbErr := client.getDbSize()
require.NoError(t, dbErr)
return totalSize < entrySize && realSize < entrySize
Expand All @@ -281,12 +287,15 @@ func TestClientReboundCompaction(t *testing.T) {
}

func TestClientConcurrentCompaction(t *testing.T) {
logCore, logObserver := observer.New(zap.DebugLevel)
logger := zap.New(logCore)

tempDir := t.TempDir()
dbFile := filepath.Join(tempDir, "my_db")

checkInterval := time.Millisecond

client, err := newClient(zap.NewNop(), dbFile, time.Second, &CompactionConfig{
client, err := newClient(logger, dbFile, time.Second, &CompactionConfig{
OnRebound: true,
CheckInterval: checkInterval,
ReboundNeededThresholdMiB: 1,
Expand Down Expand Up @@ -339,6 +348,9 @@ func TestClientConcurrentCompaction(t *testing.T) {
}

wg.Wait()

// The actual number might vary a bit depending on the actual intervals
require.GreaterOrEqual(t, len(logObserver.FilterMessage("finished compaction").All()), 3)
}

func BenchmarkClientGet(b *testing.B) {
Expand Down

0 comments on commit 614d1cd

Please sign in to comment.