From d68c04a6c0964d795a8d475c2f73d201bc72e68b Mon Sep 17 00:00:00 2001 From: kim <89579420+NyaaaWhatsUpDoc@users.noreply.github.com> Date: Fri, 2 Sep 2022 11:17:46 +0100 Subject: [PATCH] [performance] cache recently allowed/denied domains to cut down on db calls (#794) * fetch creation and fetching domain blocks from db Signed-off-by: kim * add separate domainblock cache type, handle removing block from cache on delete Signed-off-by: kim * fix sentinel nil values being passed into cache Signed-off-by: kim Signed-off-by: kim --- internal/cache/domain.go | 106 ++++++++++++++++ internal/db/bundb/bundb.go | 6 +- internal/db/bundb/domain.go | 115 +++++++++++++++--- internal/db/bundb/domain_test.go | 9 +- internal/db/domain.go | 11 ++ .../processing/admin/createdomainblock.go | 35 +++--- .../processing/admin/deletedomainblock.go | 4 +- 7 files changed, 245 insertions(+), 41 deletions(-) create mode 100644 internal/cache/domain.go diff --git a/internal/cache/domain.go b/internal/cache/domain.go new file mode 100644 index 000000000..7b5a93d39 --- /dev/null +++ b/internal/cache/domain.go @@ -0,0 +1,106 @@ +/* + GoToSocial + Copyright (C) 2021-2022 GoToSocial Authors admin@gotosocial.org + + 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 . +*/ + +package cache + +import ( + "time" + + "codeberg.org/gruf/go-cache/v2" + "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" +) + +// DomainCache is a cache wrapper to provide URL and URI lookups for gtsmodel.Status +type DomainBlockCache struct { + cache cache.LookupCache[string, string, *gtsmodel.DomainBlock] +} + +// NewStatusCache returns a new instantiated statusCache object +func NewDomainBlockCache() *DomainBlockCache { + c := &DomainBlockCache{} + c.cache = cache.NewLookup(cache.LookupCfg[string, string, *gtsmodel.DomainBlock]{ + RegisterLookups: func(lm *cache.LookupMap[string, string]) { + lm.RegisterLookup("id") + }, + + AddLookups: func(lm *cache.LookupMap[string, string], block *gtsmodel.DomainBlock) { + // Block can be equal to nil when sentinel + if block != nil && block.ID != "" { + lm.Set("id", block.ID, block.Domain) + } + }, + + DeleteLookups: func(lm *cache.LookupMap[string, string], block *gtsmodel.DomainBlock) { + // Block can be equal to nil when sentinel + if block != nil && block.ID != "" { + lm.Delete("id", block.ID) + } + }, + }) + c.cache.SetTTL(time.Minute*5, false) + c.cache.Start(time.Second * 10) + return c +} + +// GetByID attempts to fetch a status from the cache by its ID, you will receive a copy for thread-safety +func (c *DomainBlockCache) GetByID(id string) (*gtsmodel.DomainBlock, bool) { + return c.cache.GetBy("id", id) +} + +// GetByURL attempts to fetch a status from the cache by its URL, you will receive a copy for thread-safety +func (c *DomainBlockCache) GetByDomain(domain string) (*gtsmodel.DomainBlock, bool) { + return c.cache.Get(domain) +} + +// Put places a status in the cache, ensuring that the object place is a copy for thread-safety +func (c *DomainBlockCache) Put(domain string, block *gtsmodel.DomainBlock) { + if domain == "" { + panic("invalid domain") + } + + if block == nil { + // This is a sentinel value for (no block) + c.cache.Set(domain, nil) + } else { + // This is a valid domain block + c.cache.Set(domain, copyDomainBlock(block)) + } +} + +// InvalidateByDomain will invalidate a domain block from the cache by domain name. +func (c *DomainBlockCache) InvalidateByDomain(domain string) { + c.cache.Invalidate(domain) +} + +// copyStatus performs a surface-level copy of status, only keeping attached IDs intact, not the objects. +// due to all the data being copied being 99% primitive types or strings (which are immutable and passed by ptr) +// this should be a relatively cheap process +func copyDomainBlock(block *gtsmodel.DomainBlock) *gtsmodel.DomainBlock { + return >smodel.DomainBlock{ + ID: block.ID, + CreatedAt: block.CreatedAt, + UpdatedAt: block.UpdatedAt, + Domain: block.Domain, + CreatedByAccountID: block.CreatedByAccountID, + CreatedByAccount: nil, + PrivateComment: block.PrivateComment, + PublicComment: block.PublicComment, + Obfuscate: block.Obfuscate, + SubscriptionID: block.SubscriptionID, + } +} diff --git a/internal/db/bundb/bundb.go b/internal/db/bundb/bundb.go index ca123543a..6bf3571b4 100644 --- a/internal/db/bundb/bundb.go +++ b/internal/db/bundb/bundb.go @@ -173,6 +173,9 @@ func NewBunDBService(ctx context.Context) (db.DB, error) { notifCache.SetTTL(time.Minute*5, false) notifCache.Start(time.Second * 10) + // Prepare domain block cache + blockCache := cache.NewDomainBlockCache() + ps := &bunDBService{ Account: accounts, Admin: &adminDB{ @@ -182,7 +185,8 @@ func NewBunDBService(ctx context.Context) (db.DB, error) { conn: conn, }, Domain: &domainDB{ - conn: conn, + conn: conn, + cache: blockCache, }, Emoji: &emojiDB{ conn: conn, diff --git a/internal/db/bundb/domain.go b/internal/db/bundb/domain.go index fadb6dcf9..4cad75e4d 100644 --- a/internal/db/bundb/domain.go +++ b/internal/db/bundb/domain.go @@ -20,59 +20,134 @@ package bundb import ( "context" + "database/sql" "net/url" "strings" + "github.com/superseriousbusiness/gotosocial/internal/cache" "github.com/superseriousbusiness/gotosocial/internal/config" "github.com/superseriousbusiness/gotosocial/internal/db" "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" - "github.com/superseriousbusiness/gotosocial/internal/util" ) type domainDB struct { - conn *DBConn + conn *DBConn + cache *cache.DomainBlockCache } -func (d *domainDB) IsDomainBlocked(ctx context.Context, domain string) (bool, db.Error) { - if domain == "" || domain == config.GetHost() { - return false, nil +func (d *domainDB) CreateDomainBlock(ctx context.Context, block gtsmodel.DomainBlock) db.Error { + // Normalize to lowercase + block.Domain = strings.ToLower(block.Domain) + + // Attempt to insert new domain block + _, err := d.conn.NewInsert(). + Model(&block). + Exec(ctx, &block) + if err != nil { + return d.conn.ProcessError(err) } + // Cache this domain block + d.cache.Put(block.Domain, &block) + + return nil +} + +func (d *domainDB) GetDomainBlock(ctx context.Context, domain string) (*gtsmodel.DomainBlock, db.Error) { + // Normalize to lowercase + domain = strings.ToLower(domain) + + // Check for easy case, domain referencing *us* + if domain == "" || domain == config.GetAccountDomain() { + return nil, db.ErrNoEntries + } + + // Check for already cached rblock + if block, ok := d.cache.GetByDomain(domain); ok { + // A 'nil' return value is a sentinel value for no block + if block == nil { + return nil, db.ErrNoEntries + } + + // Else, this block exists + return block, nil + } + + block := >smodel.DomainBlock{} + q := d.conn. NewSelect(). - Model(>smodel.DomainBlock{}). - ExcludeColumn("id", "created_at", "updated_at", "created_by_account_id", "private_comment", "public_comment", "obfuscate", "subscription_id"). + Model(block). Where("domain = ?", domain). Limit(1) - return d.conn.Exists(ctx, q) + // Query database for domain block + switch err := q.Scan(ctx); err { + // No error, block found + case nil: + d.cache.Put(domain, block) + return block, nil + + // No error, simply not found + case sql.ErrNoRows: + d.cache.Put(domain, nil) + return nil, db.ErrNoEntries + + // Any other db error + default: + return nil, d.conn.ProcessError(err) + } +} + +func (d *domainDB) DeleteDomainBlock(ctx context.Context, domain string) db.Error { + // Normalize to lowercase + domain = strings.ToLower(domain) + + // Attempt to delete domain block + _, err := d.conn.NewDelete(). + Model((*gtsmodel.DomainBlock)(nil)). + Where("domain = ?", domain). + Exec(ctx, nil) + if err != nil { + return d.conn.ProcessError(err) + } + + // Clear domain from cache + d.cache.InvalidateByDomain(domain) + + return nil +} + +func (d *domainDB) IsDomainBlocked(ctx context.Context, domain string) (bool, db.Error) { + block, err := d.GetDomainBlock(ctx, domain) + if err == nil || err == db.ErrNoEntries { + return (block != nil), nil + } + return false, err } func (d *domainDB) AreDomainsBlocked(ctx context.Context, domains []string) (bool, db.Error) { - // filter out any doubles - uniqueDomains := util.UniqueStrings(domains) - - for _, domain := range uniqueDomains { - if blocked, err := d.IsDomainBlocked(ctx, strings.ToLower(domain)); err != nil { + for _, domain := range domains { + if blocked, err := d.IsDomainBlocked(ctx, domain); err != nil { return false, err } else if blocked { return blocked, nil } } - - // no blocks found return false, nil } func (d *domainDB) IsURIBlocked(ctx context.Context, uri *url.URL) (bool, db.Error) { - domain := uri.Hostname() - return d.IsDomainBlocked(ctx, domain) + return d.IsDomainBlocked(ctx, uri.Hostname()) } func (d *domainDB) AreURIsBlocked(ctx context.Context, uris []*url.URL) (bool, db.Error) { - domains := []string{} for _, uri := range uris { - domains = append(domains, uri.Hostname()) + if blocked, err := d.IsDomainBlocked(ctx, uri.Hostname()); err != nil { + return false, err + } else if blocked { + return blocked, nil + } } - return d.AreDomainsBlocked(ctx, domains) + return false, nil } diff --git a/internal/db/bundb/domain_test.go b/internal/db/bundb/domain_test.go index 1a3fed24d..b326236ad 100644 --- a/internal/db/bundb/domain_test.go +++ b/internal/db/bundb/domain_test.go @@ -21,6 +21,7 @@ package bundb_test import ( "context" "testing" + "time" "github.com/stretchr/testify/suite" "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" @@ -33,10 +34,15 @@ type DomainTestSuite struct { func (suite *DomainTestSuite) TestIsDomainBlocked() { ctx := context.Background() + now := time.Now() + domainBlock := >smodel.DomainBlock{ ID: "01G204214Y9TNJEBX39C7G88SW", Domain: "some.bad.apples", + CreatedAt: now, + UpdatedAt: now, CreatedByAccountID: suite.testAccounts["admin_account"].ID, + CreatedByAccount: suite.testAccounts["admin_account"], } // no domain block exists for the given domain yet @@ -44,7 +50,8 @@ func (suite *DomainTestSuite) TestIsDomainBlocked() { suite.NoError(err) suite.False(blocked) - suite.db.Put(ctx, domainBlock) + err = suite.db.CreateDomainBlock(ctx, *domainBlock) + suite.NoError(err) // domain block now exists blocked, err = suite.db.IsDomainBlocked(ctx, domainBlock.Domain) diff --git a/internal/db/domain.go b/internal/db/domain.go index b92705483..7b264165d 100644 --- a/internal/db/domain.go +++ b/internal/db/domain.go @@ -21,10 +21,21 @@ package db import ( "context" "net/url" + + "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" ) // Domain contains DB functions related to domains and domain blocks. type Domain interface { + // CreateDomainBlock ... + CreateDomainBlock(ctx context.Context, block gtsmodel.DomainBlock) Error + + // GetDomainBlock ... + GetDomainBlock(ctx context.Context, domain string) (*gtsmodel.DomainBlock, Error) + + // DeleteDomainBlock ... + DeleteDomainBlock(ctx context.Context, domain string) Error + // IsDomainBlocked checks if an instance-level domain block exists for the given domain string (eg., `example.org`). IsDomainBlocked(ctx context.Context, domain string) (bool, Error) diff --git a/internal/processing/admin/createdomainblock.go b/internal/processing/admin/createdomainblock.go index 969cb52de..fcc0cb480 100644 --- a/internal/processing/admin/createdomainblock.go +++ b/internal/processing/admin/createdomainblock.go @@ -20,6 +20,7 @@ package admin import ( "context" + "errors" "fmt" "strings" "time" @@ -41,22 +42,21 @@ func (p *processor) DomainBlockCreate(ctx context.Context, account *gtsmodel.Acc domain = strings.ToLower(domain) // first check if we already have a block -- if err == nil we already had a block so we can skip a whole lot of work - domainBlock := >smodel.DomainBlock{} - err := p.db.GetWhere(ctx, []db.Where{{Key: "domain", Value: domain}}, domainBlock) + block, err := p.db.GetDomainBlock(ctx, domain) if err != nil { - if err != db.ErrNoEntries { + if !errors.Is(err, db.ErrNoEntries) { // something went wrong in the DB - return nil, gtserror.NewErrorInternalError(fmt.Errorf("DomainBlockCreate: db error checking for existence of domain block %s: %s", domain, err)) + return nil, gtserror.NewErrorInternalError(fmt.Errorf("db error checking for existence of domain block %s: %s", domain, err)) } // there's no block for this domain yet so create one // note: we take a new ulid from timestamp here in case we need to sort blocks blockID, err := id.NewULID() if err != nil { - return nil, gtserror.NewErrorInternalError(fmt.Errorf("DomainBlockCreate: error creating id for new domain block %s: %s", domain, err)) + return nil, gtserror.NewErrorInternalError(fmt.Errorf("error creating id for new domain block %s: %s", domain, err)) } - domainBlock = >smodel.DomainBlock{ + newBlock := gtsmodel.DomainBlock{ ID: blockID, Domain: domain, CreatedByAccountID: account.ID, @@ -66,20 +66,22 @@ func (p *processor) DomainBlockCreate(ctx context.Context, account *gtsmodel.Acc SubscriptionID: subscriptionID, } - // put the new block in the database - if err := p.db.Put(ctx, domainBlock); err != nil { - if err != db.ErrNoEntries { - // there's a real error creating the block - return nil, gtserror.NewErrorInternalError(fmt.Errorf("DomainBlockCreate: db error putting new domain block %s: %s", domain, err)) - } + // Insert the new block into the database + if err := p.db.CreateDomainBlock(ctx, newBlock); err != nil { + return nil, gtserror.NewErrorInternalError(fmt.Errorf("db error putting new domain block %s: %s", domain, err)) } - // process the side effects of the domain block asynchronously since it might take a while - go p.initiateDomainBlockSideEffects(context.Background(), account, domainBlock) // TODO: add this to a queuing system so it can retry/resume + + // Set the newly created block + block = &newBlock + + // Process the side effects of the domain block asynchronously since it might take a while + go p.initiateDomainBlockSideEffects(ctx, account, block) } - apiDomainBlock, err := p.tc.DomainBlockToAPIDomainBlock(ctx, domainBlock, false) + // Convert our gts model domain block into an API model + apiDomainBlock, err := p.tc.DomainBlockToAPIDomainBlock(ctx, block, false) if err != nil { - return nil, gtserror.NewErrorInternalError(fmt.Errorf("DomainBlockCreate: error converting domain block to frontend/api representation %s: %s", domain, err)) + return nil, gtserror.NewErrorInternalError(fmt.Errorf("error converting domain block to frontend/api representation %s: %s", domain, err)) } return apiDomainBlock, nil @@ -92,7 +94,6 @@ func (p *processor) DomainBlockCreate(ctx context.Context, account *gtsmodel.Acc // 3. Select all accounts from this instance and pass them through the delete functionality of the processor. func (p *processor) initiateDomainBlockSideEffects(ctx context.Context, account *gtsmodel.Account, block *gtsmodel.DomainBlock) { l := log.WithFields(kv.Fields{ - {"domain", block.Domain}, }...) diff --git a/internal/processing/admin/deletedomainblock.go b/internal/processing/admin/deletedomainblock.go index 29e911888..b65954fe5 100644 --- a/internal/processing/admin/deletedomainblock.go +++ b/internal/processing/admin/deletedomainblock.go @@ -47,8 +47,8 @@ func (p *processor) DomainBlockDelete(ctx context.Context, account *gtsmodel.Acc return nil, gtserror.NewErrorInternalError(err) } - // delete the domain block - if err := p.db.DeleteByID(ctx, id, domainBlock); err != nil { + // Delete the domain block + if err := p.db.DeleteDomainBlock(ctx, domainBlock.Domain); err != nil { return nil, gtserror.NewErrorInternalError(err) }