[performance] Tweak media attachment cleanup; replace stale index (#2143)

internal/cleaner/media.go

@@ -323,8 +323,8 @@ func (m *Media) pruneUnused(ctx context.Context, media *gtsmodel.MediaAttachment
 	l := log.WithContext(ctx).
 		WithField("media", media.ID)
 
-	// Check whether we have the required account for media.
+	// Check whether we have the account that owns the media.
-	account, missing, err := m.getRelatedAccount(ctx, media)
+	account, missing, err := m.getOwningAccount(ctx, media)
 	if err != nil {
 		return false, err
 	} else if missing {
@@ -371,8 +371,8 @@ func (m *Media) fixCacheState(ctx context.Context, media *gtsmodel.MediaAttachme
 	l := log.WithContext(ctx).
 		WithField("media", media.ID)
 
-	// Check whether we have the required account for media.
+	// Check whether we have the account that owns the media.
-	_, missingAccount, err := m.getRelatedAccount(ctx, media)
+	_, missingAccount, err := m.getOwningAccount(ctx, media)
 	if err != nil {
 		return false, err
 	} else if missingAccount {
@@ -428,32 +428,42 @@ func (m *Media) uncacheRemote(ctx context.Context, after time.Time, media *gtsmo
 	l := log.WithContext(ctx).
 		WithField("media", media.ID)
 
-	// Check whether we have the required account for media.
+	// There are two possibilities here:
-	account, missing, err := m.getRelatedAccount(ctx, media)
+	//
-	if err != nil {
+	//   1. Media is an avatar or header; we should uncache
-		return false, err
+	//      it if we haven't seen the account recently.
-	} else if missing {
+	//   2. Media is attached to a status; we should uncache
-		l.Debug("skipping due to missing account")
+	//      it if we haven't seen the status recently.
-		return false, nil
+	if *media.Avatar || *media.Header {
-	}
+		// Check whether we have the account that owns the media.
+		account, missing, err := m.getOwningAccount(ctx, media)
+		if err != nil {
+			return false, err
+		} else if missing {
+			// PruneUnused will take care of this case.
+			l.Debug("skipping due to missing account")
+			return false, nil
+		}
 
-	if account != nil && account.FetchedAt.After(after) {
+		if account != nil && account.FetchedAt.After(after) {
-		l.Debug("skipping due to recently fetched account")
+			l.Debug("skipping due to recently fetched account")
-		return false, nil
+			return false, nil
-	}
+		}
+	} else {
+		// Check whether we have the status that media is attached to.
+		status, missing, err := m.getRelatedStatus(ctx, media)
+		if err != nil {
+			return false, err
+		} else if missing {
+			// PruneUnused will take care of this case.
+			l.Debug("skipping due to missing status")
+			return false, nil
+		}
 
-	// Check whether we have the required status for media.
+		if status != nil && status.FetchedAt.After(after) {
-	status, missing, err := m.getRelatedStatus(ctx, media)
+			l.Debug("skipping due to recently fetched status")
-	if err != nil {
+			return false, nil
-		return false, err
+		}
-	} else if missing {
-		l.Debug("skipping due to missing status")
-		return false, nil
-	}
-
-	if status != nil && status.FetchedAt.After(after) {
-		l.Debug("skipping due to recently fetched status")
-		return false, nil
 	}
 
 	// This media is too old, uncache it.
@@ -461,13 +471,13 @@ func (m *Media) uncacheRemote(ctx context.Context, after time.Time, media *gtsmo
 	return true, m.uncache(ctx, media)
 }
 
-func (m *Media) getRelatedAccount(ctx context.Context, media *gtsmodel.MediaAttachment) (*gtsmodel.Account, bool, error) {
+func (m *Media) getOwningAccount(ctx context.Context, media *gtsmodel.MediaAttachment) (*gtsmodel.Account, bool, error) {
 	if media.AccountID == "" {
 		// no related account.
 		return nil, false, nil
 	}
 
-	// Load the account related to this media.
+	// Load the account that owns this media.
 	account, err := m.state.DB.GetAccountByID(
 		gtscontext.SetBarebones(ctx),
 		media.AccountID,

internal/db/bundb/media.go

@@ -200,23 +200,6 @@ func (m *mediaDB) DeleteAttachment(ctx context.Context, id string) error {
 	return err
 }
 
-func (m *mediaDB) CountRemoteOlderThan(ctx context.Context, olderThan time.Time) (int, error) {
-	q := m.db.
-		NewSelect().
-		TableExpr("? AS ?", bun.Ident("media_attachments"), bun.Ident("media_attachment")).
-		Column("media_attachment.id").
-		Where("? = ?", bun.Ident("media_attachment.cached"), true).
-		Where("? IS NOT NULL", bun.Ident("media_attachment.remote_url")).
-		Where("? < ?", bun.Ident("media_attachment.created_at"), olderThan)
-
-	count, err := q.Count(ctx)
-	if err != nil {
-		return 0, err
-	}
-
-	return count, nil
-}
-
 func (m *mediaDB) GetAttachments(ctx context.Context, maxID string, limit int) ([]*gtsmodel.MediaAttachment, error) {
 	attachmentIDs := make([]string, 0, limit)
 
@@ -286,77 +269,3 @@ func (m *mediaDB) GetCachedAttachmentsOlderThan(ctx context.Context, olderThan t
 
 	return m.GetAttachmentsByIDs(ctx, attachmentIDs)
 }
-
-func (m *mediaDB) GetAvatarsAndHeaders(ctx context.Context, maxID string, limit int) ([]*gtsmodel.MediaAttachment, error) {
-	attachmentIDs := make([]string, 0, limit)
-
-	q := m.db.NewSelect().
-		TableExpr("? AS ?", bun.Ident("media_attachments"), bun.Ident("media_attachment")).
-		Column("media_attachment.id").
-		WhereGroup(" AND ", func(innerQ *bun.SelectQuery) *bun.SelectQuery {
-			return innerQ.
-				WhereOr("? = ?", bun.Ident("media_attachment.avatar"), true).
-				WhereOr("? = ?", bun.Ident("media_attachment.header"), true)
-		}).
-		Order("media_attachment.id DESC")
-
-	if maxID != "" {
-		q = q.Where("? < ?", bun.Ident("media_attachment.id"), maxID)
-	}
-
-	if limit != 0 {
-		q = q.Limit(limit)
-	}
-
-	if err := q.Scan(ctx, &attachmentIDs); err != nil {
-		return nil, err
-	}
-
-	return m.GetAttachmentsByIDs(ctx, attachmentIDs)
-}
-
-func (m *mediaDB) GetLocalUnattachedOlderThan(ctx context.Context, olderThan time.Time, limit int) ([]*gtsmodel.MediaAttachment, error) {
-	attachmentIDs := make([]string, 0, limit)
-
-	q := m.db.
-		NewSelect().
-		TableExpr("? AS ?", bun.Ident("media_attachments"), bun.Ident("media_attachment")).
-		Column("media_attachment.id").
-		Where("? = ?", bun.Ident("media_attachment.cached"), true).
-		Where("? = ?", bun.Ident("media_attachment.avatar"), false).
-		Where("? = ?", bun.Ident("media_attachment.header"), false).
-		Where("? < ?", bun.Ident("media_attachment.created_at"), olderThan).
-		Where("? IS NULL", bun.Ident("media_attachment.remote_url")).
-		Where("? IS NULL", bun.Ident("media_attachment.status_id")).
-		Order("media_attachment.created_at DESC")
-
-	if limit != 0 {
-		q = q.Limit(limit)
-	}
-
-	if err := q.Scan(ctx, &attachmentIDs); err != nil {
-		return nil, err
-	}
-
-	return m.GetAttachmentsByIDs(ctx, attachmentIDs)
-}
-
-func (m *mediaDB) CountLocalUnattachedOlderThan(ctx context.Context, olderThan time.Time) (int, error) {
-	q := m.db.
-		NewSelect().
-		TableExpr("? AS ?", bun.Ident("media_attachments"), bun.Ident("media_attachment")).
-		Column("media_attachment.id").
-		Where("? = ?", bun.Ident("media_attachment.cached"), true).
-		Where("? = ?", bun.Ident("media_attachment.avatar"), false).
-		Where("? = ?", bun.Ident("media_attachment.header"), false).
-		Where("? < ?", bun.Ident("media_attachment.created_at"), olderThan).
-		Where("? IS NULL", bun.Ident("media_attachment.remote_url")).
-		Where("? IS NULL", bun.Ident("media_attachment.status_id"))
-
-	count, err := q.Count(ctx)
-	if err != nil {
-		return 0, err
-	}
-
-	return count, nil
-}

internal/db/bundb/media_test.go

@@ -23,7 +23,6 @@ import (
 	"time"
 
 	"github.com/stretchr/testify/suite"
-	"github.com/superseriousbusiness/gotosocial/testrig"
 )
 
 type MediaTestSuite struct {
@@ -43,20 +42,12 @@ func (suite *MediaTestSuite) TestGetOlder() {
 	suite.Len(attachments, 2)
 }
 
-func (suite *MediaTestSuite) TestGetAvisAndHeaders() {
+func (suite *MediaTestSuite) TestGetCachedAttachmentsOlderThan() {
 	ctx := context.Background()
 
-	attachments, err := suite.db.GetAvatarsAndHeaders(ctx, "", 20)
+	attachments, err := suite.db.GetCachedAttachmentsOlderThan(ctx, time.Now(), 20)
 	suite.NoError(err)
-	suite.Len(attachments, 3)
+	suite.Len(attachments, 2)
-}
-
-func (suite *MediaTestSuite) TestGetLocalUnattachedOlderThan() {
-	ctx := context.Background()
-
-	attachments, err := suite.db.GetLocalUnattachedOlderThan(ctx, testrig.TimeMustParse("2090-06-04T13:12:00Z"), 10)
-	suite.NoError(err)
-	suite.Len(attachments, 1)
 }
 
 func TestMediaTestSuite(t *testing.T) {

internal/db/bundb/migrations/20230821075342_attachment_cleanup_updates.go

@@ -0,0 +1,61 @@
+// GoToSocial
+// Copyright (C) GoToSocial Authors admin@gotosocial.org
+// SPDX-License-Identifier: AGPL-3.0-or-later
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Affero General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU Affero General Public License for more details.
+//
+// You should have received a copy of the GNU Affero General Public License
+// along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+package migrations
+
+import (
+	"context"
+
+	"github.com/superseriousbusiness/gotosocial/internal/log"
+	"github.com/uptrace/bun"
+)
+
+func init() {
+	up := func(ctx context.Context, db *bun.DB) error {
+		return db.RunInTx(ctx, nil, func(ctx context.Context, tx bun.Tx) error {
+			log.Info(ctx, "dropping previous media attachment cleanup index, please wait and don't interrupt it (this may take a while)")
+			if _, err := tx.
+				NewDropIndex().
+				Index("media_attachments_cleanup_idx").
+				Exec(ctx); err != nil {
+				return err
+			}
+
+			log.Info(ctx, "creating new media attachment cleanup index, please wait and don't interrupt it (this may take a while)")
+			if _, err := tx.
+				NewCreateIndex().
+				Table("media_attachments").
+				Index("media_attachments_cleanup_idx").
+				Column("cached", "created_at").
+				Exec(ctx); err != nil {
+				return err
+			}
+
+			return nil
+		})
+	}
+
+	down := func(ctx context.Context, db *bun.DB) error {
+		return db.RunInTx(ctx, nil, func(ctx context.Context, tx bun.Tx) error {
+			return nil
+		})
+	}
+
+	if err := Migrations.Register(up, down); err != nil {
+		panic(err)
+	}
+}

internal/db/media.go

@@ -50,24 +50,4 @@ type Media interface {
 	// GetCachedAttachmentsOlderThan gets limit n remote attachments (including avatars and headers) older than
 	// the given time. These will be returned in order of attachment.created_at descending (i.e. newest to oldest).
 	GetCachedAttachmentsOlderThan(ctx context.Context, olderThan time.Time, limit int) ([]*gtsmodel.MediaAttachment, error)
-
-	// CountRemoteOlderThan is like GetRemoteOlderThan, except instead of getting limit n attachments,
-	// it just counts how many remote attachments in the database (including avatars and headers) meet
-	// the olderThan criteria.
-	CountRemoteOlderThan(ctx context.Context, olderThan time.Time) (int, error)
-
-	// GetAvatarsAndHeaders fetches limit n avatars and headers with an id < maxID. These headers
-	// and avis may be in use or not; the caller should check this if it's important.
-	GetAvatarsAndHeaders(ctx context.Context, maxID string, limit int) ([]*gtsmodel.MediaAttachment, error)
-
-	// GetLocalUnattachedOlderThan fetches limit n local media attachments (including avatars and headers), older than
-	// the given time, which aren't header or avatars, and aren't attached to a status. In other words, attachments which were
-	// uploaded but never used for whatever reason, or attachments that were attached to a status which was subsequently deleted.
-	//
-	// These will be returned in order of attachment.created_at descending (newest to oldest in other words).
-	GetLocalUnattachedOlderThan(ctx context.Context, olderThan time.Time, limit int) ([]*gtsmodel.MediaAttachment, error)
-
-	// CountLocalUnattachedOlderThan is like GetLocalUnattachedOlderThan, except instead of getting limit n attachments,
-	// it just counts how many local attachments in the database meet the olderThan criteria.
-	CountLocalUnattachedOlderThan(ctx context.Context, olderThan time.Time) (int, error)
 }