SuperSeriousBusiness/GoToSocial
1
1
Fork 0
mirror of https://github.com/superseriousbusiness/gotosocial synced 2025-06-05 21:59:39 +02:00
Code Issues Projects Releases Wiki Activity

[bugfix] process account delete side effects in serial, not in parallel (#2360)

Browse Source 
* [bugfix] process account delete side effects in serial, not in parallel

* StartWorkers / StartNoopWorkers for tests

* undo testrig trace logging

* log errors instead of immediately returning
This commit is contained in:
tobi
2023-11-14 15:57:25 +01:00
committed by GitHub
parent 0b99f14d64
commit 4ee436e98a
41 changed files with 181 additions and 102 deletions
Download Patch File Download Diff File Expand all files Collapse all files

162
internal/processing/account/delete.go
View File

@ -27,6 +27,7 @@ import (
"github.com/google/uuid"
"github.com/superseriousbusiness/gotosocial/internal/ap"
"github.com/superseriousbusiness/gotosocial/internal/db"
"github.com/superseriousbusiness/gotosocial/internal/gtscontext"
"github.com/superseriousbusiness/gotosocial/internal/gtserror"
"github.com/superseriousbusiness/gotosocial/internal/gtsmodel"
"github.com/superseriousbusiness/gotosocial/internal/log"
@ -39,38 +40,45 @@ const deleteSelectLimit = 50
// Delete deletes an account, and all of that account's statuses, media, follows, notifications, etc etc etc.
// The origin passed here should be either the ID of the account doing the delete (can be itself), or the ID of a domain block.
func (p *Processor) Delete(ctx context.Context, account *gtsmodel.Account, origin string) gtserror.WithCode {
func (p *Processor) Delete(
ctx context.Context,
account *gtsmodel.Account,
origin string,
) gtserror.WithCode {
l := log.WithContext(ctx).WithFields(kv.Fields{
{"username", account.Username},
{"domain", account.Domain},
}...)
l.Trace("beginning account delete process")
// Delete statuses *before* follows to ensure correct addressing
// of any outgoing fedi messages generated by deleting statuses.
if err := p.deleteAccountStatuses(ctx, account); err != nil {
l.Errorf("continuing after error during account delete: %v", err)
}
if err := p.deleteAccountFollows(ctx, account); err != nil {
return gtserror.NewErrorInternalError(err)
l.Errorf("continuing after error during account delete: %v", err)
}
if err := p.deleteAccountBlocks(ctx, account); err != nil {
return gtserror.NewErrorInternalError(err)
}
if err := p.deleteAccountStatuses(ctx, account); err != nil {
return gtserror.NewErrorInternalError(err)
l.Errorf("continuing after error during account delete: %v", err)
}
if err := p.deleteAccountNotifications(ctx, account); err != nil {
return gtserror.NewErrorInternalError(err)
l.Errorf("continuing after error during account delete: %v", err)
}
if err := p.deleteAccountPeripheral(ctx, account); err != nil {
return gtserror.NewErrorInternalError(err)
l.Errorf("continuing after error during account delete: %v", err)
}
if account.IsLocal() {
// we tokens, applications and clients for account as one of the last
// stages during deletion, as other database models rely on these.
// We delete tokens, applications and clients for
// account as one of the last stages during deletion,
// as other database models rely on these.
if err := p.deleteUserAndTokensForAccount(ctx, account); err != nil {
return gtserror.NewErrorInternalError(err)
l.Errorf("continuing after error during account delete: %v", err)
}
}
@ -83,7 +91,7 @@ func (p *Processor) Delete(ctx context.Context, account *gtsmodel.Account, origi
return gtserror.NewErrorInternalError(err)
}
l.Info("account deleted")
l.Info("account delete process complete")
return nil
}
@ -189,7 +197,7 @@ func (p *Processor) deleteAccountFollows(ctx context.Context, account *gtsmodel.
// To avoid checking if account is local over + over
// inside the subsequent loops, just generate static
// side effects function once now.
unfollowSideEffects = p.unfollowSideEffectsFunc(account)
unfollowSideEffects = p.unfollowSideEffectsFunc(account.IsLocal())
)
// Delete follows originating from this account.
@ -240,31 +248,56 @@ func (p *Processor) deleteAccountFollows(ctx context.Context, account *gtsmodel.
}
}
// Process accreted messages asynchronously.
p.state.Workers.EnqueueClientAPI(ctx, msgs...)
// Process accreted messages in serial.
for _, msg := range msgs {
if err := p.state.Workers.ProcessFromClientAPI(ctx, msg); err != nil {
log.Errorf(
ctx,
"error processing %s of %s during Delete of account %s: %v",
msg.APActivityType, msg.APObjectType, account.ID, err,
)
}
}
return nil
}
func (p *Processor) unfollowSideEffectsFunc(deletedAccount *gtsmodel.Account) func(ctx context.Context, account *gtsmodel.Account, follow *gtsmodel.Follow) *messages.FromClientAPI {
if !deletedAccount.IsLocal() {
func (p *Processor) unfollowSideEffectsFunc(local bool) func(
ctx context.Context,
account *gtsmodel.Account,
follow *gtsmodel.Follow,
) *messages.FromClientAPI {
if !local {
// Don't try to process side effects
// for accounts that aren't local.
return func(ctx context.Context, account *gtsmodel.Account, follow *gtsmodel.Follow) *messages.FromClientAPI {
return nil // noop
return func(
_ context.Context,
_ *gtsmodel.Account,
_ *gtsmodel.Follow,
) *messages.FromClientAPI {
// noop
return nil
}
}
return func(ctx context.Context, account *gtsmodel.Account, follow *gtsmodel.Follow) *messages.FromClientAPI {
return func(
ctx context.Context,
account *gtsmodel.Account,
follow *gtsmodel.Follow,
) *messages.FromClientAPI {
if follow.TargetAccount == nil {
// TargetAccount seems to have gone;
// race condition? db corruption?
log.WithContext(ctx).WithField("follow", follow).Warn("follow had no TargetAccount, likely race condition")
log.
WithContext(ctx).
WithField("follow", follow).
Warn("follow had no TargetAccount, likely race condition")
return nil
}
if follow.TargetAccount.IsLocal() {
// No side effects for local unfollows.
// No side effects
// for local unfollows.
return nil
}
@ -288,8 +321,11 @@ func (p *Processor) deleteAccountBlocks(ctx context.Context, account *gtsmodel.A
// deleteAccountStatuses iterates through all statuses owned by
// the given account, passing each discovered status (and boosts
// thereof) to the processor workers for further async processing.
func (p *Processor) deleteAccountStatuses(ctx context.Context, account *gtsmodel.Account) error {
// thereof) to the processor workers for further processing.
func (p *Processor) deleteAccountStatuses(
ctx context.Context,
account *gtsmodel.Account,
) error {
// We'll select statuses 50 at a time so we don't wreck the db,
// and pass them through to the client api worker to handle.
//
@ -331,42 +367,43 @@ statusLoop:
maxID = statuses[len(statuses)-1].ID
for _, status := range statuses {
status.Account = account // ensure account is set
// Pass the status delete through the client api worker for processing.
msgs = append(msgs, messages.FromClientAPI{
APObjectType: ap.ObjectNote,
APActivityType: ap.ActivityDelete,
GTSModel: status,
OriginAccount: account,
TargetAccount: account,
})
// Ensure account is set.
status.Account = account
// Look for any boosts of this status in DB.
boosts, err := p.state.DB.GetStatusBoosts(ctx, status.ID)
//
// We put these in the msgs slice first so
// that they're handled first, before the
// parent status that's being boosted.
//
// Use a barebones context and just select the
// origin account separately. The rest will be
// populated later anyway, and we don't want to
// stop now because we couldn't get something.
boosts, err := p.state.DB.GetStatusBoosts(
gtscontext.SetBarebones(ctx),
status.ID,
)
if err != nil && !errors.Is(err, db.ErrNoEntries) {
return gtserror.Newf("error fetching status reblogs for %s: %w", status.ID, err)
return gtserror.Newf("error fetching status boosts for %s: %w", status.ID, err)
}
// Prepare to Undo each boost.
for _, boost := range boosts {
if boost.Account == nil {
// Fetch the relevant account for this status boost.
boostAcc, err := p.state.DB.GetAccountByID(ctx, boost.AccountID)
if err != nil {
if errors.Is(err, db.ErrNoEntries) {
// We don't have an account for this boost
// for some reason, so just skip processing.
log.WithContext(ctx).WithField("boost", boost).Warnf("no account found with id %s for boost %s", boost.AccountID, boost.ID)
continue
}
return gtserror.Newf("error fetching boosted status account for %s: %w", boost.AccountID, err)
}
boost.Account, err = p.state.DB.GetAccountByID(
gtscontext.SetBarebones(ctx),
boost.AccountID,
)
// Set account model
boost.Account = boostAcc
if err != nil {
log.Warnf(
ctx,
"db error getting owner %s of status boost %s: %v",
boost.AccountID, boost.ID, err,
)
continue
}
// Pass the boost delete through the client api worker for processing.
msgs = append(msgs, messages.FromClientAPI{
APObjectType: ap.ActivityAnnounce,
APActivityType: ap.ActivityUndo,
@ -375,11 +412,28 @@ statusLoop:
TargetAccount: account,
})
}
// Now prepare to Delete status.
msgs = append(msgs, messages.FromClientAPI{
APObjectType: ap.ObjectNote,
APActivityType: ap.ActivityDelete,
GTSModel: status,
OriginAccount: account,
TargetAccount: account,
})
}
}
// Batch process all accreted messages.
p.state.Workers.EnqueueClientAPI(ctx, msgs...)
// Process accreted messages in serial.
for _, msg := range msgs {
if err := p.state.Workers.ProcessFromClientAPI(ctx, msg); err != nil {
log.Errorf(
ctx,
"error processing %s of %s during Delete of account %s: %v",
msg.APActivityType, msg.APObjectType, account.ID, err,
)
}
}
return nil
}