Skip to content

Commit

Permalink
storagenode/pieces: update used space on trash-lazyfilewalker completion
Browse files Browse the repository at this point in the history
Turns out the lazyfilewalker does not update the used space
cache after cleaning the trash, well, because it doesn't
have access to the cache.
We update the cache with the bytesDeleted returned by the
subprocess.

Also fixes a small flaky test

#6950

Change-Id: I7e6cfa37fff143a8abcc0d0ca844ad4659b4cb4a
  • Loading branch information
profclems authored and andriikotko committed May 9, 2024
1 parent a4296c1 commit d68abcf
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 21 deletions.
24 changes: 4 additions & 20 deletions storagenode/bandwidth/db_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,22 +145,14 @@ func TestEgressSummary(t *testing.T) {

// test egress summarizing.
{
usage, err := bandwidthdb.EgressSummary(ctx, now.Add(-10*time.Hour), now.Add(10*time.Hour))
require.NoError(t, err)
require.Equal(t, &bandwidth.Usage{}, usage)

// only range capturing second satellite.
usage, err = bandwidthdb.EgressSummary(ctx, time.Time{}, time.Now())
usage, err := bandwidthdb.EgressSummary(ctx, time.Time{}, time.Now())
require.NoError(t, err)
require.Equal(t, expectedEgressUsageTotal, usage)
}

{
usageBySatellite, err := bandwidthdb.SummaryBySatellite(ctx, now.Add(-10*time.Hour), now.Add(10*time.Hour))
require.NoError(t, err)
require.Equal(t, map[storj.NodeID]*bandwidth.Usage{}, usageBySatellite)

usageBySatellite, err = bandwidthdb.SummaryBySatellite(ctx, time.Time{}, time.Now())
usageBySatellite, err := bandwidthdb.SummaryBySatellite(ctx, time.Time{}, time.Now())
require.NoError(t, err)
require.Equal(t, expectedEgressUsageBySatellite, usageBySatellite)
}
Expand Down Expand Up @@ -208,22 +200,14 @@ func TestIngressSummary(t *testing.T) {

// test ingress summarizing.
{
usage, err := bandwidthdb.IngressSummary(ctx, now.Add(-10*time.Hour), now.Add(10*time.Hour))
require.NoError(t, err)
require.Equal(t, &bandwidth.Usage{}, usage)

// only range capturing second satellite.
usage, err = bandwidthdb.IngressSummary(ctx, time.Time{}, time.Now())
usage, err := bandwidthdb.IngressSummary(ctx, time.Time{}, time.Now())
require.NoError(t, err)
require.Equal(t, expectedIngressUsage, usage)
}

{
usageBySatellite, err := bandwidthdb.SummaryBySatellite(ctx, now.Add(-10*time.Hour), now.Add(10*time.Hour))
require.NoError(t, err)
require.Equal(t, map[storj.NodeID]*bandwidth.Usage{}, usageBySatellite)

usageBySatellite, err = bandwidthdb.SummaryBySatellite(ctx, time.Time{}, time.Now())
usageBySatellite, err := bandwidthdb.SummaryBySatellite(ctx, time.Time{}, time.Now())
require.NoError(t, err)
require.Equal(t, expectedIngressUsageBySatellite, usageBySatellite)
}
Expand Down
2 changes: 2 additions & 0 deletions storagenode/pieces/lazyfilewalker/supervisor.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,8 @@ func (fw *Supervisor) WalkSatellitePiecesToTrash(ctx context.Context, satelliteI
}

// WalkCleanupTrash deletes per-day trash directories which are older than the given time.
// The lazyfilewalker does not update the space used by the trash so the caller should update the space used
// after the filewalker completes.
func (fw *Supervisor) WalkCleanupTrash(ctx context.Context, satelliteID storj.NodeID, dateBefore time.Time) (bytesDeleted int64, keysDeleted []storj.PieceID, err error) {
defer mon.Task()(&ctx)(&err)

Expand Down
6 changes: 5 additions & 1 deletion storagenode/pieces/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -416,7 +416,11 @@ func (store *Store) EmptyTrash(ctx context.Context, satelliteID storj.NodeID, tr
defer mon.Task()(&ctx)(&err)

if store.config.EnableLazyFilewalker && store.lazyFilewalker != nil {
_, _, err := store.lazyFilewalker.WalkCleanupTrash(ctx, satelliteID, trashedBefore)
bytesDeleted, _, err := store.lazyFilewalker.WalkCleanupTrash(ctx, satelliteID, trashedBefore)
// The lazy filewalker does not update the space used by the trash so we need to update it here.
if cache, ok := store.blobs.(*BlobsUsageCache); ok {
cache.Update(ctx, satelliteID, 0, 0, -bytesDeleted)
}
return Error.Wrap(err)
}
_, _, err = store.blobs.EmptyTrash(ctx, satelliteID[:], trashedBefore)
Expand Down

2 comments on commit d68abcf

@storjrobot
Copy link

Choose a reason for hiding this comment

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

This commit has been mentioned on Storj Community Forum (official). There might be relevant details there:

https://forum.storj.io/t/release-preparation-v1-103/26031/10

@storjrobot
Copy link

Choose a reason for hiding this comment

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

This commit has been mentioned on Storj Community Forum (official). There might be relevant details there:

https://forum.storj.io/t/another-issue-with-trash/26025/20

Please sign in to comment.