From aa01437a5b92646936aa32797ef5c216161bd17f Mon Sep 17 00:00:00 2001 From: kim Date: Tue, 2 Apr 2024 10:47:31 +0100 Subject: [PATCH 01/35] add delivery worker type that pulls from queue to httpclient package --- internal/cache/cache.go | 6 + internal/httpclient/client.go | 201 ++++++++++++------------ internal/httpclient/delivery.go | 262 ++++++++++++++++++++++++++++++++ internal/httpclient/request.go | 72 +++++++++ internal/httpclient/util.go | 9 ++ internal/httpclient/validate.go | 7 +- internal/queue/messages.go | 68 +++++++++ internal/queue/queues.go | 46 ++++++ internal/queue/wrappers.go | 115 ++++++++++++++ internal/state/state.go | 4 + internal/workers/workers.go | 13 +- 11 files changed, 693 insertions(+), 110 deletions(-) create mode 100644 internal/httpclient/delivery.go create mode 100644 internal/httpclient/request.go create mode 100644 internal/httpclient/util.go create mode 100644 internal/queue/messages.go create mode 100644 internal/queue/queues.go create mode 100644 internal/queue/wrappers.go diff --git a/internal/cache/cache.go b/internal/cache/cache.go index fd715b8e6e..dc2be55569 100644 --- a/internal/cache/cache.go +++ b/internal/cache/cache.go @@ -20,11 +20,17 @@ package cache import ( "time" + "codeberg.org/gruf/go-cache/v3/ttl" "github.com/superseriousbusiness/gotosocial/internal/cache/headerfilter" "github.com/superseriousbusiness/gotosocial/internal/log" ) type Caches struct { + + // BadHosts provides access to the HTTP + // client bad (i.e. erroring) hosts cache. + BadHosts ttl.Cache[string, struct{}] + // GTS provides access to the collection of // gtsmodel object caches. (used by the database). GTS GTSCaches diff --git a/internal/httpclient/client.go b/internal/httpclient/client.go index 6c24273728..5682f2a5e6 100644 --- a/internal/httpclient/client.go +++ b/internal/httpclient/client.go @@ -32,13 +32,12 @@ import ( "time" "codeberg.org/gruf/go-bytesize" - "codeberg.org/gruf/go-cache/v3" errorsv2 "codeberg.org/gruf/go-errors/v2" "codeberg.org/gruf/go-iotools" - "codeberg.org/gruf/go-kv" "github.com/superseriousbusiness/gotosocial/internal/gtscontext" "github.com/superseriousbusiness/gotosocial/internal/gtserror" "github.com/superseriousbusiness/gotosocial/internal/log" + "github.com/superseriousbusiness/gotosocial/internal/state" ) var ( @@ -106,9 +105,9 @@ type Config struct { // - optional request signing // - request logging type Client struct { - client http.Client - badHosts cache.TTLCache[string, struct{}] - bodyMax int64 + state *state.State + client http.Client + bodyMax int64 } // New returns a new instance of Client initialized using configuration. @@ -176,32 +175,11 @@ func New(cfg Config) *Client { DisableCompression: cfg.DisableCompression, }} - // Initiate outgoing bad hosts lookup cache. - c.badHosts = cache.NewTTL[string, struct{}](0, 1000, 0) - c.badHosts.SetTTL(time.Hour, false) - if !c.badHosts.Start(time.Minute) { - log.Panic(nil, "failed to start transport controller cache") - } - return &c } // Do will essentially perform http.Client{}.Do() with retry-backoff functionality. -func (c *Client) Do(r *http.Request) (*http.Response, error) { - return c.DoSigned(r, func(r *http.Request) error { - return nil // no request signing - }) -} - -// DoSigned will essentially perform http.Client{}.Do() with retry-backoff functionality and requesting signing.. -func (c *Client) DoSigned(r *http.Request, sign SignFunc) (rsp *http.Response, err error) { - const ( - // max no. attempts. - maxRetries = 5 - - // starting backoff duration. - baseBackoff = 2 * time.Second - ) +func (c *Client) Do(r *http.Request) (rsp *http.Response, err error) { // First validate incoming request. if err := ValidateRequest(r); err != nil { @@ -219,122 +197,132 @@ func (c *Client) DoSigned(r *http.Request, sign SignFunc) (rsp *http.Response, e // errors that are retried upon are server failure, TLS // and domain resolution type errors, so this cached result // indicates this server is likely having issues. - fastFail = c.badHosts.Has(host) + fastFail = c.state.Caches.BadHosts.Has(host) defer func() { if err != nil { - // On error return mark as bad-host. - c.badHosts.Set(host, struct{}{}) + // On error return ensure marked as bad-host. + c.state.Caches.BadHosts.Set(host, struct{}{}) } }() } - // Start a log entry for this request - l := log.WithContext(r.Context()). - WithFields(kv.Fields{ - {"method", r.Method}, - {"url", r.URL.String()}, - }...) + // Prepare log entry. + log := requestLog(r) - for i := 0; i < maxRetries; i++ { - var backoff time.Duration + // Wrap in our own request + // type for retry-backoff. + req := wrapRequest(r) - l.Info("performing request") + for req.attempts < maxRetries { + var retry bool - // Perform the request. - rsp, err = c.do(r) - if err == nil { //nolint:gocritic + log.Info("performing request") - // TooManyRequest means we need to slow - // down and retry our request. Codes over - // 500 generally indicate temp. outages. - if code := rsp.StatusCode; code < 500 && - code != http.StatusTooManyRequests { - return rsp, nil - } + // Perform the http request. + rsp, retry, err = c.do(&req) + if err == nil || !retry { + return + } - // Create loggable error from response status code. - err = fmt.Errorf(`http response: %s`, rsp.Status) + log.Error(err) - // Search for a provided "Retry-After" header value. - if after := rsp.Header.Get("Retry-After"); after != "" { + if fastFail { + // on fast-fail, don't bother backoff/retry + return nil, fmt.Errorf("%w (fast fail)", err) + } - // Get current time. - now := time.Now() + // Start the backoff timer channel. + backoff, cncl := sleepch(req.BackOff()) - if u, _ := strconv.ParseUint(after, 10, 32); u != 0 { - // An integer number of backoff seconds was provided. - backoff = time.Duration(u) * time.Second - } else if at, _ := http.ParseTime(after); !at.Before(now) { - // An HTTP formatted future date-time was provided. - backoff = at.Sub(now) - } + select { + // Request ctx cancelled + case <-r.Context().Done(): + cncl() - // Don't let their provided backoff exceed our max. - if max := baseBackoff * maxRetries; backoff > max { - backoff = max - } - } + // Backoff for a time + case <-backoff: + } + } - // Close + unset rsp. - _ = rsp.Body.Close() - rsp = nil + // Set error return to trigger setting "bad host". + err = errors.New("transport reached max retries") + return +} + +// do wraps an underlying http.Client{}.Do() to perform our wrapped request type: +// rewinding response body to permit reuse, signing request data when SignFunc provided, +// safely limiting response body, updating retry attempt counts and setting retry-after. +func (c *Client) do(r *request) (*http.Response, bool /* retry */, error) { + // Update the + // attempts. + r.attempts++ + + // Reset backoff. + r.backoff = 0 + + // Perform the HTTP request. + rsp, err := c.client.Do(r.req) + if err != nil { - } else if errorsv2.IsV2(err, + if errorsv2.IsV2(err, context.DeadlineExceeded, context.Canceled, ErrBodyTooLarge, ErrReservedAddr, ) { // Non-retryable errors. - return nil, err - } else if errstr := err.Error(); // nocollapse + return nil, false, err + } + + if errstr := err.Error(); // nocollapse strings.Contains(errstr, "stopped after 10 redirects") || strings.Contains(errstr, "tls: ") || strings.Contains(errstr, "x509: ") { // These error types aren't wrapped // so we have to check the error string. // All are unrecoverable! - return nil, err - } else if dnserr := (*net.DNSError)(nil); // nocollapse + return nil, false, err + } + + if dnserr := (*net.DNSError)(nil); // nocollapse errors.As(err, &dnserr) && dnserr.IsNotFound { // DNS lookup failure, this domain does not exist - return nil, gtserror.SetNotFound(err) + return nil, false, gtserror.SetNotFound(err) } - if fastFail { - // on fast-fail, don't bother backoff/retry - return nil, fmt.Errorf("%w (fast fail)", err) - } + return nil, true, err - if backoff == 0 { - // No retry-after found, set our predefined - // backoff according to a multiplier of 2^n. - backoff = baseBackoff * 1 << (i + 1) - } + } else if rsp.StatusCode > 500 || + rsp.StatusCode == http.StatusTooManyRequests { - l.Errorf("backing off for %s after http request error: %v", backoff, err) + // Codes over 500 (and 429: too many requests) + // are generally temporary errors. For these + // we replace the response with a loggable error. + err = fmt.Errorf(`http response: %s`, rsp.Status) - select { - // Request ctx cancelled - case <-r.Context().Done(): - return nil, r.Context().Err() + // Search for a provided "Retry-After" header value. + if after := rsp.Header.Get("Retry-After"); after != "" { - // Backoff for some time - case <-time.After(backoff): - } - } + // Get cur time. + now := time.Now() - // Set error return to trigger setting "bad host". - err = errors.New("transport reached max retries") - return -} + if u, _ := strconv.ParseUint(after, 10, 32); u != 0 { + // An integer no. of backoff seconds was provided. + r.backoff = time.Duration(u) * time.Second + } else if at, _ := http.ParseTime(after); !at.Before(now) { + // An HTTP formatted future date-time was provided. + r.backoff = at.Sub(time.Now()) + } -// do wraps http.Client{}.Do() to provide safely limited response bodies. -func (c *Client) do(req *http.Request) (*http.Response, error) { - // Perform the HTTP request. - rsp, err := c.client.Do(req) - if err != nil { - return nil, err + // Don't let their provided backoff exceed our max. + if max := baseBackoff * maxRetries; r.backoff > max { + r.backoff = max + } + } + + // Unset + close rsp. + _ = rsp.Body.Close() + return nil, true, err } // Seperate the body implementers. @@ -364,11 +352,10 @@ func (c *Client) do(req *http.Request) (*http.Response, error) { // Check response body not too large. if rsp.ContentLength > c.bodyMax { - _ = rsp.Body.Close() - return nil, ErrBodyTooLarge + return nil, false, ErrBodyTooLarge } - return rsp, nil + return rsp, true, nil } // cast discard writer to full interface it supports. diff --git a/internal/httpclient/delivery.go b/internal/httpclient/delivery.go new file mode 100644 index 0000000000..bd1776d142 --- /dev/null +++ b/internal/httpclient/delivery.go @@ -0,0 +1,262 @@ +// 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 . + +package httpclient + +import ( + "context" + "slices" + "time" + + "codeberg.org/gruf/go-runners" + "github.com/superseriousbusiness/gotosocial/internal/log" + "github.com/superseriousbusiness/gotosocial/internal/queue" +) + +type DeliveryWorkerPool struct { + client *Client + queue *queue.StructQueue[queue.HTTPRequest] + workers []DeliveryWorker +} + +// Init ... +func (p *DeliveryWorkerPool) Init( + client *Client, + queue *queue.StructQueue[queue.HTTPRequest], + workers int, +) { + p.client = client + p.queue = queue + p.workers = make([]DeliveryWorker, workers) + for i := range p.workers { + p.workers[i] = NewDeliveryWorker( + p.client, + p.queue, + ) + } +} + +// Start ... +func (p *DeliveryWorkerPool) Start() bool { + if len(p.workers) == 0 { + return false + } + ok := true + for i := range p.workers { + ok = p.workers[i].Start() && ok + } + return ok +} + +// Stop ... +func (p *DeliveryWorkerPool) Stop() bool { + if len(p.workers) == 0 { + return false + } + ok := true + for i := range p.workers { + ok = p.workers[i].Stop() && ok + } + return ok +} + +type DeliveryWorker struct { + client *Client + queue *queue.StructQueue[queue.HTTPRequest] + backlog []*delivery + service runners.Service +} + +// NewDeliveryWorker returns a new DeliveryWorker that feeds from queue, using given HTTP client. +func NewDeliveryWorker(client *Client, queue *queue.StructQueue[queue.HTTPRequest]) DeliveryWorker { + return DeliveryWorker{ + client: client, + queue: queue, + backlog: make([]*delivery, 0, 256), + } +} + +// Start ... +func (w *DeliveryWorker) Start() bool { + return w.service.Run(w.process) +} + +// Stop ... +func (w *DeliveryWorker) Stop() bool { + return w.service.Stop() +} + +// process is the main delivery worker processing routine. +func (w *DeliveryWorker) process(ctx context.Context) { + if w.client == nil || w.queue == nil { + panic("nil delivery worker fields") + } + +loop: + for { + // Get next delivery. + dlv, ok := w.next(ctx) + if !ok { + return + } + + // Check whether backoff required. + if d := dlv.BackOff(); d != 0 { + + // Start backoff sleep timer. + backoff, cncl := sleepch(d) + + select { + case <-ctx.Done(): + // Main ctx + // cancelled. + cncl() + + case <-w.queue.Wait(): + // A new message was + // queued, re-add this + // to backlog + retry. + w.pushBacklog(dlv) + cncl() + continue loop + + case <-backoff: + // successful + // backoff! + } + } + + // Attempt outoing delivery of request. + _, retry, err := w.client.do(&dlv.request) + if err == nil || !retry { + continue loop + } + + if dlv.attempts > maxRetries { + // Drop deliveries once + // we reach max retries. + continue loop + } + + // Determine next delivery attempt. + dlv.next = time.Now().Add(dlv.BackOff()) + + // Push to backlog. + w.pushBacklog(dlv) + } +} + +// next gets the next available delivery, blocking until available if necessary. +func (w *DeliveryWorker) next(ctx context.Context) (*delivery, bool) { + // Try pop next queued. + msg, ok := w.queue.Pop() + + if !ok { + // Check the backlog. + if len(w.backlog) > 0 { + + // Sort by 'next' time. + sortDeliveries(w.backlog) + + // Pop next delivery. + dlv := w.popBacklog() + + return dlv, true + } + + // Backlog is empty, we MUST + // block until next enqueued. + msg, ok = w.queue.PopCtx(ctx) + if !ok { + return nil, false + } + } + + // Wrap msg in delivery type. + return wrapMsg(ctx, msg), true +} + +// popBacklog pops next available from the backlog. +func (w *DeliveryWorker) popBacklog() *delivery { + if len(w.backlog) == 0 { + return nil + } + + // Pop from backlog. + dlv := w.backlog[0] + + // Shift backlog down by one. + copy(w.backlog, w.backlog[1:]) + w.backlog = w.backlog[:len(w.backlog)-1] + + return dlv +} + +// pushBacklog pushes the given delivery to backlog. +func (w *DeliveryWorker) pushBacklog(dlv *delivery) { + w.backlog = append(w.backlog, dlv) +} + +// delivery wraps request{} +// to cache logging fields. +type delivery struct { + + // cached log + // entry fields. + log log.Entry + + // next attempt time. + next time.Time + + // embedded + // request. + request +} + +// BackOff returns backoff duration to sleep for, calculated +// from the .next attempt field subtracted from current time. +func (d *delivery) BackOff() time.Duration { + if d.next.IsZero() { + return 0 + } + return time.Now().Sub(d.next) +} + +// wrapMsg wraps a received queued HTTP request message in our delivery type. +func wrapMsg(ctx context.Context, msg queue.HTTPRequest) *delivery { + dlv := new(delivery) + dlv.request = wrapRequest(msg.Request) + dlv.log = requestLog(dlv.req) + dlv.req = dlv.req.WithContext(ctx) + return dlv +} + +// sortDeliveries sorts deliveries according +// to when is the first requiring re-attempt. +func sortDeliveries(d []*delivery) { + slices.SortFunc(d, func(a, b *delivery) int { + const k = +1 + switch { + case a.next.Before(b.next): + return +k + case b.next.Before(a.next): + return -k + default: + return 0 + } + }) +} diff --git a/internal/httpclient/request.go b/internal/httpclient/request.go new file mode 100644 index 0000000000..8fdbb729f2 --- /dev/null +++ b/internal/httpclient/request.go @@ -0,0 +1,72 @@ +// 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 . + +package httpclient + +import ( + "net/http" + "time" + + "github.com/superseriousbusiness/gotosocial/internal/log" +) + +const ( + // max no. attempts. + maxRetries = 5 + + // starting backoff duration. + baseBackoff = 2 * time.Second +) + +// request wraps an HTTP request +// to add our own retry / backoff. +type request struct { + + // underlying request. + req *http.Request + + // current backoff dur. + backoff time.Duration + + // delivery attempts. + attempts int +} + +// wrapRequest wraps an http.Request{} in our own request{} type. +func wrapRequest(req *http.Request) request { + var r request + r.req = req + return r +} + +// requestLog returns a prepared log entry with fields for http.Request{}. +func requestLog(r *http.Request) log.Entry { + return log.WithContext(r.Context()). + WithField("method", r.Method). + WithField("url", r.URL.String()) +} + +// BackOff returns the currently set backoff duration, +// setting a default according to no. attempts if needed. +func (r *request) BackOff() time.Duration { + if r.backoff <= 0 { + // No backoff dur found, set our predefined + // backoff according to a multiplier of 2^n. + r.backoff = baseBackoff * 1 << (r.attempts + 1) + } + return r.backoff +} diff --git a/internal/httpclient/util.go b/internal/httpclient/util.go new file mode 100644 index 0000000000..f8a2bf73b4 --- /dev/null +++ b/internal/httpclient/util.go @@ -0,0 +1,9 @@ +package httpclient + +import "time" + +// sleepch returns a blocking sleep channel and cancel function. +func sleepch(d time.Duration) (<-chan time.Time, func() bool) { + t := time.NewTimer(d) + return t.C, t.Stop +} diff --git a/internal/httpclient/validate.go b/internal/httpclient/validate.go index 881d3f6992..5a6257288c 100644 --- a/internal/httpclient/validate.go +++ b/internal/httpclient/validate.go @@ -38,7 +38,7 @@ func ValidateRequest(r *http.Request) error { return fmt.Errorf("%w: empty url host", ErrInvalidRequest) case r.URL.Scheme != "http" && r.URL.Scheme != "https": return fmt.Errorf("%w: unsupported protocol %q", ErrInvalidRequest, r.URL.Scheme) - case strings.IndexFunc(r.Method, func(r rune) bool { return !httpguts.IsTokenRune(r) }) != -1: + case strings.IndexFunc(r.Method, isNotTokenRune) != -1: return fmt.Errorf("%w: invalid method %q", ErrInvalidRequest, r.Method) } @@ -60,3 +60,8 @@ func ValidateRequest(r *http.Request) error { return nil } + +// isNotTokenRune wraps IsTokenRune to inverse result. +func isNotTokenRune(r rune) bool { + return !httpguts.IsTokenRune(r) +} diff --git a/internal/queue/messages.go b/internal/queue/messages.go new file mode 100644 index 0000000000..585b2ab4b8 --- /dev/null +++ b/internal/queue/messages.go @@ -0,0 +1,68 @@ +// 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 . + +package queue + +import ( + "net/http" +) + +// TODO: add indexable queues for +// fedi / client api workers +// type ClientAPIMsg struct { +// // ... +// APObjectType string +// // ... +// APActivityType string +// // ... +// GTSID string +// // ... +// GTSModel any +// // ... +// Origin *gtsmodel.Account +// // ... +// Target *gtsmodel.Account +// } +// +// type FediAPIMsg struct { +// // ... +// APObjectType string +// // ... +// APActivityType string +// // ... +// APObjectID *url.URL +// // ... +// APObjectModel any +// // ... +// GTSModel any +// // ... +// Requesting *gtsmodel.Account +// // ... +// Receiving *gtsmodel.Account +// } + +type HTTPRequest struct { + + // ObjectID ... + ObjectID string + + // Request ... + Request *http.Request + + // Signer ... + Signer func(*http.Request) error +} diff --git a/internal/queue/queues.go b/internal/queue/queues.go new file mode 100644 index 0000000000..62242b1396 --- /dev/null +++ b/internal/queue/queues.go @@ -0,0 +1,46 @@ +// 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 . + +package queue + +import ( + "codeberg.org/gruf/go-structr" + "github.com/superseriousbusiness/gotosocial/internal/log" +) + +// Queues ... +type Queues struct { + // HTTPRequest ... + HTTPRequest StructQueue[*HTTPRequest] +} + +// Init will re(initialize) queues. NOTE: the queue +// MUST NOT be in use anywhere, this is not thread-safe. +func (q *Queues) Init() { + log.Infof(nil, "init: %p", q) + + q.initHTTPRequest() +} + +func (q *Queues) initHTTPRequest() { + q.HTTPRequest.Init(structr.QueueConfig[*HTTPRequest]{ + Indices: []structr.IndexConfig{ + {Fields: "ObjectID", Multiple: true}, + {Fields: "Request.URL.Host", Multiple: true}, + }, + }) +} diff --git a/internal/queue/wrappers.go b/internal/queue/wrappers.go new file mode 100644 index 0000000000..087ae1f39d --- /dev/null +++ b/internal/queue/wrappers.go @@ -0,0 +1,115 @@ +// 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 . + +package queue + +import ( + "context" + "sync/atomic" + + "codeberg.org/gruf/go-structr" +) + +// StructQueue ... +type StructQueue[StructType any] struct { + queue structr.Queue[StructType] + index map[string]*structr.Index + wait atomic.Pointer[chan struct{}] +} + +// Init initializes queue with structr.QueueConfig{}. +func (q *StructQueue[T]) Init(config structr.QueueConfig[T]) { + q.index = make(map[string]*structr.Index, len(config.Indices)) + q.queue = structr.Queue[T]{} + q.queue.Init(config) + for _, cfg := range config.Indices { + q.index[cfg.Fields] = q.queue.Index(cfg.Fields) + } +} + +// Pop: see structr.Queue{}.PopFront(). +func (q *StructQueue[T]) Pop() (value T, ok bool) { + return q.queue.PopFront() +} + +// PopCtx wraps structr.Queue{}.PopFront() to add sleep until value is available. +func (q *StructQueue[T]) PopCtx(ctx context.Context) (value T, ok bool) { + for { + // Try pop from front of queue. + value, ok = q.queue.PopFront() + if ok { + return + } + + select { + // Context canceled. + case <-ctx.Done(): + return + + // Waiter released. + case <-q.Wait(): + } + } +} + +// Push wraps structr.Queue{}.PushBack() to add sleeping pop goroutine awakening. +func (q *StructQueue[T]) Push(values ...T) { + q.queue.PushBack(values...) + q.broadcast() +} + +// MoveBack ... +func (q *StructQueue[T]) MoveBack(index string, key ...any) { + i := q.index[index] + q.queue.MoveBack(i, i.Key(key...)) +} + +// Len: see structr.Queue{}.Len(). +func (q *StructQueue[T]) Len() int { + return q.queue.Len() +} + +// Wait safely returns current (read-only) wait channel. +func (q *StructQueue[T]) Wait() <-chan struct{} { + var ch chan struct{} + + for { + // Get channel ptr. + ptr := q.wait.Load() + if ptr != nil { + return *ptr + } + + if ch == nil { + // Allocate new channel. + ch = make(chan struct{}) + } + + // Try set the new wait channel ptr. + if q.wait.CompareAndSwap(ptr, &ch) { + return ch + } + } +} + +// broadcast safely closes wait channel if +// currently set, releasing waiting goroutines. +func (q *StructQueue[T]) broadcast() { + if ptr := q.wait.Swap(nil); ptr != nil { + close(*ptr) + } +} diff --git a/internal/state/state.go b/internal/state/state.go index 6120515b91..5c64d12883 100644 --- a/internal/state/state.go +++ b/internal/state/state.go @@ -21,6 +21,7 @@ import ( "codeberg.org/gruf/go-mutexes" "github.com/superseriousbusiness/gotosocial/internal/cache" "github.com/superseriousbusiness/gotosocial/internal/db" + "github.com/superseriousbusiness/gotosocial/internal/queue" "github.com/superseriousbusiness/gotosocial/internal/storage" "github.com/superseriousbusiness/gotosocial/internal/timeline" "github.com/superseriousbusiness/gotosocial/internal/workers" @@ -36,6 +37,9 @@ type State struct { // Caches provides access to this state's collection of caches. Caches cache.Caches + // Queues provides access to this state's collection of queues. + Queues queue.Queues + // Timelines provides access to this state's collection of timelines. Timelines timeline.Timelines diff --git a/internal/workers/workers.go b/internal/workers/workers.go index 3617ce333d..648310a4ba 100644 --- a/internal/workers/workers.go +++ b/internal/workers/workers.go @@ -23,7 +23,9 @@ import ( "runtime" "codeberg.org/gruf/go-runners" + "github.com/superseriousbusiness/gotosocial/internal/httpclient" "github.com/superseriousbusiness/gotosocial/internal/messages" + "github.com/superseriousbusiness/gotosocial/internal/queue" "github.com/superseriousbusiness/gotosocial/internal/scheduler" ) @@ -31,6 +33,9 @@ type Workers struct { // Main task scheduler instance. Scheduler scheduler.Scheduler + // Delivery ... + Delivery httpclient.DeliveryWorkerPool + // ClientAPI provides a worker pool that handles both // incoming client actions, and our own side-effects. ClientAPI runners.WorkerPool @@ -42,8 +47,9 @@ type Workers struct { // Enqueue functions for clientAPI / federator worker pools, // these are pointers to Processor{}.Enqueue___() msg functions. // This prevents dependency cycling as Processor depends on Workers. - EnqueueClientAPI func(context.Context, ...messages.FromClientAPI) - EnqueueFediAPI func(context.Context, ...messages.FromFediAPI) + EnqueueHTTPClient func(context.Context, ...queue.HTTPRequest) + EnqueueClientAPI func(context.Context, ...messages.FromClientAPI) + EnqueueFediAPI func(context.Context, ...messages.FromFediAPI) // Blocking processing functions for clientAPI / federator. // These are pointers to Processor{}.Process___() msg functions. @@ -72,6 +78,8 @@ func (w *Workers) Start() { tryUntil("starting scheduler", 5, w.Scheduler.Start) + tryUntil("start http client workerpool", 5, w.Delivery.Start) + tryUntil("starting client API workerpool", 5, func() bool { return w.ClientAPI.Start(4*maxprocs, 400*maxprocs) }) @@ -88,6 +96,7 @@ func (w *Workers) Start() { // Stop will stop all of the contained worker pools (and global scheduler). func (w *Workers) Stop() { tryUntil("stopping scheduler", 5, w.Scheduler.Stop) + tryUntil("stopping http client workerpool", 5, w.Delivery.Stop) tryUntil("stopping client API workerpool", 5, w.ClientAPI.Stop) tryUntil("stopping federator workerpool", 5, w.Federator.Stop) tryUntil("stopping media workerpool", 5, w.Media.Stop) From 8aede54741c95b9dd45d34b56b067bb3bedaa066 Mon Sep 17 00:00:00 2001 From: kim Date: Wed, 3 Apr 2024 11:30:03 +0100 Subject: [PATCH 02/35] finish up some code commenting, bodge a vendored activity library change, integrate the deliverypool changes into transportcontroller --- cmd/gotosocial/action/server/server.go | 6 + internal/cache/cache.go | 5 - internal/httpclient/client.go | 21 ++- internal/httpclient/delivery.go | 28 +-- internal/httpclient/util.go | 17 ++ internal/queue/messages.go | 3 - internal/transport/deliver.go | 161 ++++++++++-------- internal/transport/transport.go | 7 +- internal/workers/workers.go | 8 +- .../activity/pub/side_effect_actor.go | 8 +- .../activity/pub/transport.go | 67 +++++--- 11 files changed, 193 insertions(+), 138 deletions(-) diff --git a/cmd/gotosocial/action/server/server.go b/cmd/gotosocial/action/server/server.go index 1886cd8852..608ab7b240 100644 --- a/cmd/gotosocial/action/server/server.go +++ b/cmd/gotosocial/action/server/server.go @@ -24,6 +24,7 @@ import ( "net/http" "os" "os/signal" + "runtime" "syscall" "time" @@ -124,6 +125,11 @@ var Start action.GTSAction = func(ctx context.Context) error { TLSInsecureSkipVerify: config.GetHTTPClientTLSInsecureSkipVerify(), }) + // Initialize the queues. + state.Queues.Init() + + state.Workers.HTTPClient.Init(client, &state.Queues.HTTPRequest, runtime.GOMAXPROCS(0)) + // Initialize workers. state.Workers.Start() defer state.Workers.Stop() diff --git a/internal/cache/cache.go b/internal/cache/cache.go index dc2be55569..3aa21cdd00 100644 --- a/internal/cache/cache.go +++ b/internal/cache/cache.go @@ -20,17 +20,12 @@ package cache import ( "time" - "codeberg.org/gruf/go-cache/v3/ttl" "github.com/superseriousbusiness/gotosocial/internal/cache/headerfilter" "github.com/superseriousbusiness/gotosocial/internal/log" ) type Caches struct { - // BadHosts provides access to the HTTP - // client bad (i.e. erroring) hosts cache. - BadHosts ttl.Cache[string, struct{}] - // GTS provides access to the collection of // gtsmodel object caches. (used by the database). GTS GTSCaches diff --git a/internal/httpclient/client.go b/internal/httpclient/client.go index 5682f2a5e6..01238eb223 100644 --- a/internal/httpclient/client.go +++ b/internal/httpclient/client.go @@ -32,12 +32,12 @@ import ( "time" "codeberg.org/gruf/go-bytesize" + "codeberg.org/gruf/go-cache/v3" errorsv2 "codeberg.org/gruf/go-errors/v2" "codeberg.org/gruf/go-iotools" "github.com/superseriousbusiness/gotosocial/internal/gtscontext" "github.com/superseriousbusiness/gotosocial/internal/gtserror" "github.com/superseriousbusiness/gotosocial/internal/log" - "github.com/superseriousbusiness/gotosocial/internal/state" ) var ( @@ -105,9 +105,9 @@ type Config struct { // - optional request signing // - request logging type Client struct { - state *state.State - client http.Client - bodyMax int64 + client http.Client + badHosts cache.TTLCache[string, struct{}] + bodyMax int64 } // New returns a new instance of Client initialized using configuration. @@ -175,6 +175,13 @@ func New(cfg Config) *Client { DisableCompression: cfg.DisableCompression, }} + // Initiate outgoing bad hosts lookup cache. + c.badHosts = cache.NewTTL[string, struct{}](0, 512, 0) + c.badHosts.SetTTL(time.Hour, false) + if !c.badHosts.Start(time.Minute) { + log.Panic(nil, "failed to start transport controller cache") + } + return &c } @@ -197,11 +204,11 @@ func (c *Client) Do(r *http.Request) (rsp *http.Response, err error) { // errors that are retried upon are server failure, TLS // and domain resolution type errors, so this cached result // indicates this server is likely having issues. - fastFail = c.state.Caches.BadHosts.Has(host) + fastFail = c.badHosts.Has(host) defer func() { if err != nil { - // On error return ensure marked as bad-host. - c.state.Caches.BadHosts.Set(host, struct{}{}) + // On error mark as a bad-host. + c.badHosts.Set(host, struct{}{}) } }() } diff --git a/internal/httpclient/delivery.go b/internal/httpclient/delivery.go index bd1776d142..ee0d05eeb4 100644 --- a/internal/httpclient/delivery.go +++ b/internal/httpclient/delivery.go @@ -27,20 +27,21 @@ import ( "github.com/superseriousbusiness/gotosocial/internal/queue" ) +// DeliveryWorkerPool ... type DeliveryWorkerPool struct { client *Client - queue *queue.StructQueue[queue.HTTPRequest] + queue *queue.StructQueue[*queue.HTTPRequest] workers []DeliveryWorker } -// Init ... +// Init will initialize the DeliveryWorker{} pool +// with given http client, request queue to pull +// from and number of delivery workers to spawn. func (p *DeliveryWorkerPool) Init( client *Client, - queue *queue.StructQueue[queue.HTTPRequest], + queue *queue.StructQueue[*queue.HTTPRequest], workers int, ) { - p.client = client - p.queue = queue p.workers = make([]DeliveryWorker, workers) for i := range p.workers { p.workers[i] = NewDeliveryWorker( @@ -50,7 +51,8 @@ func (p *DeliveryWorkerPool) Init( } } -// Start ... +// Start will attempt to start all of the contained DeliveryWorker{}s. +// NOTE: this is not safe to call concurrently with .Init(). func (p *DeliveryWorkerPool) Start() bool { if len(p.workers) == 0 { return false @@ -62,7 +64,8 @@ func (p *DeliveryWorkerPool) Start() bool { return ok } -// Stop ... +// Stop will attempt to stop all of the contained DeliveryWorker{}s. +// NOTE: this is not safe to call concurrently with .Init(). func (p *DeliveryWorkerPool) Stop() bool { if len(p.workers) == 0 { return false @@ -74,15 +77,16 @@ func (p *DeliveryWorkerPool) Stop() bool { return ok } +// DeliveryWorker ... type DeliveryWorker struct { client *Client - queue *queue.StructQueue[queue.HTTPRequest] + queue *queue.StructQueue[*queue.HTTPRequest] backlog []*delivery service runners.Service } // NewDeliveryWorker returns a new DeliveryWorker that feeds from queue, using given HTTP client. -func NewDeliveryWorker(client *Client, queue *queue.StructQueue[queue.HTTPRequest]) DeliveryWorker { +func NewDeliveryWorker(client *Client, queue *queue.StructQueue[*queue.HTTPRequest]) DeliveryWorker { return DeliveryWorker{ client: client, queue: queue, @@ -90,12 +94,12 @@ func NewDeliveryWorker(client *Client, queue *queue.StructQueue[queue.HTTPReques } } -// Start ... +// Start will attempt to start the DeliveryWorker{}. func (w *DeliveryWorker) Start() bool { return w.service.Run(w.process) } -// Stop ... +// Stop will attempt to stop the DeliveryWorker{}. func (w *DeliveryWorker) Stop() bool { return w.service.Stop() } @@ -237,7 +241,7 @@ func (d *delivery) BackOff() time.Duration { } // wrapMsg wraps a received queued HTTP request message in our delivery type. -func wrapMsg(ctx context.Context, msg queue.HTTPRequest) *delivery { +func wrapMsg(ctx context.Context, msg *queue.HTTPRequest) *delivery { dlv := new(delivery) dlv.request = wrapRequest(msg.Request) dlv.log = requestLog(dlv.req) diff --git a/internal/httpclient/util.go b/internal/httpclient/util.go index f8a2bf73b4..c19418c963 100644 --- a/internal/httpclient/util.go +++ b/internal/httpclient/util.go @@ -1,3 +1,20 @@ +// 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 . + package httpclient import "time" diff --git a/internal/queue/messages.go b/internal/queue/messages.go index 585b2ab4b8..fc96c911a7 100644 --- a/internal/queue/messages.go +++ b/internal/queue/messages.go @@ -62,7 +62,4 @@ type HTTPRequest struct { // Request ... Request *http.Request - - // Signer ... - Signer func(*http.Request) error } diff --git a/internal/transport/deliver.go b/internal/transport/deliver.go index fe4d045821..ab44ecca54 100644 --- a/internal/transport/deliver.go +++ b/internal/transport/deliver.go @@ -19,118 +19,139 @@ package transport import ( "context" + "encoding/json" "net/http" "net/url" - "sync" "codeberg.org/gruf/go-byteutil" apiutil "github.com/superseriousbusiness/gotosocial/internal/api/util" "github.com/superseriousbusiness/gotosocial/internal/config" + "github.com/superseriousbusiness/gotosocial/internal/gtscontext" "github.com/superseriousbusiness/gotosocial/internal/gtserror" + "github.com/superseriousbusiness/gotosocial/internal/queue" ) -func (t *transport) BatchDeliver(ctx context.Context, b []byte, recipients []*url.URL) error { +func (t *transport) BatchDeliver(ctx context.Context, obj map[string]interface{}, recipients []*url.URL) error { var ( - // errs accumulates errors received during - // attempted delivery by deliverer routines. - errs gtserror.MultiError - - // wait blocks until all sender - // routines have returned. - wait sync.WaitGroup + // accumulated prepared reqs. + reqs []*queue.HTTPRequest - // mutex protects 'recipients' and - // 'errs' for concurrent access. - mutex sync.Mutex + // accumulated preparation errs. + errs gtserror.MultiError // Get current instance host info. domain = config.GetAccountDomain() host = config.GetHost() ) - // Block on expect no. senders. - wait.Add(t.controller.senders) - - for i := 0; i < t.controller.senders; i++ { - go func() { - // Mark returned. - defer wait.Done() - - for { - // Acquire lock. - mutex.Lock() - - if len(recipients) == 0 { - // Reached end. - mutex.Unlock() - return - } - - // Pop next recipient. - i := len(recipients) - 1 - to := recipients[i] - recipients = recipients[:i] - - // Done with lock. - mutex.Unlock() - - // Skip delivery to recipient if it is "us". - if to.Host == host || to.Host == domain { - continue - } - - // Attempt to deliver data to recipient. - if err := t.deliver(ctx, b, to); err != nil { - mutex.Lock() // safely append err to accumulator. - errs.Appendf("error delivering to %s: %w", to, err) - mutex.Unlock() - } - } - }() + // Marshal object as JSON. + b, err := json.Marshal(obj) + if err != nil { + return gtserror.Newf("error marshaling json: %w", err) + } + + // Extract object ID. + id := getObjectID(obj) + + for _, to := range recipients { + // Skip delivery to recipient if it is "us". + if to.Host == host || to.Host == domain { + continue + } + + // Prepare new http client request. + req, err := t.prepare(ctx, id, b, to) + if err != nil { + errs.Append(err) + continue + } + + // Append to request queue. + reqs = append(reqs, req) } - // Wait for finish. - wait.Wait() + // Push the request list to HTTP client worker queue. + t.controller.state.Queues.HTTPRequest.Push(reqs...) // Return combined err. return errs.Combine() } -func (t *transport) Deliver(ctx context.Context, b []byte, to *url.URL) error { +func (t *transport) Deliver(ctx context.Context, obj map[string]interface{}, to *url.URL) error { // if 'to' host is our own, skip as we don't need to deliver to ourselves... if to.Host == config.GetHost() || to.Host == config.GetAccountDomain() { return nil } - // Deliver data to recipient. - return t.deliver(ctx, b, to) + // Marshal object as JSON. + b, err := json.Marshal(obj) + if err != nil { + return gtserror.Newf("error marshaling json: %w", err) + } + + // Extract object ID. + id := getObjectID(obj) + + // Prepare new http client request. + req, err := t.prepare(ctx, id, b, to) + if err != nil { + return err + } + + // Push the request to HTTP client worker queue. + t.controller.state.Queues.HTTPRequest.Push(req) + + return nil } -func (t *transport) deliver(ctx context.Context, b []byte, to *url.URL) error { +// prepare will prepare a POST http.Request{} +// to recipient at 'to', wrapping in a queued +// request object with signing function. +func (t *transport) prepare( + ctx context.Context, + objectID string, + data []byte, + to *url.URL, +) ( + *queue.HTTPRequest, + error, +) { url := to.String() - // Use rewindable bytes reader for body. + // Use rewindable reader for body. var body byteutil.ReadNopCloser - body.Reset(b) + body.Reset(data) + + // Prepare POST signer. + sign := t.signPOST(data) + + // Update to-be-used request context with signing details. + ctx = gtscontext.SetOutgoingPublicKeyID(ctx, t.pubKeyID) + ctx = gtscontext.SetHTTPClientSignFunc(ctx, sign) req, err := http.NewRequestWithContext(ctx, "POST", url, &body) if err != nil { - return err + return nil, gtserror.Newf("error preparing request: %w", err) } req.Header.Add("Content-Type", string(apiutil.AppActivityLDJSON)) req.Header.Add("Accept-Charset", "utf-8") - rsp, err := t.POST(req, b) - if err != nil { - return err - } - defer rsp.Body.Close() + return &queue.HTTPRequest{ + ObjectID: objectID, + Request: req, + }, nil +} - if code := rsp.StatusCode; code != http.StatusOK && - code != http.StatusCreated && code != http.StatusAccepted { - return gtserror.NewFromResponse(rsp) +// getObjectID extracts an object ID from 'serialized' ActivityPub object map. +func getObjectID(obj map[string]interface{}) string { + switch t := obj["object"].(type) { + case string: + return t + case map[string]interface{}: + id, _ := t["id"].(string) + return id + default: + return "" } - - return nil } diff --git a/internal/transport/transport.go b/internal/transport/transport.go index 3ae5c89671..110c19b3d4 100644 --- a/internal/transport/transport.go +++ b/internal/transport/transport.go @@ -51,10 +51,10 @@ type Transport interface { POST(*http.Request, []byte) (*http.Response, error) // Deliver sends an ActivityStreams object. - Deliver(ctx context.Context, b []byte, to *url.URL) error + Deliver(ctx context.Context, obj map[string]interface{}, to *url.URL) error // BatchDeliver sends an ActivityStreams object to multiple recipients. - BatchDeliver(ctx context.Context, b []byte, recipients []*url.URL) error + BatchDeliver(ctx context.Context, obj map[string]interface{}, recipients []*url.URL) error /* GET functions @@ -77,7 +77,8 @@ type Transport interface { Finger(ctx context.Context, targetUsername string, targetDomain string) ([]byte, error) } -// transport implements the Transport interface. +// transport implements +// the Transport interface. type transport struct { controller *controller pubKeyID string diff --git a/internal/workers/workers.go b/internal/workers/workers.go index 648310a4ba..574e6f8150 100644 --- a/internal/workers/workers.go +++ b/internal/workers/workers.go @@ -33,8 +33,8 @@ type Workers struct { // Main task scheduler instance. Scheduler scheduler.Scheduler - // Delivery ... - Delivery httpclient.DeliveryWorkerPool + // HTTPClient ... + HTTPClient httpclient.DeliveryWorkerPool // ClientAPI provides a worker pool that handles both // incoming client actions, and our own side-effects. @@ -78,7 +78,7 @@ func (w *Workers) Start() { tryUntil("starting scheduler", 5, w.Scheduler.Start) - tryUntil("start http client workerpool", 5, w.Delivery.Start) + tryUntil("start http client workerpool", 5, w.HTTPClient.Start) tryUntil("starting client API workerpool", 5, func() bool { return w.ClientAPI.Start(4*maxprocs, 400*maxprocs) @@ -96,7 +96,7 @@ func (w *Workers) Start() { // Stop will stop all of the contained worker pools (and global scheduler). func (w *Workers) Stop() { tryUntil("stopping scheduler", 5, w.Scheduler.Stop) - tryUntil("stopping http client workerpool", 5, w.Delivery.Stop) + tryUntil("stopping http client workerpool", 5, w.HTTPClient.Stop) tryUntil("stopping client API workerpool", 5, w.ClientAPI.Stop) tryUntil("stopping federator workerpool", 5, w.Federator.Stop) tryUntil("stopping media workerpool", 5, w.Media.Stop) diff --git a/vendor/github.com/superseriousbusiness/activity/pub/side_effect_actor.go b/vendor/github.com/superseriousbusiness/activity/pub/side_effect_actor.go index 4062cf5078..0c1da9e91e 100644 --- a/vendor/github.com/superseriousbusiness/activity/pub/side_effect_actor.go +++ b/vendor/github.com/superseriousbusiness/activity/pub/side_effect_actor.go @@ -2,7 +2,6 @@ package pub import ( "context" - "encoding/json" "fmt" "net/http" "net/url" @@ -477,17 +476,12 @@ func (a *SideEffectActor) deliverToRecipients(c context.Context, boxIRI *url.URL return err } - b, err := json.Marshal(m) - if err != nil { - return err - } - tp, err := a.common.NewTransport(c, boxIRI, goFedUserAgent()) if err != nil { return err } - return tp.BatchDeliver(c, b, recipients) + return tp.BatchDeliver(c, m, recipients) } // addToOutbox adds the activity to the outbox and creates the activity in the diff --git a/vendor/github.com/superseriousbusiness/activity/pub/transport.go b/vendor/github.com/superseriousbusiness/activity/pub/transport.go index a770b8b468..101ff5c079 100644 --- a/vendor/github.com/superseriousbusiness/activity/pub/transport.go +++ b/vendor/github.com/superseriousbusiness/activity/pub/transport.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "crypto" + "encoding/json" "fmt" "net/http" "net/url" @@ -44,10 +45,10 @@ type Transport interface { Dereference(c context.Context, iri *url.URL) (*http.Response, error) // Deliver sends an ActivityStreams object. - Deliver(c context.Context, b []byte, to *url.URL) error + Deliver(c context.Context, obj map[string]interface{}, to *url.URL) error // BatchDeliver sends an ActivityStreams object to multiple recipients. - BatchDeliver(c context.Context, b []byte, recipients []*url.URL) error + BatchDeliver(c context.Context, obj map[string]interface{}, recipients []*url.URL) error } // Transport must be implemented by HttpSigTransport. @@ -138,43 +139,27 @@ func (h HttpSigTransport) Dereference(c context.Context, iri *url.URL) (*http.Re } // Deliver sends a POST request with an HTTP Signature. -func (h HttpSigTransport) Deliver(c context.Context, b []byte, to *url.URL) error { - req, err := http.NewRequest("POST", to.String(), bytes.NewReader(b)) - if err != nil { - return err - } - req = req.WithContext(c) - req.Header.Add(contentTypeHeader, contentTypeHeaderValue) - req.Header.Add("Accept-Charset", "utf-8") - req.Header.Add("Date", h.clock.Now().UTC().Format("Mon, 02 Jan 2006 15:04:05")+" GMT") - req.Header.Add("User-Agent", fmt.Sprintf("%s %s", h.appAgent, h.gofedAgent)) - req.Header.Set("Host", to.Host) - h.postSignerMu.Lock() - err = h.postSigner.SignRequest(h.privKey, h.pubKeyId, req, b) - h.postSignerMu.Unlock() +func (h HttpSigTransport) Deliver(c context.Context, data map[string]interface{}, to *url.URL) error { + b, err := json.Marshal(data) if err != nil { return err } - resp, err := h.client.Do(req) - if err != nil { - return err - } - defer resp.Body.Close() - if !isSuccess(resp.StatusCode) { - return fmt.Errorf("POST request to %s failed (%d): %s", to.String(), resp.StatusCode, resp.Status) - } - return nil + return h.deliver(c, b, to) } // BatchDeliver sends concurrent POST requests. Returns an error if any of the requests had an error. -func (h HttpSigTransport) BatchDeliver(c context.Context, b []byte, recipients []*url.URL) error { +func (h HttpSigTransport) BatchDeliver(c context.Context, data map[string]interface{}, recipients []*url.URL) error { + b, err := json.Marshal(data) + if err != nil { + return err + } var wg sync.WaitGroup errCh := make(chan error, len(recipients)) for _, recipient := range recipients { wg.Add(1) go func(r *url.URL) { defer wg.Done() - if err := h.Deliver(c, b, r); err != nil { + if err := h.deliver(c, b, r); err != nil { errCh <- err } }(recipient) @@ -196,6 +181,34 @@ outer: return nil } +func (h HttpSigTransport) deliver(c context.Context, b []byte, to *url.URL) error { + req, err := http.NewRequest("POST", to.String(), bytes.NewReader(b)) + if err != nil { + return err + } + req = req.WithContext(c) + req.Header.Add(contentTypeHeader, contentTypeHeaderValue) + req.Header.Add("Accept-Charset", "utf-8") + req.Header.Add("Date", h.clock.Now().UTC().Format("Mon, 02 Jan 2006 15:04:05")+" GMT") + req.Header.Add("User-Agent", fmt.Sprintf("%s %s", h.appAgent, h.gofedAgent)) + req.Header.Set("Host", to.Host) + h.postSignerMu.Lock() + err = h.postSigner.SignRequest(h.privKey, h.pubKeyId, req, b) + h.postSignerMu.Unlock() + if err != nil { + return err + } + resp, err := h.client.Do(req) + if err != nil { + return err + } + defer resp.Body.Close() + if !isSuccess(resp.StatusCode) { + return fmt.Errorf("POST request to %s failed (%d): %s", to.String(), resp.StatusCode, resp.Status) + } + return nil +} + // HttpClient sends http requests, and is an abstraction only needed by the // HttpSigTransport. The standard library's Client satisfies this interface. type HttpClient interface { From 8fdd5b18aa1f63fff7f3ad048af25216c17726be Mon Sep 17 00:00:00 2001 From: kim Date: Wed, 3 Apr 2024 16:54:22 +0100 Subject: [PATCH 03/35] hook up queue deletion logic --- cmd/gotosocial/action/server/server.go | 2 +- internal/federation/federatingdb/delete.go | 4 +- internal/httpclient/delivery.go | 52 +++++++++++----------- internal/processing/workers/federate.go | 9 ++++ internal/queue/messages.go | 5 ++- internal/queue/queues.go | 6 +-- internal/queue/wrappers.go | 6 +-- internal/transport/deliver.go | 45 +++++++++++++------ internal/workers/workers.go | 14 +++--- 9 files changed, 84 insertions(+), 59 deletions(-) diff --git a/cmd/gotosocial/action/server/server.go b/cmd/gotosocial/action/server/server.go index 608ab7b240..6f486b71a8 100644 --- a/cmd/gotosocial/action/server/server.go +++ b/cmd/gotosocial/action/server/server.go @@ -128,7 +128,7 @@ var Start action.GTSAction = func(ctx context.Context) error { // Initialize the queues. state.Queues.Init() - state.Workers.HTTPClient.Init(client, &state.Queues.HTTPRequest, runtime.GOMAXPROCS(0)) + state.Workers.APDelivery.Init(client, &state.Queues.APRequests, runtime.GOMAXPROCS(0)) // Initialize workers. state.Workers.Start() diff --git a/internal/federation/federatingdb/delete.go b/internal/federation/federatingdb/delete.go index 7390cf0f54..384291463d 100644 --- a/internal/federation/federatingdb/delete.go +++ b/internal/federation/federatingdb/delete.go @@ -51,7 +51,7 @@ func (f *federatingDB) Delete(ctx context.Context, id *url.URL) error { // in a delete we only get the URI, we can't know if we have a status or a profile or something else, // so we have to try a few different things... if s, err := f.state.DB.GetStatusByURI(ctx, id.String()); err == nil && requestingAcct.ID == s.AccountID { - l.Debugf("uri is for STATUS with id: %s", s.ID) + l.Debugf("deleting status: %s", s.ID) f.state.Workers.EnqueueFediAPI(ctx, messages.FromFediAPI{ APObjectType: ap.ObjectNote, APActivityType: ap.ActivityDelete, @@ -61,7 +61,7 @@ func (f *federatingDB) Delete(ctx context.Context, id *url.URL) error { } if a, err := f.state.DB.GetAccountByURI(ctx, id.String()); err == nil && requestingAcct.ID == a.ID { - l.Debugf("uri is for ACCOUNT with id %s", a.ID) + l.Debugf("deleting account: %s", a.ID) f.state.Workers.EnqueueFediAPI(ctx, messages.FromFediAPI{ APObjectType: ap.ObjectProfile, APActivityType: ap.ActivityDelete, diff --git a/internal/httpclient/delivery.go b/internal/httpclient/delivery.go index ee0d05eeb4..ae00f94a1a 100644 --- a/internal/httpclient/delivery.go +++ b/internal/httpclient/delivery.go @@ -27,33 +27,31 @@ import ( "github.com/superseriousbusiness/gotosocial/internal/queue" ) -// DeliveryWorkerPool ... -type DeliveryWorkerPool struct { - client *Client - queue *queue.StructQueue[*queue.HTTPRequest] - workers []DeliveryWorker +// APDeliveryWorkerPool ... +type APDeliveryWorkerPool struct { + workers []APDeliveryWorker } // Init will initialize the DeliveryWorker{} pool // with given http client, request queue to pull // from and number of delivery workers to spawn. -func (p *DeliveryWorkerPool) Init( +func (p *APDeliveryWorkerPool) Init( client *Client, - queue *queue.StructQueue[*queue.HTTPRequest], + queue *queue.StructQueue[*queue.APRequest], workers int, ) { - p.workers = make([]DeliveryWorker, workers) + p.workers = make([]APDeliveryWorker, workers) for i := range p.workers { - p.workers[i] = NewDeliveryWorker( - p.client, - p.queue, + p.workers[i] = NewAPDeliveryWorker( + client, + queue, ) } } // Start will attempt to start all of the contained DeliveryWorker{}s. // NOTE: this is not safe to call concurrently with .Init(). -func (p *DeliveryWorkerPool) Start() bool { +func (p *APDeliveryWorkerPool) Start() bool { if len(p.workers) == 0 { return false } @@ -66,7 +64,7 @@ func (p *DeliveryWorkerPool) Start() bool { // Stop will attempt to stop all of the contained DeliveryWorker{}s. // NOTE: this is not safe to call concurrently with .Init(). -func (p *DeliveryWorkerPool) Stop() bool { +func (p *APDeliveryWorkerPool) Stop() bool { if len(p.workers) == 0 { return false } @@ -77,17 +75,17 @@ func (p *DeliveryWorkerPool) Stop() bool { return ok } -// DeliveryWorker ... -type DeliveryWorker struct { +// APDeliveryWorker ... +type APDeliveryWorker struct { client *Client - queue *queue.StructQueue[*queue.HTTPRequest] + queue *queue.StructQueue[*queue.APRequest] backlog []*delivery service runners.Service } -// NewDeliveryWorker returns a new DeliveryWorker that feeds from queue, using given HTTP client. -func NewDeliveryWorker(client *Client, queue *queue.StructQueue[*queue.HTTPRequest]) DeliveryWorker { - return DeliveryWorker{ +// NewAPDeliveryWorker returns a new APDeliveryWorker that feeds from queue, using given HTTP client. +func NewAPDeliveryWorker(client *Client, queue *queue.StructQueue[*queue.APRequest]) APDeliveryWorker { + return APDeliveryWorker{ client: client, queue: queue, backlog: make([]*delivery, 0, 256), @@ -95,17 +93,17 @@ func NewDeliveryWorker(client *Client, queue *queue.StructQueue[*queue.HTTPReque } // Start will attempt to start the DeliveryWorker{}. -func (w *DeliveryWorker) Start() bool { +func (w *APDeliveryWorker) Start() bool { return w.service.Run(w.process) } // Stop will attempt to stop the DeliveryWorker{}. -func (w *DeliveryWorker) Stop() bool { +func (w *APDeliveryWorker) Stop() bool { return w.service.Stop() } // process is the main delivery worker processing routine. -func (w *DeliveryWorker) process(ctx context.Context) { +func (w *APDeliveryWorker) process(ctx context.Context) { if w.client == nil || w.queue == nil { panic("nil delivery worker fields") } @@ -165,7 +163,7 @@ loop: } // next gets the next available delivery, blocking until available if necessary. -func (w *DeliveryWorker) next(ctx context.Context) (*delivery, bool) { +func (w *APDeliveryWorker) next(ctx context.Context) (*delivery, bool) { // Try pop next queued. msg, ok := w.queue.Pop() @@ -195,7 +193,7 @@ func (w *DeliveryWorker) next(ctx context.Context) (*delivery, bool) { } // popBacklog pops next available from the backlog. -func (w *DeliveryWorker) popBacklog() *delivery { +func (w *APDeliveryWorker) popBacklog() *delivery { if len(w.backlog) == 0 { return nil } @@ -211,7 +209,7 @@ func (w *DeliveryWorker) popBacklog() *delivery { } // pushBacklog pushes the given delivery to backlog. -func (w *DeliveryWorker) pushBacklog(dlv *delivery) { +func (w *APDeliveryWorker) pushBacklog(dlv *delivery) { w.backlog = append(w.backlog, dlv) } @@ -240,8 +238,8 @@ func (d *delivery) BackOff() time.Duration { return time.Now().Sub(d.next) } -// wrapMsg wraps a received queued HTTP request message in our delivery type. -func wrapMsg(ctx context.Context, msg *queue.HTTPRequest) *delivery { +// wrapMsg wraps a received queued AP request message in our delivery type. +func wrapMsg(ctx context.Context, msg *queue.APRequest) *delivery { dlv := new(delivery) dlv.request = wrapRequest(msg.Request) dlv.log = requestLog(dlv.req) diff --git a/internal/processing/workers/federate.go b/internal/processing/workers/federate.go index 9fdb8f6627..0ec23d3c07 100644 --- a/internal/processing/workers/federate.go +++ b/internal/processing/workers/federate.go @@ -75,6 +75,11 @@ func (f *federate) DeleteAccount(ctx context.Context, account *gtsmodel.Account) return nil } + // Drop any queued outgoing AP requests to / from account, + // (this stops any queued likes, boosts, creates etc). + f.state.Queues.APRequests.Delete("ActorID", account.URI) + f.state.Queues.APRequests.Delete("ObjectID", account.URI) + // Parse relevant URI(s). outboxIRI, err := parseURI(account.OutboxURI) if err != nil { @@ -222,6 +227,10 @@ func (f *federate) DeleteStatus(ctx context.Context, status *gtsmodel.Status) er return nil } + // Drop any queued outgoing http requests for status, + // (this stops any queued likes, boosts, creates etc). + f.state.Queues.APRequests.Delete("ObjectID", status.URI) + // Ensure the status model is fully populated. if err := f.state.DB.PopulateStatus(ctx, status); err != nil { return gtserror.Newf("error populating status: %w", err) diff --git a/internal/queue/messages.go b/internal/queue/messages.go index fc96c911a7..86601ca6ea 100644 --- a/internal/queue/messages.go +++ b/internal/queue/messages.go @@ -55,7 +55,10 @@ import ( // Receiving *gtsmodel.Account // } -type HTTPRequest struct { +type APRequest struct { + + // ActorID ... + ActorID string // ObjectID ... ObjectID string diff --git a/internal/queue/queues.go b/internal/queue/queues.go index 62242b1396..ead24cedab 100644 --- a/internal/queue/queues.go +++ b/internal/queue/queues.go @@ -24,8 +24,8 @@ import ( // Queues ... type Queues struct { - // HTTPRequest ... - HTTPRequest StructQueue[*HTTPRequest] + // APRequests ... + APRequests StructQueue[*APRequest] } // Init will re(initialize) queues. NOTE: the queue @@ -37,7 +37,7 @@ func (q *Queues) Init() { } func (q *Queues) initHTTPRequest() { - q.HTTPRequest.Init(structr.QueueConfig[*HTTPRequest]{ + q.APRequests.Init(structr.QueueConfig[*APRequest]{ Indices: []structr.IndexConfig{ {Fields: "ObjectID", Multiple: true}, {Fields: "Request.URL.Host", Multiple: true}, diff --git a/internal/queue/wrappers.go b/internal/queue/wrappers.go index 087ae1f39d..6995d8768a 100644 --- a/internal/queue/wrappers.go +++ b/internal/queue/wrappers.go @@ -72,10 +72,10 @@ func (q *StructQueue[T]) Push(values ...T) { q.broadcast() } -// MoveBack ... -func (q *StructQueue[T]) MoveBack(index string, key ...any) { +// Delete removes all queued entries under index with key. +func (q *StructQueue[T]) Delete(index string, key ...any) { i := q.index[index] - q.queue.MoveBack(i, i.Key(key...)) + q.queue.Pop(i, i.Key(key...)) } // Len: see structr.Queue{}.Len(). diff --git a/internal/transport/deliver.go b/internal/transport/deliver.go index ab44ecca54..2e678c9c07 100644 --- a/internal/transport/deliver.go +++ b/internal/transport/deliver.go @@ -34,7 +34,7 @@ import ( func (t *transport) BatchDeliver(ctx context.Context, obj map[string]interface{}, recipients []*url.URL) error { var ( // accumulated prepared reqs. - reqs []*queue.HTTPRequest + reqs []*queue.APRequest // accumulated preparation errs. errs gtserror.MultiError @@ -50,8 +50,9 @@ func (t *transport) BatchDeliver(ctx context.Context, obj map[string]interface{} return gtserror.Newf("error marshaling json: %w", err) } - // Extract object ID. - id := getObjectID(obj) + // Extract object IDs. + objID := getObjectID(obj) + actID := getActorID(obj) for _, to := range recipients { // Skip delivery to recipient if it is "us". @@ -59,8 +60,8 @@ func (t *transport) BatchDeliver(ctx context.Context, obj map[string]interface{} continue } - // Prepare new http client request. - req, err := t.prepare(ctx, id, b, to) + // Prepare new outgoing http client request. + req, err := t.prepare(ctx, objID, actID, b, to) if err != nil { errs.Append(err) continue @@ -71,7 +72,7 @@ func (t *transport) BatchDeliver(ctx context.Context, obj map[string]interface{} } // Push the request list to HTTP client worker queue. - t.controller.state.Queues.HTTPRequest.Push(reqs...) + t.controller.state.Queues.APRequests.Push(reqs...) // Return combined err. return errs.Combine() @@ -89,17 +90,19 @@ func (t *transport) Deliver(ctx context.Context, obj map[string]interface{}, to return gtserror.Newf("error marshaling json: %w", err) } - // Extract object ID. - id := getObjectID(obj) - - // Prepare new http client request. - req, err := t.prepare(ctx, id, b, to) + // Prepare http client request. + req, err := t.prepare(ctx, + getObjectID(obj), + getActorID(obj), + b, + to, + ) if err != nil { return err } // Push the request to HTTP client worker queue. - t.controller.state.Queues.HTTPRequest.Push(req) + t.controller.state.Queues.APRequests.Push(req) return nil } @@ -110,10 +113,11 @@ func (t *transport) Deliver(ctx context.Context, obj map[string]interface{}, to func (t *transport) prepare( ctx context.Context, objectID string, + actorID string, data []byte, to *url.URL, ) ( - *queue.HTTPRequest, + *queue.APRequest, error, ) { url := to.String() @@ -137,7 +141,7 @@ func (t *transport) prepare( req.Header.Add("Content-Type", string(apiutil.AppActivityLDJSON)) req.Header.Add("Accept-Charset", "utf-8") - return &queue.HTTPRequest{ + return &queue.APRequest{ ObjectID: objectID, Request: req, }, nil @@ -155,3 +159,16 @@ func getObjectID(obj map[string]interface{}) string { return "" } } + +// getActorID extracts an actor ID from 'serialized' ActivityPub object map. +func getActorID(obj map[string]interface{}) string { + switch t := obj["actor"].(type) { + case string: + return t + case map[string]interface{}: + id, _ := t["id"].(string) + return id + default: + return "" + } +} diff --git a/internal/workers/workers.go b/internal/workers/workers.go index 574e6f8150..b6906857ed 100644 --- a/internal/workers/workers.go +++ b/internal/workers/workers.go @@ -25,7 +25,6 @@ import ( "codeberg.org/gruf/go-runners" "github.com/superseriousbusiness/gotosocial/internal/httpclient" "github.com/superseriousbusiness/gotosocial/internal/messages" - "github.com/superseriousbusiness/gotosocial/internal/queue" "github.com/superseriousbusiness/gotosocial/internal/scheduler" ) @@ -33,8 +32,8 @@ type Workers struct { // Main task scheduler instance. Scheduler scheduler.Scheduler - // HTTPClient ... - HTTPClient httpclient.DeliveryWorkerPool + // APDelivery ... + APDelivery httpclient.APDeliveryWorkerPool // ClientAPI provides a worker pool that handles both // incoming client actions, and our own side-effects. @@ -47,9 +46,8 @@ type Workers struct { // Enqueue functions for clientAPI / federator worker pools, // these are pointers to Processor{}.Enqueue___() msg functions. // This prevents dependency cycling as Processor depends on Workers. - EnqueueHTTPClient func(context.Context, ...queue.HTTPRequest) - EnqueueClientAPI func(context.Context, ...messages.FromClientAPI) - EnqueueFediAPI func(context.Context, ...messages.FromFediAPI) + EnqueueClientAPI func(context.Context, ...messages.FromClientAPI) + EnqueueFediAPI func(context.Context, ...messages.FromFediAPI) // Blocking processing functions for clientAPI / federator. // These are pointers to Processor{}.Process___() msg functions. @@ -78,7 +76,7 @@ func (w *Workers) Start() { tryUntil("starting scheduler", 5, w.Scheduler.Start) - tryUntil("start http client workerpool", 5, w.HTTPClient.Start) + tryUntil("start ap delivery workerpool", 5, w.APDelivery.Start) tryUntil("starting client API workerpool", 5, func() bool { return w.ClientAPI.Start(4*maxprocs, 400*maxprocs) @@ -96,7 +94,7 @@ func (w *Workers) Start() { // Stop will stop all of the contained worker pools (and global scheduler). func (w *Workers) Stop() { tryUntil("stopping scheduler", 5, w.Scheduler.Stop) - tryUntil("stopping http client workerpool", 5, w.HTTPClient.Stop) + tryUntil("stopping ap delivery workerpool", 5, w.APDelivery.Stop) tryUntil("stopping client API workerpool", 5, w.ClientAPI.Stop) tryUntil("stopping federator workerpool", 5, w.Federator.Stop) tryUntil("stopping media workerpool", 5, w.Media.Stop) From d7e14d5dbbe011c2db6d29ab847f9b0e5b55801a Mon Sep 17 00:00:00 2001 From: kim Date: Wed, 3 Apr 2024 16:58:59 +0100 Subject: [PATCH 04/35] support deleting queued http requests by target ID --- internal/processing/workers/federate.go | 2 ++ internal/queue/messages.go | 3 +++ internal/queue/queues.go | 2 ++ internal/transport/deliver.go | 34 +++++++++++++++++++++---- 4 files changed, 36 insertions(+), 5 deletions(-) diff --git a/internal/processing/workers/federate.go b/internal/processing/workers/federate.go index 0ec23d3c07..581a61af44 100644 --- a/internal/processing/workers/federate.go +++ b/internal/processing/workers/federate.go @@ -79,6 +79,7 @@ func (f *federate) DeleteAccount(ctx context.Context, account *gtsmodel.Account) // (this stops any queued likes, boosts, creates etc). f.state.Queues.APRequests.Delete("ActorID", account.URI) f.state.Queues.APRequests.Delete("ObjectID", account.URI) + f.state.Queues.APRequests.Delete("TargetID", account.URI) // Parse relevant URI(s). outboxIRI, err := parseURI(account.OutboxURI) @@ -230,6 +231,7 @@ func (f *federate) DeleteStatus(ctx context.Context, status *gtsmodel.Status) er // Drop any queued outgoing http requests for status, // (this stops any queued likes, boosts, creates etc). f.state.Queues.APRequests.Delete("ObjectID", status.URI) + f.state.Queues.APRequests.Delete("TargetID", status.URI) // Ensure the status model is fully populated. if err := f.state.DB.PopulateStatus(ctx, status); err != nil { diff --git a/internal/queue/messages.go b/internal/queue/messages.go index 86601ca6ea..05f423102f 100644 --- a/internal/queue/messages.go +++ b/internal/queue/messages.go @@ -63,6 +63,9 @@ type APRequest struct { // ObjectID ... ObjectID string + // TargetID ... + TargetID string + // Request ... Request *http.Request } diff --git a/internal/queue/queues.go b/internal/queue/queues.go index ead24cedab..a601d7d056 100644 --- a/internal/queue/queues.go +++ b/internal/queue/queues.go @@ -39,7 +39,9 @@ func (q *Queues) Init() { func (q *Queues) initHTTPRequest() { q.APRequests.Init(structr.QueueConfig[*APRequest]{ Indices: []structr.IndexConfig{ + {Fields: "ActorID", Multiple: true}, {Fields: "ObjectID", Multiple: true}, + {Fields: "TargetID", Multiple: true}, {Fields: "Request.URL.Host", Multiple: true}, }, }) diff --git a/internal/transport/deliver.go b/internal/transport/deliver.go index 2e678c9c07..b8654ec58f 100644 --- a/internal/transport/deliver.go +++ b/internal/transport/deliver.go @@ -51,8 +51,9 @@ func (t *transport) BatchDeliver(ctx context.Context, obj map[string]interface{} } // Extract object IDs. - objID := getObjectID(obj) actID := getActorID(obj) + objID := getObjectID(obj) + tgtID := getTargetID(obj) for _, to := range recipients { // Skip delivery to recipient if it is "us". @@ -60,8 +61,14 @@ func (t *transport) BatchDeliver(ctx context.Context, obj map[string]interface{} continue } - // Prepare new outgoing http client request. - req, err := t.prepare(ctx, objID, actID, b, to) + // Prepare http client request. + req, err := t.prepare(ctx, + actID, + objID, + tgtID, + b, + to, + ) if err != nil { errs.Append(err) continue @@ -92,8 +99,9 @@ func (t *transport) Deliver(ctx context.Context, obj map[string]interface{}, to // Prepare http client request. req, err := t.prepare(ctx, - getObjectID(obj), getActorID(obj), + getObjectID(obj), + getTargetID(obj), b, to, ) @@ -112,8 +120,9 @@ func (t *transport) Deliver(ctx context.Context, obj map[string]interface{}, to // request object with signing function. func (t *transport) prepare( ctx context.Context, - objectID string, actorID string, + objectID string, + targetID string, data []byte, to *url.URL, ) ( @@ -142,7 +151,9 @@ func (t *transport) prepare( req.Header.Add("Accept-Charset", "utf-8") return &queue.APRequest{ + ActorID: actorID, ObjectID: objectID, + TargetID: targetID, Request: req, }, nil } @@ -172,3 +183,16 @@ func getActorID(obj map[string]interface{}) string { return "" } } + +// getTargetID extracts a target ID from 'serialized' ActivityPub object map. +func getTargetID(obj map[string]interface{}) string { + switch t := obj["target"].(type) { + case string: + return t + case map[string]interface{}: + id, _ := t["id"].(string) + return id + default: + return "" + } +} From 033b2e7d6064d476e67bcb1d38a38a44756baee3 Mon Sep 17 00:00:00 2001 From: kim Date: Wed, 3 Apr 2024 17:05:16 +0100 Subject: [PATCH 05/35] don't index APRequest by hostname in the queue --- internal/queue/queues.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/queue/queues.go b/internal/queue/queues.go index a601d7d056..d8186f1a68 100644 --- a/internal/queue/queues.go +++ b/internal/queue/queues.go @@ -42,7 +42,6 @@ func (q *Queues) initHTTPRequest() { {Fields: "ActorID", Multiple: true}, {Fields: "ObjectID", Multiple: true}, {Fields: "TargetID", Multiple: true}, - {Fields: "Request.URL.Host", Multiple: true}, }, }) } From 73ef63689d07e7b09e665a7f878ab842b159d7c3 Mon Sep 17 00:00:00 2001 From: kim Date: Wed, 3 Apr 2024 17:11:31 +0100 Subject: [PATCH 06/35] use gorun --- internal/httpclient/delivery.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/httpclient/delivery.go b/internal/httpclient/delivery.go index ae00f94a1a..a52838f8de 100644 --- a/internal/httpclient/delivery.go +++ b/internal/httpclient/delivery.go @@ -94,7 +94,7 @@ func NewAPDeliveryWorker(client *Client, queue *queue.StructQueue[*queue.APReque // Start will attempt to start the DeliveryWorker{}. func (w *APDeliveryWorker) Start() bool { - return w.service.Run(w.process) + return w.service.GoRun(w.process) } // Stop will attempt to stop the DeliveryWorker{}. From 3439ce7012f541e73868066aba52de5c7158b894 Mon Sep 17 00:00:00 2001 From: kim Date: Wed, 3 Apr 2024 17:15:13 +0100 Subject: [PATCH 07/35] use the original context's values when wrapping msg type as delivery{} --- internal/httpclient/delivery.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/internal/httpclient/delivery.go b/internal/httpclient/delivery.go index a52838f8de..ca38e26fb4 100644 --- a/internal/httpclient/delivery.go +++ b/internal/httpclient/delivery.go @@ -23,6 +23,7 @@ import ( "time" "codeberg.org/gruf/go-runners" + "github.com/superseriousbusiness/gotosocial/internal/gtscontext" "github.com/superseriousbusiness/gotosocial/internal/log" "github.com/superseriousbusiness/gotosocial/internal/queue" ) @@ -243,6 +244,7 @@ func wrapMsg(ctx context.Context, msg *queue.APRequest) *delivery { dlv := new(delivery) dlv.request = wrapRequest(msg.Request) dlv.log = requestLog(dlv.req) + ctx = gtscontext.WithValues(ctx, msg.Request.Context()) dlv.req = dlv.req.WithContext(ctx) return dlv } From a8ea1fe3402b81b26e2c5d0bce6a12aaee70bf20 Mon Sep 17 00:00:00 2001 From: kim Date: Wed, 3 Apr 2024 17:30:17 +0100 Subject: [PATCH 08/35] actually log in the AP delivery worker ... --- internal/httpclient/delivery.go | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/internal/httpclient/delivery.go b/internal/httpclient/delivery.go index ca38e26fb4..f29810b03f 100644 --- a/internal/httpclient/delivery.go +++ b/internal/httpclient/delivery.go @@ -143,15 +143,23 @@ loop: } } + dlv.log.Info("performing request") + // Attempt outoing delivery of request. _, retry, err := w.client.do(&dlv.request) - if err == nil || !retry { + if err == nil { + continue loop + } + + dlv.log.Error(err) + + if !retry { continue loop } - if dlv.attempts > maxRetries { - // Drop deliveries once - // we reach max retries. + if !retry || dlv.attempts > maxRetries { + // Drop deliveries when no retry + // requested, or we reach max. continue loop } From 3c9451b32715c1d6d87975c708cc19962b453215 Mon Sep 17 00:00:00 2001 From: kim Date: Wed, 3 Apr 2024 17:31:12 +0100 Subject: [PATCH 09/35] add uncommitted changes --- internal/httpclient/delivery.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/internal/httpclient/delivery.go b/internal/httpclient/delivery.go index f29810b03f..99f78151f8 100644 --- a/internal/httpclient/delivery.go +++ b/internal/httpclient/delivery.go @@ -153,10 +153,6 @@ loop: dlv.log.Error(err) - if !retry { - continue loop - } - if !retry || dlv.attempts > maxRetries { // Drop deliveries when no retry // requested, or we reach max. From 175c8700d1cb99a1a2075b76bb06f4d7773d877d Mon Sep 17 00:00:00 2001 From: kim Date: Thu, 4 Apr 2024 11:02:16 +0100 Subject: [PATCH 10/35] use errors.AsV2() --- internal/httpclient/client.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/httpclient/client.go b/internal/httpclient/client.go index 01238eb223..65b050d90a 100644 --- a/internal/httpclient/client.go +++ b/internal/httpclient/client.go @@ -291,8 +291,8 @@ func (c *Client) do(r *request) (*http.Response, bool /* retry */, error) { return nil, false, err } - if dnserr := (*net.DNSError)(nil); // nocollapse - errors.As(err, &dnserr) && dnserr.IsNotFound { + if dnserr := errorsv2.AsV2[*net.DNSError](err); // nocollapse + dnserr != nil && dnserr.IsNotFound { // DNS lookup failure, this domain does not exist return nil, false, gtserror.SetNotFound(err) } From d8992392adab87f387d8bbfdb687e3975a364b70 Mon Sep 17 00:00:00 2001 From: kim Date: Thu, 4 Apr 2024 11:09:03 +0100 Subject: [PATCH 11/35] use errorsv2.AsV2() --- internal/api/activitypub/users/inboxpost.go | 11 ++++++----- internal/federation/federatingactor.go | 6 +++--- internal/federation/federatingprotocol_test.go | 6 +++--- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/internal/api/activitypub/users/inboxpost.go b/internal/api/activitypub/users/inboxpost.go index 03ba5c5a67..b0a9a49eef 100644 --- a/internal/api/activitypub/users/inboxpost.go +++ b/internal/api/activitypub/users/inboxpost.go @@ -18,13 +18,14 @@ package users import ( - "errors" "net/http" "github.com/gin-gonic/gin" apiutil "github.com/superseriousbusiness/gotosocial/internal/api/util" "github.com/superseriousbusiness/gotosocial/internal/gtserror" "github.com/superseriousbusiness/gotosocial/internal/log" + + errorsv2 "codeberg.org/gruf/go-errors/v2" ) // InboxPOSTHandler deals with incoming POST requests to an actor's inbox. @@ -32,18 +33,18 @@ import ( func (m *Module) InboxPOSTHandler(c *gin.Context) { _, err := m.processor.Fedi().InboxPost(c.Request.Context(), c.Writer, c.Request) if err != nil { - errWithCode := new(gtserror.WithCode) + errWithCode := errorsv2.AsV2[gtserror.WithCode](err) - if !errors.As(err, errWithCode) { + if errWithCode == nil { // Something else went wrong, and someone forgot to return // an errWithCode! It's chill though. Log the error but don't // return it as-is to the caller, to avoid leaking internals. log.Errorf(c.Request.Context(), "returning Bad Request to caller, err was: %q", err) - *errWithCode = gtserror.NewErrorBadRequest(err) + errWithCode = gtserror.NewErrorBadRequest(err) } // Pass along confirmed error with code to the main error handler - apiutil.ErrorHandler(c, *errWithCode, m.processor.InstanceGetV1) + apiutil.ErrorHandler(c, errWithCode, m.processor.InstanceGetV1) return } diff --git a/internal/federation/federatingactor.go b/internal/federation/federatingactor.go index 18cee16662..b9b2c80014 100644 --- a/internal/federation/federatingactor.go +++ b/internal/federation/federatingactor.go @@ -89,7 +89,7 @@ func (f *federatingActor) PostInboxScheme(ctx context.Context, w http.ResponseWr // so we specifically have to check for already wrapped with code. // ctx, authenticated, err := f.sideEffectActor.AuthenticatePostInbox(ctx, w, r) - if errors.As(err, new(gtserror.WithCode)) { + if errorsv2.AsV2[gtserror.WithCode](err) != nil { // If it was already wrapped with an // HTTP code then don't bother rewrapping // it, just return it as-is for caller to @@ -131,7 +131,7 @@ func (f *federatingActor) PostInboxScheme(ctx context.Context, w http.ResponseWr // Check authorization of the activity; this will include blocks. authorized, err := f.sideEffectActor.AuthorizePostInbox(ctx, w, activity) if err != nil { - if errors.As(err, new(errOtherIRIBlocked)) { + if errorsv2.AsV2[*errOtherIRIBlocked](err) != nil { // There's no direct block between requester(s) and // receiver. However, one or more of the other IRIs // involved in the request (account replied to, note @@ -139,7 +139,7 @@ func (f *federatingActor) PostInboxScheme(ctx context.Context, w http.ResponseWr // by the receiver. We don't need to return 403 here, // instead, just return 202 accepted but don't do any // further processing of the activity. - return true, nil + return true, nil //nolint } // Real error has occurred. diff --git a/internal/federation/federatingprotocol_test.go b/internal/federation/federatingprotocol_test.go index f975cd7d66..085d6c4742 100644 --- a/internal/federation/federatingprotocol_test.go +++ b/internal/federation/federatingprotocol_test.go @@ -21,13 +21,13 @@ import ( "bytes" "context" "encoding/json" - "errors" "io" "net/http" "net/http/httptest" "net/url" "testing" + errorsv2 "codeberg.org/gruf/go-errors/v2" "github.com/stretchr/testify/suite" "github.com/superseriousbusiness/gotosocial/internal/ap" "github.com/superseriousbusiness/gotosocial/internal/gtscontext" @@ -101,8 +101,8 @@ func (suite *FederatingProtocolTestSuite) authenticatePostInbox( recorder := httptest.NewRecorder() newContext, authed, err := suite.federator.AuthenticatePostInbox(ctx, recorder, request) - if withCode := new(gtserror.WithCode); (errors.As(err, withCode) && - (*withCode).Code() >= 500) || (err != nil && (*withCode) == nil) { + if withCode := errorsv2.AsV2[gtserror.WithCode](err); // nocollapse + (withCode != nil && withCode.Code() >= 500) || (err != nil && withCode == nil) { // NOTE: the behaviour here is a little strange as we have // the competing code styles of the go-fed interface expecting // that any err is a no-go, but authed bool is intended to be From f70ef72f7b6b49c41a0c3fbc6b008e12e00ea154 Mon Sep 17 00:00:00 2001 From: kim Date: Thu, 4 Apr 2024 11:40:34 +0100 Subject: [PATCH 12/35] finish adding some code comments, add bad host handling to delivery workers --- internal/httpclient/client.go | 6 +++++- internal/httpclient/delivery.go | 21 ++++++++++++++++----- internal/httpclient/request.go | 2 +- 3 files changed, 22 insertions(+), 7 deletions(-) diff --git a/internal/httpclient/client.go b/internal/httpclient/client.go index 65b050d90a..7f7727ae8d 100644 --- a/internal/httpclient/client.go +++ b/internal/httpclient/client.go @@ -108,12 +108,16 @@ type Client struct { client http.Client badHosts cache.TTLCache[string, struct{}] bodyMax int64 + retries uint } // New returns a new instance of Client initialized using configuration. func New(cfg Config) *Client { var c Client + // For now use const. + c.retries = maxRetries + d := &net.Dialer{ Timeout: 15 * time.Second, KeepAlive: 30 * time.Second, @@ -220,7 +224,7 @@ func (c *Client) Do(r *http.Request) (rsp *http.Response, err error) { // type for retry-backoff. req := wrapRequest(r) - for req.attempts < maxRetries { + for req.attempts < c.retries { var retry bool log.Info("performing request") diff --git a/internal/httpclient/delivery.go b/internal/httpclient/delivery.go index 99f78151f8..beeb9f4766 100644 --- a/internal/httpclient/delivery.go +++ b/internal/httpclient/delivery.go @@ -28,7 +28,8 @@ import ( "github.com/superseriousbusiness/gotosocial/internal/queue" ) -// APDeliveryWorkerPool ... +// APDeliveryWorkerPool wraps APDeliveryWorker{}s +// in a singular struct for easy multi start/stop. type APDeliveryWorkerPool struct { workers []APDeliveryWorker } @@ -76,7 +77,10 @@ func (p *APDeliveryWorkerPool) Stop() bool { return ok } -// APDeliveryWorker ... +// APDeliveryWorker wraps a Client{} to feed from +// a queue.StructQueue{} for ActivityPub requests +// to deliver. It does so while prioritizing new +// queued requests over backlogged retries. type APDeliveryWorker struct { client *Client queue *queue.StructQueue[*queue.APRequest] @@ -153,9 +157,11 @@ loop: dlv.log.Error(err) - if !retry || dlv.attempts > maxRetries { - // Drop deliveries when no retry - // requested, or we reach max. + if !retry || w.client.badHosts.Has(dlv.host) || + dlv.attempts > w.client.retries { + // Drop deliveries when no retry requested, + // or we reach max defined retry attempts. + w.client.badHosts.Set(dlv.host, struct{}{}) continue loop } @@ -229,6 +235,10 @@ type delivery struct { // next attempt time. next time.Time + // hostname string + // for bad host check. + host string + // embedded // request. request @@ -248,6 +258,7 @@ func wrapMsg(ctx context.Context, msg *queue.APRequest) *delivery { dlv := new(delivery) dlv.request = wrapRequest(msg.Request) dlv.log = requestLog(dlv.req) + dlv.host = dlv.req.URL.Hostname() ctx = gtscontext.WithValues(ctx, msg.Request.Context()) dlv.req = dlv.req.WithContext(ctx) return dlv diff --git a/internal/httpclient/request.go b/internal/httpclient/request.go index 8fdbb729f2..256d97943c 100644 --- a/internal/httpclient/request.go +++ b/internal/httpclient/request.go @@ -43,7 +43,7 @@ type request struct { backoff time.Duration // delivery attempts. - attempts int + attempts uint } // wrapRequest wraps an http.Request{} in our own request{} type. From 7080930371630e5ac25dbb3cdf2031c5525828c9 Mon Sep 17 00:00:00 2001 From: kim Date: Thu, 4 Apr 2024 12:09:03 +0100 Subject: [PATCH 13/35] slightly tweak deliveryworkerpool API, use advanced sender multiplier --- cmd/gotosocial/action/server/server.go | 4 +- internal/httpclient/client.go | 11 +++-- internal/httpclient/delivery.go | 56 +++++++++++++------------- internal/httpclient/request.go | 3 -- internal/workers/workers.go | 6 ++- 5 files changed, 40 insertions(+), 40 deletions(-) diff --git a/cmd/gotosocial/action/server/server.go b/cmd/gotosocial/action/server/server.go index 6f486b71a8..e0b7e0113e 100644 --- a/cmd/gotosocial/action/server/server.go +++ b/cmd/gotosocial/action/server/server.go @@ -24,7 +24,6 @@ import ( "net/http" "os" "os/signal" - "runtime" "syscall" "time" @@ -127,8 +126,7 @@ var Start action.GTSAction = func(ctx context.Context) error { // Initialize the queues. state.Queues.Init() - - state.Workers.APDelivery.Init(client, &state.Queues.APRequests, runtime.GOMAXPROCS(0)) + state.Workers.APDelivery.Init(client, &state.Queues.APRequests) // Initialize workers. state.Workers.Start() diff --git a/internal/httpclient/client.go b/internal/httpclient/client.go index 7f7727ae8d..afe78b8974 100644 --- a/internal/httpclient/client.go +++ b/internal/httpclient/client.go @@ -114,9 +114,7 @@ type Client struct { // New returns a new instance of Client initialized using configuration. func New(cfg Config) *Client { var c Client - - // For now use const. - c.retries = maxRetries + c.retries = 5 d := &net.Dialer{ Timeout: 15 * time.Second, @@ -285,7 +283,7 @@ func (c *Client) do(r *request) (*http.Response, bool /* retry */, error) { return nil, false, err } - if errstr := err.Error(); // nocollapse + if errstr := err.Error(); // strings.Contains(errstr, "stopped after 10 redirects") || strings.Contains(errstr, "tls: ") || strings.Contains(errstr, "x509: ") { @@ -295,7 +293,7 @@ func (c *Client) do(r *request) (*http.Response, bool /* retry */, error) { return nil, false, err } - if dnserr := errorsv2.AsV2[*net.DNSError](err); // nocollapse + if dnserr := errorsv2.AsV2[*net.DNSError](err); // dnserr != nil && dnserr.IsNotFound { // DNS lookup failure, this domain does not exist return nil, false, gtserror.SetNotFound(err) @@ -326,7 +324,8 @@ func (c *Client) do(r *request) (*http.Response, bool /* retry */, error) { } // Don't let their provided backoff exceed our max. - if max := baseBackoff * maxRetries; r.backoff > max { + if max := baseBackoff * time.Duration(c.retries); // + r.backoff > max { r.backoff = max } } diff --git a/internal/httpclient/delivery.go b/internal/httpclient/delivery.go index beeb9f4766..4345da55e5 100644 --- a/internal/httpclient/delivery.go +++ b/internal/httpclient/delivery.go @@ -20,6 +20,7 @@ package httpclient import ( "context" "slices" + "sync" "time" "codeberg.org/gruf/go-runners" @@ -31,7 +32,10 @@ import ( // APDeliveryWorkerPool wraps APDeliveryWorker{}s // in a singular struct for easy multi start/stop. type APDeliveryWorkerPool struct { + client *Client + queue *queue.StructQueue[*queue.APRequest] workers []APDeliveryWorker + mutex sync.Mutex } // Init will initialize the DeliveryWorker{} pool @@ -40,41 +44,39 @@ type APDeliveryWorkerPool struct { func (p *APDeliveryWorkerPool) Init( client *Client, queue *queue.StructQueue[*queue.APRequest], - workers int, ) { - p.workers = make([]APDeliveryWorker, workers) - for i := range p.workers { - p.workers[i] = NewAPDeliveryWorker( - client, - queue, - ) - } + p.mutex.Lock() + p.client = client + p.queue = queue + p.mutex.Unlock() } -// Start will attempt to start all of the contained DeliveryWorker{}s. -// NOTE: this is not safe to call concurrently with .Init(). -func (p *APDeliveryWorkerPool) Start() bool { - if len(p.workers) == 0 { - return false - } - ok := true - for i := range p.workers { - ok = p.workers[i].Start() && ok +// Start will attempt to start 'n' DeliveryWorker{}s. +func (p *APDeliveryWorkerPool) Start(n int) (ok bool) { + p.mutex.Lock() + if ok = (len(p.workers) == 0); ok { + p.workers = make([]APDeliveryWorker, n) + for i := range p.workers { + p.workers[i].client = p.client + p.workers[i].queue = p.queue + ok = p.workers[i].Start() && ok + } } - return ok + p.mutex.Unlock() + return } // Stop will attempt to stop all of the contained DeliveryWorker{}s. -// NOTE: this is not safe to call concurrently with .Init(). -func (p *APDeliveryWorkerPool) Stop() bool { - if len(p.workers) == 0 { - return false - } - ok := true - for i := range p.workers { - ok = p.workers[i].Stop() && ok +func (p *APDeliveryWorkerPool) Stop() (ok bool) { + p.mutex.Lock() + if ok = (len(p.workers) > 0); ok { + for i := range p.workers { + ok = p.workers[i].Stop() && ok + } + p.workers = p.workers[:0] } - return ok + p.mutex.Unlock() + return } // APDeliveryWorker wraps a Client{} to feed from diff --git a/internal/httpclient/request.go b/internal/httpclient/request.go index 256d97943c..1c3d8d0d38 100644 --- a/internal/httpclient/request.go +++ b/internal/httpclient/request.go @@ -25,9 +25,6 @@ import ( ) const ( - // max no. attempts. - maxRetries = 5 - // starting backoff duration. baseBackoff = 2 * time.Second ) diff --git a/internal/workers/workers.go b/internal/workers/workers.go index b6906857ed..e94e99f796 100644 --- a/internal/workers/workers.go +++ b/internal/workers/workers.go @@ -23,6 +23,7 @@ import ( "runtime" "codeberg.org/gruf/go-runners" + "github.com/superseriousbusiness/gotosocial/internal/config" "github.com/superseriousbusiness/gotosocial/internal/httpclient" "github.com/superseriousbusiness/gotosocial/internal/messages" "github.com/superseriousbusiness/gotosocial/internal/scheduler" @@ -76,7 +77,10 @@ func (w *Workers) Start() { tryUntil("starting scheduler", 5, w.Scheduler.Start) - tryUntil("start ap delivery workerpool", 5, w.APDelivery.Start) + tryUntil("start ap delivery workerpool", 5, func() bool { + n := config.GetAdvancedSenderMultiplier() + return w.APDelivery.Start(n * maxprocs) + }) tryUntil("starting client API workerpool", 5, func() bool { return w.ClientAPI.Start(4*maxprocs, 400*maxprocs) From ba60397e3099303da940f070c7afde5f408eceeb Mon Sep 17 00:00:00 2001 From: kim Date: Thu, 4 Apr 2024 16:17:05 +0100 Subject: [PATCH 14/35] remove PopCtx() method, let others instead rely on Wait() --- internal/httpclient/delivery.go | 46 +++++++++++++++++++-------------- internal/queue/wrappers.go | 33 +++++------------------ 2 files changed, 34 insertions(+), 45 deletions(-) diff --git a/internal/httpclient/delivery.go b/internal/httpclient/delivery.go index 4345da55e5..0ed620b6ce 100644 --- a/internal/httpclient/delivery.go +++ b/internal/httpclient/delivery.go @@ -163,6 +163,7 @@ loop: dlv.attempts > w.client.retries { // Drop deliveries when no retry requested, // or we reach max defined retry attempts. + // "bad" hosts support a max of 1 attempt. w.client.badHosts.Set(dlv.host, struct{}{}) continue loop } @@ -177,32 +178,39 @@ loop: // next gets the next available delivery, blocking until available if necessary. func (w *APDeliveryWorker) next(ctx context.Context) (*delivery, bool) { - // Try pop next queued. - msg, ok := w.queue.Pop() +loop: + for { + // Try pop next queued. + msg, ok := w.queue.Pop() - if !ok { - // Check the backlog. - if len(w.backlog) > 0 { + if !ok { + // Check the backlog. + if len(w.backlog) > 0 { - // Sort by 'next' time. - sortDeliveries(w.backlog) + // Sort by 'next' time. + sortDeliveries(w.backlog) - // Pop next delivery. - dlv := w.popBacklog() + // Pop next delivery. + dlv := w.popBacklog() - return dlv, true - } + return dlv, true + } - // Backlog is empty, we MUST - // block until next enqueued. - msg, ok = w.queue.PopCtx(ctx) - if !ok { - return nil, false + select { + // Backlog is empty, we MUST + // block until next enqueued. + case <-w.queue.Wait(): + continue loop + + // Worker was stopped. + case <-ctx.Done(): + return nil, false + } } - } - // Wrap msg in delivery type. - return wrapMsg(ctx, msg), true + // Wrap msg in delivery type. + return wrapMsg(ctx, msg), true + } } // popBacklog pops next available from the backlog. diff --git a/internal/queue/wrappers.go b/internal/queue/wrappers.go index 6995d8768a..e07984f84e 100644 --- a/internal/queue/wrappers.go +++ b/internal/queue/wrappers.go @@ -18,13 +18,13 @@ package queue import ( - "context" "sync/atomic" "codeberg.org/gruf/go-structr" ) -// StructQueue ... +// StructQueue wraps a structr.Queue{} to +// provide simple index caching by name. type StructQueue[StructType any] struct { queue structr.Queue[StructType] index map[string]*structr.Index @@ -46,36 +46,16 @@ func (q *StructQueue[T]) Pop() (value T, ok bool) { return q.queue.PopFront() } -// PopCtx wraps structr.Queue{}.PopFront() to add sleep until value is available. -func (q *StructQueue[T]) PopCtx(ctx context.Context) (value T, ok bool) { - for { - // Try pop from front of queue. - value, ok = q.queue.PopFront() - if ok { - return - } - - select { - // Context canceled. - case <-ctx.Done(): - return - - // Waiter released. - case <-q.Wait(): - } - } -} - -// Push wraps structr.Queue{}.PushBack() to add sleeping pop goroutine awakening. +// Push wraps structr.Queue{}.PushBack() to awaken those blocking on <-.Wait(). func (q *StructQueue[T]) Push(values ...T) { q.queue.PushBack(values...) q.broadcast() } -// Delete removes all queued entries under index with key. +// Delete pops (and drops!) all queued entries under index with key. func (q *StructQueue[T]) Delete(index string, key ...any) { i := q.index[index] - q.queue.Pop(i, i.Key(key...)) + _ = q.queue.Pop(i, i.Key(key...)) } // Len: see structr.Queue{}.Len(). @@ -83,7 +63,8 @@ func (q *StructQueue[T]) Len() int { return q.queue.Len() } -// Wait safely returns current (read-only) wait channel. +// Wait returns current wait channel, which may be +// blocked on to awaken when new value pushed to queue. func (q *StructQueue[T]) Wait() <-chan struct{} { var ch chan struct{} From a3bd01ddb89bd55370d62f98aa425266f6337b7a Mon Sep 17 00:00:00 2001 From: kim Date: Fri, 5 Apr 2024 14:40:18 +0100 Subject: [PATCH 15/35] shuffle things around to move delivery stuff into transport/ subpkg --- cmd/gotosocial/action/server/server.go | 5 +- internal/httpclient/client.go | 133 +++++++---- internal/httpclient/delivery.go | 291 ------------------------ internal/httpclient/request.go | 45 ++-- internal/processing/workers/federate.go | 10 +- internal/queue/messages.go | 71 ------ internal/queue/queues.go | 47 ---- internal/state/state.go | 4 - internal/transport/controller.go | 16 +- internal/transport/deliver.go | 27 +-- internal/transport/delivery/delivery.go | 281 +++++++++++++++++++++++ internal/workers/workers.go | 13 +- 12 files changed, 420 insertions(+), 523 deletions(-) delete mode 100644 internal/httpclient/delivery.go delete mode 100644 internal/queue/messages.go delete mode 100644 internal/queue/queues.go create mode 100644 internal/transport/delivery/delivery.go diff --git a/cmd/gotosocial/action/server/server.go b/cmd/gotosocial/action/server/server.go index e0b7e0113e..fab88fe212 100644 --- a/cmd/gotosocial/action/server/server.go +++ b/cmd/gotosocial/action/server/server.go @@ -124,9 +124,8 @@ var Start action.GTSAction = func(ctx context.Context) error { TLSInsecureSkipVerify: config.GetHTTPClientTLSInsecureSkipVerify(), }) - // Initialize the queues. - state.Queues.Init() - state.Workers.APDelivery.Init(client, &state.Queues.APRequests) + // Initialize delivery worker with http client. + state.Workers.Delivery.Init(client) // Initialize workers. state.Workers.Start() diff --git a/internal/httpclient/client.go b/internal/httpclient/client.go index afe78b8974..b37ea2d40d 100644 --- a/internal/httpclient/client.go +++ b/internal/httpclient/client.go @@ -195,82 +195,114 @@ func (c *Client) Do(r *http.Request) (rsp *http.Response, err error) { return nil, err } - // Get request hostname. - host := r.URL.Hostname() - - // Check whether request should fast fail. - fastFail := gtscontext.IsFastfail(r.Context()) - if !fastFail { - // Check if recently reached max retries for this host - // so we don't bother with a retry-backoff loop. The only - // errors that are retried upon are server failure, TLS - // and domain resolution type errors, so this cached result - // indicates this server is likely having issues. - fastFail = c.badHosts.Has(host) - defer func() { - if err != nil { - // On error mark as a bad-host. - c.badHosts.Set(host, struct{}{}) - } - }() - } - - // Prepare log entry. - log := requestLog(r) - // Wrap in our own request // type for retry-backoff. - req := wrapRequest(r) + req := WrapRequest(r) + + if gtscontext.IsFastfail(r.Context()) { + // If the fast-fail flag was set, just + // attempt a single iteration instead of + // following the below retry-backoff loop. + rsp, _, err = c.DoOnce(&req) + if err != nil { + return nil, fmt.Errorf("%w (fast fail)", err) + } + return rsp, nil + } - for req.attempts < c.retries { + for { var retry bool - log.Info("performing request") - // Perform the http request. - rsp, retry, err = c.do(&req) - if err == nil || !retry { - return + rsp, retry, err = c.DoOnce(&req) + if err == nil { + return rsp, nil } - log.Error(err) - - if fastFail { - // on fast-fail, don't bother backoff/retry - return nil, fmt.Errorf("%w (fast fail)", err) + if !retry { + // reached max retries, don't further backoff + return nil, fmt.Errorf("%w (max retries)", err) } - // Start the backoff timer channel. - backoff, cncl := sleepch(req.BackOff()) + // Start new backoff sleep timer. + backoff := time.NewTimer(req.BackOff()) select { - // Request ctx cancelled + // Request ctx cancelled. case <-r.Context().Done(): - cncl() + backoff.Stop() + + // Return context error. + err = r.Context().Err() + return nil, err - // Backoff for a time - case <-backoff: + // Backoff for time. + case <-backoff.C: } } - - // Set error return to trigger setting "bad host". - err = errors.New("transport reached max retries") - return } -// do wraps an underlying http.Client{}.Do() to perform our wrapped request type: +// DoOnce wraps an underlying http.Client{}.Do() to perform our wrapped request type: // rewinding response body to permit reuse, signing request data when SignFunc provided, -// safely limiting response body, updating retry attempt counts and setting retry-after. -func (c *Client) do(r *request) (*http.Response, bool /* retry */, error) { - // Update the +// marking erroring hosts, updating retry attempt counts and setting backoff from header. +func (c *Client) DoOnce(r *Request) (rsp *http.Response, retry bool, err error) { + if r.attempts > c.retries { + // Ensure request hasn't reached max number of attempts. + err = fmt.Errorf("httpclient: reached max retries (%d)", c.retries) + return + } + + // Update no. // attempts. r.attempts++ // Reset backoff. r.backoff = 0 + // Perform main routine. + rsp, retry, err = c.do(r) + + if rsp != nil { + // Log successful rsp. + r.log.Info(rsp.Status) + return + } + + // Log any errors. + r.log.Error(err) + + switch { + case !retry: + // If they were told not to + // retry, also set number of + // attempts to prevent retry. + r.attempts = c.retries + 1 + + case r.attempts > c.retries: + // On max retries, mark this as + // a "badhost", i.e. is erroring. + c.badHosts.Set(r.Host, struct{}{}) + + // Ensure retry flag is unset + // when reached max attempts. + retry = false + + case c.badHosts.Has(r.Host): + // When retry is still permitted, + // check host hasn't been marked + // as a "badhost", i.e. erroring. + r.attempts = c.retries + 1 + retry = false + } + + return +} + +// do performs the "meat" of DoOnce(), but it's separated out to allow +// easier wrapping of the response, retry, error returns with further logic. +func (c *Client) do(r *Request) (rsp *http.Response, retry bool, err error) { // Perform the HTTP request. - rsp, err := c.client.Do(r.req) + rsp, err = c.client.Do(r.Request) if err != nil { if errorsv2.IsV2(err, @@ -299,6 +331,7 @@ func (c *Client) do(r *request) (*http.Response, bool /* retry */, error) { return nil, false, gtserror.SetNotFound(err) } + // A retryable error. return nil, true, err } else if rsp.StatusCode > 500 || diff --git a/internal/httpclient/delivery.go b/internal/httpclient/delivery.go deleted file mode 100644 index 0ed620b6ce..0000000000 --- a/internal/httpclient/delivery.go +++ /dev/null @@ -1,291 +0,0 @@ -// 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 . - -package httpclient - -import ( - "context" - "slices" - "sync" - "time" - - "codeberg.org/gruf/go-runners" - "github.com/superseriousbusiness/gotosocial/internal/gtscontext" - "github.com/superseriousbusiness/gotosocial/internal/log" - "github.com/superseriousbusiness/gotosocial/internal/queue" -) - -// APDeliveryWorkerPool wraps APDeliveryWorker{}s -// in a singular struct for easy multi start/stop. -type APDeliveryWorkerPool struct { - client *Client - queue *queue.StructQueue[*queue.APRequest] - workers []APDeliveryWorker - mutex sync.Mutex -} - -// Init will initialize the DeliveryWorker{} pool -// with given http client, request queue to pull -// from and number of delivery workers to spawn. -func (p *APDeliveryWorkerPool) Init( - client *Client, - queue *queue.StructQueue[*queue.APRequest], -) { - p.mutex.Lock() - p.client = client - p.queue = queue - p.mutex.Unlock() -} - -// Start will attempt to start 'n' DeliveryWorker{}s. -func (p *APDeliveryWorkerPool) Start(n int) (ok bool) { - p.mutex.Lock() - if ok = (len(p.workers) == 0); ok { - p.workers = make([]APDeliveryWorker, n) - for i := range p.workers { - p.workers[i].client = p.client - p.workers[i].queue = p.queue - ok = p.workers[i].Start() && ok - } - } - p.mutex.Unlock() - return -} - -// Stop will attempt to stop all of the contained DeliveryWorker{}s. -func (p *APDeliveryWorkerPool) Stop() (ok bool) { - p.mutex.Lock() - if ok = (len(p.workers) > 0); ok { - for i := range p.workers { - ok = p.workers[i].Stop() && ok - } - p.workers = p.workers[:0] - } - p.mutex.Unlock() - return -} - -// APDeliveryWorker wraps a Client{} to feed from -// a queue.StructQueue{} for ActivityPub requests -// to deliver. It does so while prioritizing new -// queued requests over backlogged retries. -type APDeliveryWorker struct { - client *Client - queue *queue.StructQueue[*queue.APRequest] - backlog []*delivery - service runners.Service -} - -// NewAPDeliveryWorker returns a new APDeliveryWorker that feeds from queue, using given HTTP client. -func NewAPDeliveryWorker(client *Client, queue *queue.StructQueue[*queue.APRequest]) APDeliveryWorker { - return APDeliveryWorker{ - client: client, - queue: queue, - backlog: make([]*delivery, 0, 256), - } -} - -// Start will attempt to start the DeliveryWorker{}. -func (w *APDeliveryWorker) Start() bool { - return w.service.GoRun(w.process) -} - -// Stop will attempt to stop the DeliveryWorker{}. -func (w *APDeliveryWorker) Stop() bool { - return w.service.Stop() -} - -// process is the main delivery worker processing routine. -func (w *APDeliveryWorker) process(ctx context.Context) { - if w.client == nil || w.queue == nil { - panic("nil delivery worker fields") - } - -loop: - for { - // Get next delivery. - dlv, ok := w.next(ctx) - if !ok { - return - } - - // Check whether backoff required. - if d := dlv.BackOff(); d != 0 { - - // Start backoff sleep timer. - backoff, cncl := sleepch(d) - - select { - case <-ctx.Done(): - // Main ctx - // cancelled. - cncl() - - case <-w.queue.Wait(): - // A new message was - // queued, re-add this - // to backlog + retry. - w.pushBacklog(dlv) - cncl() - continue loop - - case <-backoff: - // successful - // backoff! - } - } - - dlv.log.Info("performing request") - - // Attempt outoing delivery of request. - _, retry, err := w.client.do(&dlv.request) - if err == nil { - continue loop - } - - dlv.log.Error(err) - - if !retry || w.client.badHosts.Has(dlv.host) || - dlv.attempts > w.client.retries { - // Drop deliveries when no retry requested, - // or we reach max defined retry attempts. - // "bad" hosts support a max of 1 attempt. - w.client.badHosts.Set(dlv.host, struct{}{}) - continue loop - } - - // Determine next delivery attempt. - dlv.next = time.Now().Add(dlv.BackOff()) - - // Push to backlog. - w.pushBacklog(dlv) - } -} - -// next gets the next available delivery, blocking until available if necessary. -func (w *APDeliveryWorker) next(ctx context.Context) (*delivery, bool) { -loop: - for { - // Try pop next queued. - msg, ok := w.queue.Pop() - - if !ok { - // Check the backlog. - if len(w.backlog) > 0 { - - // Sort by 'next' time. - sortDeliveries(w.backlog) - - // Pop next delivery. - dlv := w.popBacklog() - - return dlv, true - } - - select { - // Backlog is empty, we MUST - // block until next enqueued. - case <-w.queue.Wait(): - continue loop - - // Worker was stopped. - case <-ctx.Done(): - return nil, false - } - } - - // Wrap msg in delivery type. - return wrapMsg(ctx, msg), true - } -} - -// popBacklog pops next available from the backlog. -func (w *APDeliveryWorker) popBacklog() *delivery { - if len(w.backlog) == 0 { - return nil - } - - // Pop from backlog. - dlv := w.backlog[0] - - // Shift backlog down by one. - copy(w.backlog, w.backlog[1:]) - w.backlog = w.backlog[:len(w.backlog)-1] - - return dlv -} - -// pushBacklog pushes the given delivery to backlog. -func (w *APDeliveryWorker) pushBacklog(dlv *delivery) { - w.backlog = append(w.backlog, dlv) -} - -// delivery wraps request{} -// to cache logging fields. -type delivery struct { - - // cached log - // entry fields. - log log.Entry - - // next attempt time. - next time.Time - - // hostname string - // for bad host check. - host string - - // embedded - // request. - request -} - -// BackOff returns backoff duration to sleep for, calculated -// from the .next attempt field subtracted from current time. -func (d *delivery) BackOff() time.Duration { - if d.next.IsZero() { - return 0 - } - return time.Now().Sub(d.next) -} - -// wrapMsg wraps a received queued AP request message in our delivery type. -func wrapMsg(ctx context.Context, msg *queue.APRequest) *delivery { - dlv := new(delivery) - dlv.request = wrapRequest(msg.Request) - dlv.log = requestLog(dlv.req) - dlv.host = dlv.req.URL.Hostname() - ctx = gtscontext.WithValues(ctx, msg.Request.Context()) - dlv.req = dlv.req.WithContext(ctx) - return dlv -} - -// sortDeliveries sorts deliveries according -// to when is the first requiring re-attempt. -func sortDeliveries(d []*delivery) { - slices.SortFunc(d, func(a, b *delivery) int { - const k = +1 - switch { - case a.next.Before(b.next): - return +k - case b.next.Before(a.next): - return -k - default: - return 0 - } - }) -} diff --git a/internal/httpclient/request.go b/internal/httpclient/request.go index 1c3d8d0d38..914310ac00 100644 --- a/internal/httpclient/request.go +++ b/internal/httpclient/request.go @@ -29,37 +29,42 @@ const ( baseBackoff = 2 * time.Second ) -// request wraps an HTTP request +// Request wraps an HTTP request // to add our own retry / backoff. -type request struct { +type Request struct { + // log entry fields. + log log.Entry - // underlying request. - req *http.Request - - // current backoff dur. + // Current backoff dur. backoff time.Duration - // delivery attempts. + // Delivery attempts. attempts uint -} -// wrapRequest wraps an http.Request{} in our own request{} type. -func wrapRequest(req *http.Request) request { - var r request - r.req = req - return r + // done is marked when + // no more requests may + // be attempted. + done bool + + // underlying request. + *http.Request } -// requestLog returns a prepared log entry with fields for http.Request{}. -func requestLog(r *http.Request) log.Entry { - return log.WithContext(r.Context()). +// WrapRequest wraps an existing http.Request within +// our own httpclient.Request with retry / backoff tracking. +func WrapRequest(r *http.Request) Request { + var rr Request + rr.Request = r + rr.log = log.WithContext(r.Context()). WithField("method", r.Method). - WithField("url", r.URL.String()) + WithField("url", r.URL.String()). + WithField("contentType", r.Header.Get("Content-Type")) + return rr } -// BackOff returns the currently set backoff duration, -// setting a default according to no. attempts if needed. -func (r *request) BackOff() time.Duration { +// GetBackOff returns the currently set backoff duration, +// (using a default according to no. attempts if needed). +func (r *Request) BackOff() time.Duration { if r.backoff <= 0 { // No backoff dur found, set our predefined // backoff according to a multiplier of 2^n. diff --git a/internal/processing/workers/federate.go b/internal/processing/workers/federate.go index 581a61af44..e737513f54 100644 --- a/internal/processing/workers/federate.go +++ b/internal/processing/workers/federate.go @@ -77,9 +77,9 @@ func (f *federate) DeleteAccount(ctx context.Context, account *gtsmodel.Account) // Drop any queued outgoing AP requests to / from account, // (this stops any queued likes, boosts, creates etc). - f.state.Queues.APRequests.Delete("ActorID", account.URI) - f.state.Queues.APRequests.Delete("ObjectID", account.URI) - f.state.Queues.APRequests.Delete("TargetID", account.URI) + f.state.Workers.Delivery.Queue.Delete("ActorID", account.URI) + f.state.Workers.Delivery.Queue.Delete("ObjectID", account.URI) + f.state.Workers.Delivery.Queue.Delete("TargetID", account.URI) // Parse relevant URI(s). outboxIRI, err := parseURI(account.OutboxURI) @@ -230,8 +230,8 @@ func (f *federate) DeleteStatus(ctx context.Context, status *gtsmodel.Status) er // Drop any queued outgoing http requests for status, // (this stops any queued likes, boosts, creates etc). - f.state.Queues.APRequests.Delete("ObjectID", status.URI) - f.state.Queues.APRequests.Delete("TargetID", status.URI) + f.state.Workers.Delivery.Queue.Delete("ObjectID", status.URI) + f.state.Workers.Delivery.Queue.Delete("TargetID", status.URI) // Ensure the status model is fully populated. if err := f.state.DB.PopulateStatus(ctx, status); err != nil { diff --git a/internal/queue/messages.go b/internal/queue/messages.go deleted file mode 100644 index 05f423102f..0000000000 --- a/internal/queue/messages.go +++ /dev/null @@ -1,71 +0,0 @@ -// 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 . - -package queue - -import ( - "net/http" -) - -// TODO: add indexable queues for -// fedi / client api workers -// type ClientAPIMsg struct { -// // ... -// APObjectType string -// // ... -// APActivityType string -// // ... -// GTSID string -// // ... -// GTSModel any -// // ... -// Origin *gtsmodel.Account -// // ... -// Target *gtsmodel.Account -// } -// -// type FediAPIMsg struct { -// // ... -// APObjectType string -// // ... -// APActivityType string -// // ... -// APObjectID *url.URL -// // ... -// APObjectModel any -// // ... -// GTSModel any -// // ... -// Requesting *gtsmodel.Account -// // ... -// Receiving *gtsmodel.Account -// } - -type APRequest struct { - - // ActorID ... - ActorID string - - // ObjectID ... - ObjectID string - - // TargetID ... - TargetID string - - // Request ... - Request *http.Request -} diff --git a/internal/queue/queues.go b/internal/queue/queues.go deleted file mode 100644 index d8186f1a68..0000000000 --- a/internal/queue/queues.go +++ /dev/null @@ -1,47 +0,0 @@ -// 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 . - -package queue - -import ( - "codeberg.org/gruf/go-structr" - "github.com/superseriousbusiness/gotosocial/internal/log" -) - -// Queues ... -type Queues struct { - // APRequests ... - APRequests StructQueue[*APRequest] -} - -// Init will re(initialize) queues. NOTE: the queue -// MUST NOT be in use anywhere, this is not thread-safe. -func (q *Queues) Init() { - log.Infof(nil, "init: %p", q) - - q.initHTTPRequest() -} - -func (q *Queues) initHTTPRequest() { - q.APRequests.Init(structr.QueueConfig[*APRequest]{ - Indices: []structr.IndexConfig{ - {Fields: "ActorID", Multiple: true}, - {Fields: "ObjectID", Multiple: true}, - {Fields: "TargetID", Multiple: true}, - }, - }) -} diff --git a/internal/state/state.go b/internal/state/state.go index 5c64d12883..6120515b91 100644 --- a/internal/state/state.go +++ b/internal/state/state.go @@ -21,7 +21,6 @@ import ( "codeberg.org/gruf/go-mutexes" "github.com/superseriousbusiness/gotosocial/internal/cache" "github.com/superseriousbusiness/gotosocial/internal/db" - "github.com/superseriousbusiness/gotosocial/internal/queue" "github.com/superseriousbusiness/gotosocial/internal/storage" "github.com/superseriousbusiness/gotosocial/internal/timeline" "github.com/superseriousbusiness/gotosocial/internal/workers" @@ -37,9 +36,6 @@ type State struct { // Caches provides access to this state's collection of caches. Caches cache.Caches - // Queues provides access to this state's collection of queues. - Queues queue.Queues - // Timelines provides access to this state's collection of timelines. Timelines timeline.Timelines diff --git a/internal/transport/controller.go b/internal/transport/controller.go index 891a24495c..519298d8ee 100644 --- a/internal/transport/controller.go +++ b/internal/transport/controller.go @@ -28,7 +28,6 @@ import ( "io" "net/http" "net/url" - "runtime" "codeberg.org/gruf/go-byteutil" "codeberg.org/gruf/go-cache/v3" @@ -56,24 +55,16 @@ type controller struct { client pub.HttpClient trspCache cache.TTLCache[string, *transport] userAgent string - senders int // no. concurrent batch delivery routines. } // NewController returns an implementation of the Controller interface for creating new transports func NewController(state *state.State, federatingDB federatingdb.DB, clock pub.Clock, client pub.HttpClient) Controller { var ( - host = config.GetHost() - proto = config.GetProtocol() - version = config.GetSoftwareVersion() - senderMultiplier = config.GetAdvancedSenderMultiplier() + host = config.GetHost() + proto = config.GetProtocol() + version = config.GetSoftwareVersion() ) - senders := senderMultiplier * runtime.GOMAXPROCS(0) - if senders < 1 { - // Clamp senders to 1. - senders = 1 - } - c := &controller{ state: state, fedDB: federatingDB, @@ -81,7 +72,6 @@ func NewController(state *state.State, federatingDB federatingdb.DB, clock pub.C client: client, trspCache: cache.NewTTL[string, *transport](0, 100, 0), userAgent: fmt.Sprintf("gotosocial/%s (+%s://%s)", version, proto, host), - senders: senders, } return c diff --git a/internal/transport/deliver.go b/internal/transport/deliver.go index b8654ec58f..3a67326c13 100644 --- a/internal/transport/deliver.go +++ b/internal/transport/deliver.go @@ -28,13 +28,14 @@ import ( "github.com/superseriousbusiness/gotosocial/internal/config" "github.com/superseriousbusiness/gotosocial/internal/gtscontext" "github.com/superseriousbusiness/gotosocial/internal/gtserror" - "github.com/superseriousbusiness/gotosocial/internal/queue" + "github.com/superseriousbusiness/gotosocial/internal/httpclient" + "github.com/superseriousbusiness/gotosocial/internal/transport/delivery" ) func (t *transport) BatchDeliver(ctx context.Context, obj map[string]interface{}, recipients []*url.URL) error { var ( - // accumulated prepared reqs. - reqs []*queue.APRequest + // accumulated delivery reqs. + reqs []*delivery.Delivery // accumulated preparation errs. errs gtserror.MultiError @@ -78,8 +79,7 @@ func (t *transport) BatchDeliver(ctx context.Context, obj map[string]interface{} reqs = append(reqs, req) } - // Push the request list to HTTP client worker queue. - t.controller.state.Queues.APRequests.Push(reqs...) + t.controller.state.Workers.Delivery.Queue.Push(reqs...) // Return combined err. return errs.Combine() @@ -109,8 +109,7 @@ func (t *transport) Deliver(ctx context.Context, obj map[string]interface{}, to return err } - // Push the request to HTTP client worker queue. - t.controller.state.Queues.APRequests.Push(req) + t.controller.state.Workers.Delivery.Queue.Push(req) return nil } @@ -126,7 +125,7 @@ func (t *transport) prepare( data []byte, to *url.URL, ) ( - *queue.APRequest, + *delivery.Delivery, error, ) { url := to.String() @@ -142,19 +141,21 @@ func (t *transport) prepare( ctx = gtscontext.SetOutgoingPublicKeyID(ctx, t.pubKeyID) ctx = gtscontext.SetHTTPClientSignFunc(ctx, sign) - req, err := http.NewRequestWithContext(ctx, "POST", url, &body) + // Prepare a new request with data body directed at URL. + r, err := http.NewRequestWithContext(ctx, "POST", url, &body) if err != nil { return nil, gtserror.Newf("error preparing request: %w", err) } - req.Header.Add("Content-Type", string(apiutil.AppActivityLDJSON)) - req.Header.Add("Accept-Charset", "utf-8") + // Set the standard ActivityPub content-type + charset headers. + r.Header.Add("Content-Type", string(apiutil.AppActivityLDJSON)) + r.Header.Add("Accept-Charset", "utf-8") - return &queue.APRequest{ + return &delivery.Delivery{ ActorID: actorID, ObjectID: objectID, TargetID: targetID, - Request: req, + Request: httpclient.WrapRequest(r), }, nil } diff --git a/internal/transport/delivery/delivery.go b/internal/transport/delivery/delivery.go new file mode 100644 index 0000000000..29511b7fa9 --- /dev/null +++ b/internal/transport/delivery/delivery.go @@ -0,0 +1,281 @@ +// 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 . + +package delivery + +import ( + "context" + "slices" + "time" + + "codeberg.org/gruf/go-runners" + "codeberg.org/gruf/go-structr" + "github.com/superseriousbusiness/gotosocial/internal/httpclient" + "github.com/superseriousbusiness/gotosocial/internal/queue" +) + +type Delivery struct { + // ActorID ... + ActorID string + + // ObjectID ... + ObjectID string + + // TargetID ... + TargetID string + + // Request ... + Request httpclient.Request + + // internal fields. + next time.Time +} + +// BackOff ... +func (dlv *Delivery) BackOff() time.Duration { + if dlv.next.IsZero() { + return 0 + } + return time.Until(dlv.next) +} + +// WorkerPool wraps multiple Worker{}s in +// a singular struct for easy multi start/stop. +type WorkerPool struct { + + // Client defines httpclient.Client{} + // passed to each of delivery pool Worker{}s. + Client *httpclient.Client + + // Queue is the embedded queue.StructQueue{} + // passed to each of delivery pool Worker{}s. + Queue queue.StructQueue[*Delivery] + + // internal fields. + workers []Worker +} + +// Init will initialize the Worker{} pool +// with given http client, request queue to pull +// from and number of delivery workers to spawn. +func (p *WorkerPool) Init(client *httpclient.Client) { + p.Client = client + p.Queue.Init(structr.QueueConfig[*Delivery]{ + Indices: []structr.IndexConfig{ + {Fields: "ActorID", Multiple: true}, + {Fields: "ObjectID", Multiple: true}, + {Fields: "TargetID", Multiple: true}, + }, + }) +} + +// Start will attempt to start 'n' Worker{}s. +func (p *WorkerPool) Start(n int) (ok bool) { + if ok = (len(p.workers) == 0); ok { + p.workers = make([]Worker, n) + for i := range p.workers { + p.workers[i].Client = p.Client + p.workers[i].Queue = &p.Queue + ok = p.workers[i].Start() && ok + } + } + return +} + +// Stop will attempt to stop contained Worker{}s. +func (p *WorkerPool) Stop() (ok bool) { + if ok = (len(p.workers) > 0); ok { + for i := range p.workers { + ok = p.workers[i].Stop() && ok + } + p.workers = p.workers[:0] + } + return +} + +// Worker wraps an httpclient.Client{} to feed +// from queue.StructQueue{} for ActivityPub reqs +// to deliver. It does so while prioritizing new +// queued requests over backlogged retries. +type Worker struct { + + // Client is the httpclient.Client{} that + // delivery worker will use for requests. + Client *httpclient.Client + + // Queue is the Delivery{} message queue + // that delivery worker will feed from. + Queue *queue.StructQueue[*Delivery] + + // internal fields. + backlog []*Delivery + service runners.Service +} + +// Start will attempt to start the Worker{}. +func (w *Worker) Start() bool { + return w.service.GoRun(w.process) +} + +// Stop will attempt to stop the Worker{}. +func (w *Worker) Stop() bool { + return w.service.Stop() +} + +// process is the main delivery worker processing routine. +func (w *Worker) process(ctx context.Context) { + if w.Client == nil || w.Queue == nil { + panic("not yet initialized") + } + +loop: + for { + // Get next delivery. + dlv, ok := w.next(ctx) + if !ok { + return + } + + // Check whether backoff required. + if d := dlv.BackOff(); d != 0 { + + // Get queue wait ch. + queue := w.Queue.Wait() + + // Start backoff sleep timer. + backoff := time.NewTimer(d) + + select { + case <-ctx.Done(): + // Main ctx + // cancelled. + backoff.Stop() + + case <-queue: + // A new message was + // queued, re-add this + // to backlog + retry. + w.pushBacklog(dlv) + backoff.Stop() + continue loop + + case <-backoff.C: + // success! + } + } + + // Attempt delivery of AP request. + rsp, retry, err := w.Client.DoOnce( + &dlv.Request, + ) + + if err == nil { + // Ensure body closed. + _ = rsp.Body.Close() + continue loop + } + + if !retry { + // Drop deliveries when no + // retry requested, or they + // reached max (either). + continue loop + } + + // Determine next delivery attempt. + dlv.next = time.Now().Add(dlv.BackOff()) + + // Push to backlog. + w.pushBacklog(dlv) + } +} + +// next gets the next available delivery, blocking until available if necessary. +func (w *Worker) next(ctx context.Context) (*Delivery, bool) { +loop: + for { + // Try pop next queued. + dlv, ok := w.Queue.Pop() + + if !ok { + // Check the backlog. + if len(w.backlog) > 0 { + + // Sort by 'next' time. + sortDeliveries(w.backlog) + + // Pop next delivery. + dlv := w.popBacklog() + + return dlv, true + } + + select { + // Backlog is empty, we MUST + // block until next enqueued. + case <-w.Queue.Wait(): + continue loop + + // Worker was stopped. + case <-ctx.Done(): + return nil, false + } + } + + // Replace request context for worker state canceling. + dlv.Request.Request = dlv.Request.WithContext(ctx) + + return dlv, true + } +} + +// popBacklog pops next available from the backlog. +func (w *Worker) popBacklog() *Delivery { + if len(w.backlog) == 0 { + return nil + } + + // Pop from backlog. + dlv := w.backlog[0] + + // Shift backlog down by one. + copy(w.backlog, w.backlog[1:]) + w.backlog = w.backlog[:len(w.backlog)-1] + + return dlv +} + +// pushBacklog pushes the given delivery to backlog. +func (w *Worker) pushBacklog(dlv *Delivery) { + w.backlog = append(w.backlog, dlv) +} + +// sortDeliveries sorts deliveries according +// to when is the first requiring re-attempt. +func sortDeliveries(d []*Delivery) { + slices.SortFunc(d, func(a, b *Delivery) int { + const k = +1 + switch { + case a.next.Before(b.next): + return +k + case b.next.Before(a.next): + return -k + default: + return 0 + } + }) +} diff --git a/internal/workers/workers.go b/internal/workers/workers.go index e94e99f796..6b4c52b19d 100644 --- a/internal/workers/workers.go +++ b/internal/workers/workers.go @@ -24,17 +24,17 @@ import ( "codeberg.org/gruf/go-runners" "github.com/superseriousbusiness/gotosocial/internal/config" - "github.com/superseriousbusiness/gotosocial/internal/httpclient" "github.com/superseriousbusiness/gotosocial/internal/messages" "github.com/superseriousbusiness/gotosocial/internal/scheduler" + "github.com/superseriousbusiness/gotosocial/internal/transport/delivery" ) type Workers struct { // Main task scheduler instance. Scheduler scheduler.Scheduler - // APDelivery ... - APDelivery httpclient.APDeliveryWorkerPool + // ... + Delivery delivery.WorkerPool // ClientAPI provides a worker pool that handles both // incoming client actions, and our own side-effects. @@ -70,7 +70,8 @@ type Workers struct { _ nocopy } -// Start will start all of the contained worker pools (and global scheduler). +// Start will start all of the contained +// worker pools (and global scheduler). func (w *Workers) Start() { // Get currently set GOMAXPROCS. maxprocs := runtime.GOMAXPROCS(0) @@ -79,7 +80,7 @@ func (w *Workers) Start() { tryUntil("start ap delivery workerpool", 5, func() bool { n := config.GetAdvancedSenderMultiplier() - return w.APDelivery.Start(n * maxprocs) + return w.Delivery.Start(n * maxprocs) }) tryUntil("starting client API workerpool", 5, func() bool { @@ -98,7 +99,7 @@ func (w *Workers) Start() { // Stop will stop all of the contained worker pools (and global scheduler). func (w *Workers) Stop() { tryUntil("stopping scheduler", 5, w.Scheduler.Stop) - tryUntil("stopping ap delivery workerpool", 5, w.APDelivery.Stop) + tryUntil("stopping delivery workerpool", 5, w.Delivery.Stop) tryUntil("stopping client API workerpool", 5, w.ClientAPI.Stop) tryUntil("stopping federator workerpool", 5, w.Federator.Stop) tryUntil("stopping media workerpool", 5, w.Media.Stop) From 5d6ffd753b34a65206d7ec5143d1fd1f8f9b4a6e Mon Sep 17 00:00:00 2001 From: kim Date: Fri, 5 Apr 2024 15:23:20 +0100 Subject: [PATCH 16/35] remove dead code --- internal/httpclient/util.go | 26 -------------------------- 1 file changed, 26 deletions(-) delete mode 100644 internal/httpclient/util.go diff --git a/internal/httpclient/util.go b/internal/httpclient/util.go deleted file mode 100644 index c19418c963..0000000000 --- a/internal/httpclient/util.go +++ /dev/null @@ -1,26 +0,0 @@ -// 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 . - -package httpclient - -import "time" - -// sleepch returns a blocking sleep channel and cancel function. -func sleepch(d time.Duration) (<-chan time.Time, func() bool) { - t := time.NewTimer(d) - return t.C, t.Stop -} From 9ce0ffc8bd497fac913d4c8e953872b0f4974b65 Mon Sep 17 00:00:00 2001 From: kim Date: Fri, 5 Apr 2024 15:34:57 +0100 Subject: [PATCH 17/35] formatting --- internal/httpclient/sign.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/internal/httpclient/sign.go b/internal/httpclient/sign.go index 6b561c45a4..eff20be490 100644 --- a/internal/httpclient/sign.go +++ b/internal/httpclient/sign.go @@ -32,9 +32,7 @@ type SignFunc func(r *http.Request) error // (RoundTripper implementer) to check request // context for a signing function and using for // all subsequent trips through RoundTrip(). -type signingtransport struct { - http.Transport // underlying transport -} +type signingtransport struct{ http.Transport } func (t *signingtransport) RoundTrip(r *http.Request) (*http.Response, error) { // Ensure updated host always set. From 0f4c4257ed510aa013d0622dadfce32dd00d53f9 Mon Sep 17 00:00:00 2001 From: kim Date: Fri, 5 Apr 2024 15:37:28 +0100 Subject: [PATCH 18/35] validate request before queueing for delivery --- internal/transport/deliver.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/internal/transport/deliver.go b/internal/transport/deliver.go index 3a67326c13..a9e686605d 100644 --- a/internal/transport/deliver.go +++ b/internal/transport/deliver.go @@ -151,6 +151,11 @@ func (t *transport) prepare( r.Header.Add("Content-Type", string(apiutil.AppActivityLDJSON)) r.Header.Add("Accept-Charset", "utf-8") + // Validate the request before queueing for delivery. + if err := httpclient.ValidateRequest(r); err != nil { + return nil, err + } + return &delivery.Delivery{ ActorID: actorID, ObjectID: objectID, From 9264a82337dcad732434abdbb9cc5bd4a3e9a5d6 Mon Sep 17 00:00:00 2001 From: kim Date: Fri, 5 Apr 2024 15:44:10 +0100 Subject: [PATCH 19/35] finish adding code comments, fix up backoff code --- internal/transport/delivery/delivery.go | 30 ++++++++++++++++++------- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/internal/transport/delivery/delivery.go b/internal/transport/delivery/delivery.go index 29511b7fa9..70660a224d 100644 --- a/internal/transport/delivery/delivery.go +++ b/internal/transport/delivery/delivery.go @@ -28,25 +28,38 @@ import ( "github.com/superseriousbusiness/gotosocial/internal/queue" ) +// Delivery wraps an httpclient.Request{} +// to add ActivityPub ID IRI fields of the +// outgoing activity, so that deliveries may +// be indexed (and so, dropped from queue) +// by any of these possible ID IRIs. type Delivery struct { - // ActorID ... + + // ActorID contains the ActivityPub + // actor ID IRI (if any) of the activity + // being sent out by this request. ActorID string - // ObjectID ... + // ObjectID contains the ActivityPub + // object ID IRI (if any) of the activity + // being sent out by this request. ObjectID string - // TargetID ... + // TargetID contains the ActivityPub + // target ID IRI (if any) of the activity + // being sent out by this request. TargetID string - // Request ... + // Request is the prepared (+ wrapped) + // httpclient.Client{} request that + // constitutes this ActivtyPub delivery. Request httpclient.Request // internal fields. next time.Time } -// BackOff ... -func (dlv *Delivery) BackOff() time.Duration { +func (dlv *Delivery) backoff() time.Duration { if dlv.next.IsZero() { return 0 } @@ -151,7 +164,7 @@ loop: } // Check whether backoff required. - if d := dlv.BackOff(); d != 0 { + if d := dlv.backoff(); d != 0 { // Get queue wait ch. queue := w.Queue.Wait() @@ -197,7 +210,8 @@ loop: } // Determine next delivery attempt. - dlv.next = time.Now().Add(dlv.BackOff()) + backoff := dlv.Request.BackOff() + dlv.next = time.Now().Add(backoff) // Push to backlog. w.pushBacklog(dlv) From fd7c9588878b99e23158313359bc30387d3ba46e Mon Sep 17 00:00:00 2001 From: kim Date: Fri, 5 Apr 2024 15:48:36 +0100 Subject: [PATCH 20/35] finish adding more code comments --- internal/workers/workers.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/internal/workers/workers.go b/internal/workers/workers.go index 6b4c52b19d..2505676206 100644 --- a/internal/workers/workers.go +++ b/internal/workers/workers.go @@ -33,7 +33,10 @@ type Workers struct { // Main task scheduler instance. Scheduler scheduler.Scheduler - // ... + // Delivery provides a worker pool that + // handles outgoing ActivityPub deliveries. + // It contains an embedded (but accessible) + // indexed queue of Delivery{} objects. Delivery delivery.WorkerPool // ClientAPI provides a worker pool that handles both @@ -78,7 +81,7 @@ func (w *Workers) Start() { tryUntil("starting scheduler", 5, w.Scheduler.Start) - tryUntil("start ap delivery workerpool", 5, func() bool { + tryUntil("start delivery workerpool", 5, func() bool { n := config.GetAdvancedSenderMultiplier() return w.Delivery.Start(n * maxprocs) }) From e06617d24c392ee54ca9e74ca8cef9274d69f8ef Mon Sep 17 00:00:00 2001 From: kim Date: Fri, 5 Apr 2024 16:05:33 +0100 Subject: [PATCH 21/35] clamp minimum no. senders to 1 --- internal/workers/workers.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/internal/workers/workers.go b/internal/workers/workers.go index 2505676206..17728c2557 100644 --- a/internal/workers/workers.go +++ b/internal/workers/workers.go @@ -83,6 +83,10 @@ func (w *Workers) Start() { tryUntil("start delivery workerpool", 5, func() bool { n := config.GetAdvancedSenderMultiplier() + if n < 1 { + // clamp min senders to 1. + return w.Delivery.Start(1) + } return w.Delivery.Start(n * maxprocs) }) From e17474ff2d42cd8081275d3827923e9a0c3cdf82 Mon Sep 17 00:00:00 2001 From: kim Date: Fri, 5 Apr 2024 18:19:26 +0100 Subject: [PATCH 22/35] add start/stop logging to delivery worker, some slight changes --- internal/transport/delivery/delivery.go | 31 ++++++++++++++++++++----- 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/internal/transport/delivery/delivery.go b/internal/transport/delivery/delivery.go index 70660a224d..8d6f880fad 100644 --- a/internal/transport/delivery/delivery.go +++ b/internal/transport/delivery/delivery.go @@ -25,6 +25,7 @@ import ( "codeberg.org/gruf/go-runners" "codeberg.org/gruf/go-structr" "github.com/superseriousbusiness/gotosocial/internal/httpclient" + "github.com/superseriousbusiness/gotosocial/internal/log" "github.com/superseriousbusiness/gotosocial/internal/queue" ) @@ -99,6 +100,7 @@ func (p *WorkerPool) Init(client *httpclient.Client) { // Start will attempt to start 'n' Worker{}s. func (p *WorkerPool) Start(n int) (ok bool) { if ok = (len(p.workers) == 0); ok { + log.Infof(nil, "starting %d delivery workers", n) p.workers = make([]Worker, n) for i := range p.workers { p.workers[i].Client = p.Client @@ -112,6 +114,7 @@ func (p *WorkerPool) Start(n int) (ok bool) { // Stop will attempt to stop contained Worker{}s. func (p *WorkerPool) Stop() (ok bool) { if ok = (len(p.workers) > 0); ok { + log.Infof(nil, "stopping %d delivery workers", len(p.workers)) for i := range p.workers { ok = p.workers[i].Stop() && ok } @@ -141,7 +144,7 @@ type Worker struct { // Start will attempt to start the Worker{}. func (w *Worker) Start() bool { - return w.service.GoRun(w.process) + return w.service.GoRun(w.run) } // Stop will attempt to stop the Worker{}. @@ -149,6 +152,23 @@ func (w *Worker) Stop() bool { return w.service.Stop() } +// run wraps process to restart on any panic. +func (w *Worker) run(ctx context.Context) { + log.Infof(ctx, "%p: started delivery worker", w) + defer log.Infof(ctx, "%p: stopped delivery worker", w) + for returned := false; !returned; { + func() { + defer func() { + if r := recover(); r != nil { + log.Errorf(ctx, "recovered panic: %v", r) + } + }() + w.process(ctx) + returned = true + }() + } +} + // process is the main delivery worker processing routine. func (w *Worker) process(ctx context.Context) { if w.Client == nil || w.Queue == nil { @@ -164,10 +184,8 @@ loop: } // Check whether backoff required. - if d := dlv.backoff(); d != 0 { - - // Get queue wait ch. - queue := w.Queue.Wait() + const min = 100 * time.Millisecond + if d := dlv.backoff(); d > min { // Start backoff sleep timer. backoff := time.NewTimer(d) @@ -177,8 +195,9 @@ loop: // Main ctx // cancelled. backoff.Stop() + return - case <-queue: + case <-w.Queue.Wait(): // A new message was // queued, re-add this // to backlog + retry. From 500e7558e8e79e330bce1b83002901a52316b128 Mon Sep 17 00:00:00 2001 From: kim Date: Fri, 5 Apr 2024 18:20:11 +0100 Subject: [PATCH 23/35] remove double logging --- internal/transport/delivery/delivery.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/internal/transport/delivery/delivery.go b/internal/transport/delivery/delivery.go index 8d6f880fad..6275daf5f9 100644 --- a/internal/transport/delivery/delivery.go +++ b/internal/transport/delivery/delivery.go @@ -100,7 +100,6 @@ func (p *WorkerPool) Init(client *httpclient.Client) { // Start will attempt to start 'n' Worker{}s. func (p *WorkerPool) Start(n int) (ok bool) { if ok = (len(p.workers) == 0); ok { - log.Infof(nil, "starting %d delivery workers", n) p.workers = make([]Worker, n) for i := range p.workers { p.workers[i].Client = p.Client @@ -114,7 +113,6 @@ func (p *WorkerPool) Start(n int) (ok bool) { // Stop will attempt to stop contained Worker{}s. func (p *WorkerPool) Stop() (ok bool) { if ok = (len(p.workers) > 0); ok { - log.Infof(nil, "stopping %d delivery workers", len(p.workers)) for i := range p.workers { ok = p.workers[i].Stop() && ok } From f17ef91fb9a093c8f1ac40ba5794d15ede7bb2c0 Mon Sep 17 00:00:00 2001 From: kim Date: Fri, 5 Apr 2024 18:29:13 +0100 Subject: [PATCH 24/35] use worker ptrs --- internal/transport/delivery/delivery.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/internal/transport/delivery/delivery.go b/internal/transport/delivery/delivery.go index 6275daf5f9..6cff72c0ef 100644 --- a/internal/transport/delivery/delivery.go +++ b/internal/transport/delivery/delivery.go @@ -80,7 +80,7 @@ type WorkerPool struct { Queue queue.StructQueue[*Delivery] // internal fields. - workers []Worker + workers []*Worker } // Init will initialize the Worker{} pool @@ -100,8 +100,9 @@ func (p *WorkerPool) Init(client *httpclient.Client) { // Start will attempt to start 'n' Worker{}s. func (p *WorkerPool) Start(n int) (ok bool) { if ok = (len(p.workers) == 0); ok { - p.workers = make([]Worker, n) + p.workers = make([]*Worker, n) for i := range p.workers { + p.workers[i] = new(Worker) p.workers[i].Client = p.Client p.workers[i].Queue = &p.Queue ok = p.workers[i].Start() && ok @@ -115,6 +116,7 @@ func (p *WorkerPool) Stop() (ok bool) { if ok = (len(p.workers) > 0); ok { for i := range p.workers { ok = p.workers[i].Stop() && ok + p.workers[i] = nil } p.workers = p.workers[:0] } From 4811ded078ff34e2408e75d04590411182b3e2ae Mon Sep 17 00:00:00 2001 From: kim Date: Fri, 5 Apr 2024 22:35:16 +0100 Subject: [PATCH 25/35] expose the embedded log fields in httpclient.Request{} --- internal/httpclient/client.go | 4 ++-- internal/httpclient/request.go | 8 ++++---- internal/transport/deliver.go | 2 ++ 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/internal/httpclient/client.go b/internal/httpclient/client.go index b37ea2d40d..a7100bcc4b 100644 --- a/internal/httpclient/client.go +++ b/internal/httpclient/client.go @@ -264,12 +264,12 @@ func (c *Client) DoOnce(r *Request) (rsp *http.Response, retry bool, err error) if rsp != nil { // Log successful rsp. - r.log.Info(rsp.Status) + r.Entry.Info(rsp.Status) return } // Log any errors. - r.log.Error(err) + r.Entry.Error(err) switch { case !retry: diff --git a/internal/httpclient/request.go b/internal/httpclient/request.go index 914310ac00..311649adb6 100644 --- a/internal/httpclient/request.go +++ b/internal/httpclient/request.go @@ -32,9 +32,6 @@ const ( // Request wraps an HTTP request // to add our own retry / backoff. type Request struct { - // log entry fields. - log log.Entry - // Current backoff dur. backoff time.Duration @@ -46,6 +43,9 @@ type Request struct { // be attempted. done bool + // log fields. + log.Entry + // underlying request. *http.Request } @@ -55,7 +55,7 @@ type Request struct { func WrapRequest(r *http.Request) Request { var rr Request rr.Request = r - rr.log = log.WithContext(r.Context()). + rr.Entry = log.WithContext(r.Context()). WithField("method", r.Method). WithField("url", r.URL.String()). WithField("contentType", r.Header.Get("Content-Type")) diff --git a/internal/transport/deliver.go b/internal/transport/deliver.go index a9e686605d..a7e73465d9 100644 --- a/internal/transport/deliver.go +++ b/internal/transport/deliver.go @@ -79,6 +79,7 @@ func (t *transport) BatchDeliver(ctx context.Context, obj map[string]interface{} reqs = append(reqs, req) } + // Push prepared request list to the delivery queue. t.controller.state.Workers.Delivery.Queue.Push(reqs...) // Return combined err. @@ -109,6 +110,7 @@ func (t *transport) Deliver(ctx context.Context, obj map[string]interface{}, to return err } + // Push prepared request to the delivery queue. t.controller.state.Workers.Delivery.Queue.Push(req) return nil From 59ec876b571cbfa13e28b86b4a8987a1c871fa96 Mon Sep 17 00:00:00 2001 From: kim Date: Fri, 5 Apr 2024 22:46:35 +0100 Subject: [PATCH 26/35] ensure request context values are preserved when updating ctx --- internal/transport/delivery/delivery.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/transport/delivery/delivery.go b/internal/transport/delivery/delivery.go index 6cff72c0ef..b016b575a1 100644 --- a/internal/transport/delivery/delivery.go +++ b/internal/transport/delivery/delivery.go @@ -24,6 +24,7 @@ import ( "codeberg.org/gruf/go-runners" "codeberg.org/gruf/go-structr" + "github.com/superseriousbusiness/gotosocial/internal/gtscontext" "github.com/superseriousbusiness/gotosocial/internal/httpclient" "github.com/superseriousbusiness/gotosocial/internal/log" "github.com/superseriousbusiness/gotosocial/internal/queue" @@ -270,7 +271,8 @@ loop: } // Replace request context for worker state canceling. - dlv.Request.Request = dlv.Request.WithContext(ctx) + ctx := gtscontext.WithValues(ctx, dlv.Request.Context()) + dlv.Request.Request = dlv.Request.Request.WithContext(ctx) return dlv, true } From 1b9308ed8a9d1e7ec8325cf8366e4dbc68effe1d Mon Sep 17 00:00:00 2001 From: kim Date: Sat, 6 Apr 2024 17:35:20 +0100 Subject: [PATCH 27/35] add delivery worker tests --- internal/transport/delivery/delivery_test.go | 201 +++++++++++++++++++ 1 file changed, 201 insertions(+) create mode 100644 internal/transport/delivery/delivery_test.go diff --git a/internal/transport/delivery/delivery_test.go b/internal/transport/delivery/delivery_test.go new file mode 100644 index 0000000000..b45eccaea9 --- /dev/null +++ b/internal/transport/delivery/delivery_test.go @@ -0,0 +1,201 @@ +package delivery_test + +import ( + "fmt" + "io" + "math/rand" + "net" + "net/http" + "strconv" + "strings" + "testing" + + "codeberg.org/gruf/go-byteutil" + "github.com/superseriousbusiness/gotosocial/internal/config" + "github.com/superseriousbusiness/gotosocial/internal/httpclient" + "github.com/superseriousbusiness/gotosocial/internal/queue" + "github.com/superseriousbusiness/gotosocial/internal/transport/delivery" +) + +func TestDeliveryWorkerPool(t *testing.T) { + for _, i := range []int{1, 2, 4, 8, 16, 32} { + t.Run("size="+strconv.Itoa(i), func(t *testing.T) { + testDeliveryWorkerPool(t, i, generateInput(100*i)) + }) + } +} + +func testDeliveryWorkerPool(t *testing.T, sz int, input []*testrequest) { + wp := new(delivery.WorkerPool) + wp.Init(httpclient.New(httpclient.Config{ + AllowRanges: config.MustParseIPPrefixes([]string{ + "127.0.0.0/8", + }), + })) + if !wp.Start(sz) { + t.Fatal("failed starting pool") + } + defer wp.Stop() + test(t, &wp.Queue, input) +} + +func test( + t *testing.T, + queue *queue.StructQueue[*delivery.Delivery], + input []*testrequest, +) { + expect := make(chan *testrequest) + errors := make(chan error) + + // Prepare an HTTP test handler that ensures expected delivery is received. + handler := http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { + errors <- (<-expect).Equal(r) + }) + + // Start new HTTP test server listener. + l, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + defer l.Close() + + // Start the HTTP server. + srv := new(http.Server) + srv.Addr = "http://" + l.Addr().String() + srv.Handler = handler + go srv.Serve(l) + defer srv.Close() + + // Range over test input. + for _, test := range input { + + // Generate req for input. + req := test.Generate(srv.Addr) + r := httpclient.WrapRequest(req) + + // Wrap the request in delivery. + dlv := new(delivery.Delivery) + dlv.Request = r + + // Enqueue delivery! + queue.Push(dlv) + expect <- test + + // Wait for errors from handler. + if err := <-errors; err != nil { + t.Error(err) + } + } +} + +type testrequest struct { + method string + uri string + body []byte +} + +// generateInput generates 'n' many testrequest cases. +func generateInput(n int) []*testrequest { + tests := make([]*testrequest, n) + for i := range tests { + tests[i] = new(testrequest) + tests[i].method = randomMethod() + tests[i].uri = randomURI() + tests[i].body = randomBody(tests[i].method) + } + return tests +} + +var methods = []string{ + http.MethodConnect, + http.MethodDelete, + http.MethodGet, + http.MethodHead, + http.MethodOptions, + http.MethodPatch, + http.MethodPost, + http.MethodPut, + http.MethodTrace, +} + +// randomMethod generates a random http method. +func randomMethod() string { + return methods[rand.Intn(len(methods))] +} + +// randomURI generates a random http uri. +func randomURI() string { + n := rand.Intn(5) + p := make([]string, n) + for i := range p { + p[i] = strconv.Itoa(rand.Int()) + } + return "/" + strings.Join(p, "/") +} + +// randomBody generates a random http body DEPENDING on method. +func randomBody(method string) []byte { + if requiresBody(method) { + return []byte(method + " " + randomURI()) + } + return nil +} + +// requiresBody returns whether method requires body. +func requiresBody(method string) bool { + switch method { + case http.MethodPatch, + http.MethodPost, + http.MethodPut: + return true + default: + return false + } +} + +// Generate will generate a real http.Request{} from test data. +func (t *testrequest) Generate(addr string) *http.Request { + var body io.ReadCloser + if t.body != nil { + var b byteutil.ReadNopCloser + b.Reset(t.body) + body = &b + } + req, err := http.NewRequest(t.method, addr+t.uri, body) + if err != nil { + panic(err) + } + return req +} + +// Equal checks if request matches receiving test request. +func (t *testrequest) Equal(r *http.Request) error { + // Ensure methods match. + if t.method != r.Method { + return fmt.Errorf("differing request methods: t=%q r=%q", t.method, r.Method) + } + + // Ensure request URIs match. + if t.uri != r.URL.RequestURI() { + return fmt.Errorf("differing request urls: t=%q r=%q", t.uri, r.URL.RequestURI()) + } + + // Ensure body cases match. + if requiresBody(t.method) { + + // Read request into memory. + b, err := io.ReadAll(r.Body) + if err != nil { + return fmt.Errorf("error reading request body: %v", err) + } + + // Compare the request bodies. + st := strings.TrimSpace(string(t.body)) + sr := strings.TrimSpace(string(b)) + if st != sr { + return fmt.Errorf("differing request bodies: t=%q r=%q", st, sr) + } + } + + return nil +} From 51f6fd5986ad03fd5ccac6c9c2dd411bc6c3c8b7 Mon Sep 17 00:00:00 2001 From: kim Date: Sat, 6 Apr 2024 18:19:33 +0100 Subject: [PATCH 28/35] fix linter issues --- internal/httpclient/client.go | 2 +- internal/httpclient/request.go | 5 ----- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/internal/httpclient/client.go b/internal/httpclient/client.go index a7100bcc4b..31c6df7d0f 100644 --- a/internal/httpclient/client.go +++ b/internal/httpclient/client.go @@ -353,7 +353,7 @@ func (c *Client) do(r *Request) (rsp *http.Response, retry bool, err error) { r.backoff = time.Duration(u) * time.Second } else if at, _ := http.ParseTime(after); !at.Before(now) { // An HTTP formatted future date-time was provided. - r.backoff = at.Sub(time.Now()) + r.backoff = at.Sub(now) } // Don't let their provided backoff exceed our max. diff --git a/internal/httpclient/request.go b/internal/httpclient/request.go index 311649adb6..0df9211e73 100644 --- a/internal/httpclient/request.go +++ b/internal/httpclient/request.go @@ -38,11 +38,6 @@ type Request struct { // Delivery attempts. attempts uint - // done is marked when - // no more requests may - // be attempted. - done bool - // log fields. log.Entry From 39e6d05b6c2e38ebb65ec80a41b012bd0b08e9e5 Mon Sep 17 00:00:00 2001 From: kim Date: Sun, 7 Apr 2024 19:32:54 +0100 Subject: [PATCH 29/35] ensure delivery worker gets inited in testrig --- testrig/util.go | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/testrig/util.go b/testrig/util.go index e078074e55..b3d92f29db 100644 --- a/testrig/util.go +++ b/testrig/util.go @@ -30,7 +30,7 @@ import ( "github.com/superseriousbusiness/gotosocial/internal/filter/visibility" "github.com/superseriousbusiness/gotosocial/internal/messages" tlprocessor "github.com/superseriousbusiness/gotosocial/internal/processing/timeline" - wprocessor "github.com/superseriousbusiness/gotosocial/internal/processing/workers" + "github.com/superseriousbusiness/gotosocial/internal/processing/workers" "github.com/superseriousbusiness/gotosocial/internal/state" "github.com/superseriousbusiness/gotosocial/internal/timeline" "github.com/superseriousbusiness/gotosocial/internal/typeutils" @@ -44,6 +44,8 @@ func StartNoopWorkers(state *state.State) { state.Workers.ProcessFromClientAPI = func(context.Context, messages.FromClientAPI) error { return nil } state.Workers.ProcessFromFediAPI = func(context.Context, messages.FromFediAPI) error { return nil } + state.Workers.Delivery.Init(nil) + _ = state.Workers.Scheduler.Start() _ = state.Workers.ClientAPI.Start(1, 10) _ = state.Workers.Federator.Start(1, 10) @@ -52,11 +54,13 @@ func StartNoopWorkers(state *state.State) { // Starts workers on the provided state using processing functions from the given // workers processor. Useful when you *do* want to trigger side effects in a test. -func StartWorkers(state *state.State, wProcessor *wprocessor.Processor) { - state.Workers.EnqueueClientAPI = wProcessor.EnqueueClientAPI - state.Workers.EnqueueFediAPI = wProcessor.EnqueueFediAPI - state.Workers.ProcessFromClientAPI = wProcessor.ProcessFromClientAPI - state.Workers.ProcessFromFediAPI = wProcessor.ProcessFromFediAPI +func StartWorkers(state *state.State, processor *workers.Processor) { + state.Workers.EnqueueClientAPI = processor.EnqueueClientAPI + state.Workers.EnqueueFediAPI = processor.EnqueueFediAPI + state.Workers.ProcessFromClientAPI = processor.ProcessFromClientAPI + state.Workers.ProcessFromFediAPI = processor.ProcessFromFediAPI + + state.Workers.Delivery.Init(nil) _ = state.Workers.Scheduler.Start() _ = state.Workers.ClientAPI.Start(1, 10) From 3880020166f1188b6545c3c67e5e3a40a1e3788e Mon Sep 17 00:00:00 2001 From: kim Date: Mon, 8 Apr 2024 12:21:19 +0100 Subject: [PATCH 30/35] fix tests to delivering messages to check worker delivery queue --- internal/federation/federatingactor_test.go | 25 +++--- internal/processing/account_test.go | 27 +++--- internal/processing/followrequest_test.go | 83 +++++++++++-------- .../processing/workers/fromfediapi_test.go | 42 ++++++---- internal/transport/delivery/delivery.go | 7 ++ testrig/util.go | 58 +++++++++++++ 6 files changed, 168 insertions(+), 74 deletions(-) diff --git a/internal/federation/federatingactor_test.go b/internal/federation/federatingactor_test.go index 0c805a2c60..b5b65827ba 100644 --- a/internal/federation/federatingactor_test.go +++ b/internal/federation/federatingactor_test.go @@ -21,6 +21,7 @@ import ( "bytes" "context" "encoding/json" + "io" "net/url" "testing" "time" @@ -129,23 +130,27 @@ func (suite *FederatingActorTestSuite) TestSendRemoteFollower() { suite.NotNil(activity) // because we added 1 remote follower for zork, there should be a url in sentMessage - var sent [][]byte + var sent []byte if !testrig.WaitFor(func() bool { - sentI, ok := httpClient.SentMessages.Load(*testRemoteAccount.SharedInboxURI) - if ok { - sent, ok = sentI.([][]byte) - if !ok { - panic("SentMessages entry was not []byte") - } - return true + delivery, ok := suite.state.Workers.Delivery.Queue.Pop() + if !ok { + return false } - return false + if !testrig.EqualRequestURIs(delivery.Request.URL, *testRemoteAccount.SharedInboxURI) { + panic("differing request uris") + } + sent, err = io.ReadAll(delivery.Request.Body) + if err != nil { + panic("error reading body: " + err.Error()) + } + return true + }) { suite.FailNow("timed out waiting for message") } dst := new(bytes.Buffer) - err = json.Indent(dst, sent[0], "", " ") + err = json.Indent(dst, sent, "", " ") suite.NoError(err) suite.Equal(`{ "@context": "https://www.w3.org/ns/activitystreams", diff --git a/internal/processing/account_test.go b/internal/processing/account_test.go index 83eebedbad..82c28115eb 100644 --- a/internal/processing/account_test.go +++ b/internal/processing/account_test.go @@ -21,6 +21,7 @@ import ( "context" "encoding/json" "fmt" + "io" "testing" "time" @@ -55,7 +56,7 @@ func (suite *AccountTestSuite) TestAccountDeleteLocal() { suite.NoError(errWithCode) // the delete should be federated outwards to the following account's inbox - var sent [][]byte + var sent []byte delete := new(struct { Actor string `json:"actor"` ID string `json:"id"` @@ -66,16 +67,22 @@ func (suite *AccountTestSuite) TestAccountDeleteLocal() { }) if !testrig.WaitFor(func() bool { - sentI, ok := suite.httpClient.SentMessages.Load(*followingAccount.SharedInboxURI) - if ok { - sent, ok = sentI.([][]byte) - if !ok { - panic("SentMessages entry was not [][]byte") - } - err = json.Unmarshal(sent[0], delete) - return err == nil + delivery, ok := suite.state.Workers.Delivery.Queue.Pop() + if !ok { + return false } - return false + if !testrig.EqualRequestURIs(delivery.Request.URL, *followingAccount.SharedInboxURI) { + panic("differing request uris") + } + sent, err = io.ReadAll(delivery.Request.Body) + if err != nil { + panic("error reading body: " + err.Error()) + } + err = json.Unmarshal(sent, delete) + if err != nil { + panic("error unmarshaling json: " + err.Error()) + } + return true }) { suite.FailNow("timed out waiting for message") } diff --git a/internal/processing/followrequest_test.go b/internal/processing/followrequest_test.go index 4c089be4aa..db0419522c 100644 --- a/internal/processing/followrequest_test.go +++ b/internal/processing/followrequest_test.go @@ -21,6 +21,7 @@ import ( "context" "encoding/json" "fmt" + "io" "testing" "time" @@ -77,22 +78,6 @@ func (suite *FollowRequestTestSuite) TestFollowRequestAccept() { Note: "", }, relationship) - // accept should be sent to Some_User - var sent [][]byte - if !testrig.WaitFor(func() bool { - sentI, ok := suite.httpClient.SentMessages.Load(targetAccount.InboxURI) - if ok { - sent, ok = sentI.([][]byte) - if !ok { - panic("SentMessages entry was not []byte") - } - return true - } - return false - }) { - suite.FailNow("timed out waiting for message") - } - accept := &struct { Actor string `json:"actor"` ID string `json:"id"` @@ -106,8 +91,29 @@ func (suite *FollowRequestTestSuite) TestFollowRequestAccept() { To string `json:"to"` Type string `json:"type"` }{} - err = json.Unmarshal(sent[0], accept) - suite.NoError(err) + + // accept should be sent to Some_User + var sent []byte + if !testrig.WaitFor(func() bool { + delivery, ok := suite.state.Workers.Delivery.Queue.Pop() + if !ok { + return false + } + if !testrig.EqualRequestURIs(delivery.Request.URL, targetAccount.InboxURI) { + panic("differing request uris") + } + sent, err = io.ReadAll(delivery.Request.Body) + if err != nil { + panic("error reading body: " + err.Error()) + } + err = json.Unmarshal(sent, accept) + if err != nil { + panic("error unmarshaling json: " + err.Error()) + } + return true + }) { + suite.FailNow("timed out waiting for message") + } suite.Equal(requestingAccount.URI, accept.Actor) suite.Equal(targetAccount.URI, accept.Object.Actor) @@ -144,22 +150,6 @@ func (suite *FollowRequestTestSuite) TestFollowRequestReject() { suite.NoError(errWithCode) suite.EqualValues(&apimodel.Relationship{ID: "01FHMQX3GAABWSM0S2VZEC2SWC", Following: false, ShowingReblogs: false, Notifying: false, FollowedBy: false, Blocking: false, BlockedBy: false, Muting: false, MutingNotifications: false, Requested: false, DomainBlocking: false, Endorsed: false, Note: ""}, relationship) - // reject should be sent to Some_User - var sent [][]byte - if !testrig.WaitFor(func() bool { - sentI, ok := suite.httpClient.SentMessages.Load(targetAccount.InboxURI) - if ok { - sent, ok = sentI.([][]byte) - if !ok { - panic("SentMessages entry was not []byte") - } - return true - } - return false - }) { - suite.FailNow("timed out waiting for message") - } - reject := &struct { Actor string `json:"actor"` ID string `json:"id"` @@ -173,8 +163,29 @@ func (suite *FollowRequestTestSuite) TestFollowRequestReject() { To string `json:"to"` Type string `json:"type"` }{} - err = json.Unmarshal(sent[0], reject) - suite.NoError(err) + + // reject should be sent to Some_User + var sent []byte + if !testrig.WaitFor(func() bool { + delivery, ok := suite.state.Workers.Delivery.Queue.Pop() + if !ok { + return false + } + if !testrig.EqualRequestURIs(delivery.Request.URL, targetAccount.InboxURI) { + panic("differing request uris") + } + sent, err = io.ReadAll(delivery.Request.Body) + if err != nil { + panic("error reading body: " + err.Error()) + } + err = json.Unmarshal(sent, reject) + if err != nil { + panic("error unmarshaling json: " + err.Error()) + } + return true + }) { + suite.FailNow("timed out waiting for message") + } suite.Equal(requestingAccount.URI, reject.Actor) suite.Equal(targetAccount.URI, reject.Object.Actor) diff --git a/internal/processing/workers/fromfediapi_test.go b/internal/processing/workers/fromfediapi_test.go index 1dbefca840..51f61bd12b 100644 --- a/internal/processing/workers/fromfediapi_test.go +++ b/internal/processing/workers/fromfediapi_test.go @@ -21,6 +21,7 @@ import ( "context" "encoding/json" "fmt" + "io" "testing" "time" @@ -457,22 +458,6 @@ func (suite *FromFediAPITestSuite) TestProcessFollowRequestUnlocked() { }) suite.NoError(err) - // an accept message should be sent to satan's inbox - var sent [][]byte - if !testrig.WaitFor(func() bool { - sentI, ok := suite.httpClient.SentMessages.Load(*originAccount.SharedInboxURI) - if ok { - sent, ok = sentI.([][]byte) - if !ok { - panic("SentMessages entry was not []byte") - } - return true - } - return false - }) { - suite.FailNow("timed out waiting for message") - } - accept := &struct { Actor string `json:"actor"` ID string `json:"id"` @@ -486,8 +471,29 @@ func (suite *FromFediAPITestSuite) TestProcessFollowRequestUnlocked() { To string `json:"to"` Type string `json:"type"` }{} - err = json.Unmarshal(sent[0], accept) - suite.NoError(err) + + // an accept message should be sent to satan's inbox + var sent []byte + if !testrig.WaitFor(func() bool { + delivery, ok := suite.state.Workers.Delivery.Queue.Pop() + if !ok { + return false + } + if !testrig.EqualRequestURIs(delivery.Request.URL, *originAccount.SharedInboxURI) { + panic("differing request uris") + } + sent, err = io.ReadAll(delivery.Request.Body) + if err != nil { + panic("error reading body: " + err.Error()) + } + err = json.Unmarshal(sent, accept) + if err != nil { + panic("error unmarshaling json: " + err.Error()) + } + return true + }) { + suite.FailNow("timed out waiting for message") + } suite.Equal(targetAccount.URI, accept.Actor) suite.Equal(originAccount.URI, accept.Object.Actor) diff --git a/internal/transport/delivery/delivery.go b/internal/transport/delivery/delivery.go index b016b575a1..27281399fa 100644 --- a/internal/transport/delivery/delivery.go +++ b/internal/transport/delivery/delivery.go @@ -155,6 +155,9 @@ func (w *Worker) Stop() bool { // run wraps process to restart on any panic. func (w *Worker) run(ctx context.Context) { + if w.Client == nil || w.Queue == nil { + panic("not yet initialized") + } log.Infof(ctx, "%p: started delivery worker", w) defer log.Infof(ctx, "%p: stopped delivery worker", w) for returned := false; !returned; { @@ -173,6 +176,10 @@ func (w *Worker) run(ctx context.Context) { // process is the main delivery worker processing routine. func (w *Worker) process(ctx context.Context) { if w.Client == nil || w.Queue == nil { + // we perform this check here just + // to ensure the compiler knows these + // variables aren't nil in the loop, + // even if already checked by caller. panic("not yet initialized") } diff --git a/testrig/util.go b/testrig/util.go index b3d92f29db..f6f139e791 100644 --- a/testrig/util.go +++ b/testrig/util.go @@ -97,6 +97,64 @@ func StartTimelines(state *state.State, filter *visibility.Filter, converter *ty } } +// EqualRequestURIs checks whether inputs have equal request URIs, +// handling cases of url.URL{}, *url.URL{}, string, *string. +func EqualRequestURIs(u1, u2 any) bool { + var uri1, uri2 string + + requestURI := func(in string) (string, error) { + u, err := url.Parse(in) + if err != nil { + return "", err + } + return u.RequestURI(), nil + } + + switch u1 := u1.(type) { + case url.URL: + uri1 = u1.RequestURI() + case *url.URL: + uri1 = u1.RequestURI() + case *string: + var err error + uri1, err = requestURI(*u1) + if err != nil { + return false + } + case string: + var err error + uri1, err = requestURI(u1) + if err != nil { + return false + } + default: + panic("unsupported type") + } + + switch u2 := u2.(type) { + case url.URL: + uri2 = u2.RequestURI() + case *url.URL: + uri2 = u2.RequestURI() + case *string: + var err error + uri2, err = requestURI(*u2) + if err != nil { + return false + } + case string: + var err error + uri2, err = requestURI(u2) + if err != nil { + return false + } + default: + panic("unsupported type") + } + + return uri1 == uri2 +} + // CreateMultipartFormData is a handy function for taking a fieldname and a filename, and creating a multipart form bytes buffer // with the file contents set in the given fieldname. The extraFields param can be used to add extra FormFields to the request, as necessary. // The returned bytes.Buffer b can be used like so: From a90d51b6da7633e5a765d0a2a3766703dab6b7bb Mon Sep 17 00:00:00 2001 From: kim Date: Mon, 8 Apr 2024 12:51:34 +0100 Subject: [PATCH 31/35] update error type to use ptr instead of value receiver --- internal/federation/federatingprotocol.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/federation/federatingprotocol.go b/internal/federation/federatingprotocol.go index f8e5b4c091..1a655994c5 100644 --- a/internal/federation/federatingprotocol.go +++ b/internal/federation/federatingprotocol.go @@ -44,7 +44,7 @@ type errOtherIRIBlocked struct { iriStrs []string } -func (e errOtherIRIBlocked) Error() string { +func (e *errOtherIRIBlocked) Error() string { iriStrsNice := "[" + strings.Join(e.iriStrs, ", ") + "]" if e.domainBlock { return "domain block exists for one or more of " + iriStrsNice @@ -67,7 +67,7 @@ func newErrOtherIRIBlocked( e.iriStrs = append(e.iriStrs, iri.String()) } - return e + return &e } /* From 46130df630a3b294f0a330892cb2ca16b1d17a24 Mon Sep 17 00:00:00 2001 From: kim Date: Mon, 8 Apr 2024 13:12:06 +0100 Subject: [PATCH 32/35] fix test calling Workers{}.Start() instead of testrig.StartWorkers() --- internal/media/manager_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/internal/media/manager_test.go b/internal/media/manager_test.go index dbc9c634a7..ac4286c731 100644 --- a/internal/media/manager_test.go +++ b/internal/media/manager_test.go @@ -33,6 +33,7 @@ import ( "github.com/superseriousbusiness/gotosocial/internal/media" "github.com/superseriousbusiness/gotosocial/internal/state" gtsstorage "github.com/superseriousbusiness/gotosocial/internal/storage" + "github.com/superseriousbusiness/gotosocial/testrig" ) type ManagerTestSuite struct { @@ -1197,8 +1198,8 @@ func (suite *ManagerTestSuite) TestSimpleJpegProcessBlockingWithDiskStorage() { var state state.State - state.Workers.Start() - defer state.Workers.Stop() + testrig.StartNoopWorkers(&state) + defer testrig.StopWorkers(&state) storage := >sstorage.Driver{ Storage: disk, From 58d5bd5f6091a8e0c7862faf3e317dad55dcef95 Mon Sep 17 00:00:00 2001 From: kim Date: Mon, 8 Apr 2024 13:15:58 +0100 Subject: [PATCH 33/35] update docs for advanced-sender-multiplier --- docs/configuration/advanced.md | 13 ++++--------- example/config.yaml | 13 ++++--------- 2 files changed, 8 insertions(+), 18 deletions(-) diff --git a/docs/configuration/advanced.md b/docs/configuration/advanced.md index f776dac540..b97d8a6ba7 100644 --- a/docs/configuration/advanced.md +++ b/docs/configuration/advanced.md @@ -119,15 +119,10 @@ advanced-throttling-multiplier: 8 # Default: "30s" advanced-throttling-retry-after: "30s" -# Int. CPU multiplier for the amount of goroutines to spawn in order to send messages via ActivityPub. -# Messages will be batched so that at most multiplier * CPU count messages will be sent out at once. -# This can be tuned to limit concurrent POSTing to remote inboxes, preventing your instance CPU -# usage from skyrocketing when an account with many followers posts a new status. -# -# Messages are split among available senders, and each sender processes its assigned messages in serial. -# For example, say a user with 1000 followers is on an instance with 2 CPUs. With the default multiplier -# of 2, this means 4 senders would be in process at once on this instance. When the user creates a new post, -# each sender would end up iterating through about 250 Create messages + delivering them to remote instances. +# Int. CPU multiplier for the fixed number of goroutines to spawn in order to send messages via ActivityPub. +# Messages will be batched and pushed to a singular queue, from which multiplier * CPU count goroutines will +# pull and attempt deliveries. This can be tuned to limit concurrent posting to remote inboxes, preventing +# your instance CPU usage skyrocketing when accounts with many followers post statuses. # # If you set this to 0 or less, only 1 sender will be used regardless of CPU count. This may be # useful in cases where you are working with very tight network or CPU constraints. diff --git a/example/config.yaml b/example/config.yaml index 8c8db0932a..28518bf14d 100644 --- a/example/config.yaml +++ b/example/config.yaml @@ -1042,15 +1042,10 @@ advanced-throttling-multiplier: 8 # Default: "30s" advanced-throttling-retry-after: "30s" -# Int. CPU multiplier for the amount of goroutines to spawn in order to send messages via ActivityPub. -# Messages will be batched so that at most multiplier * CPU count messages will be sent out at once. -# This can be tuned to limit concurrent POSTing to remote inboxes, preventing your instance CPU -# usage from skyrocketing when an account with many followers posts a new status. -# -# Messages are split among available senders, and each sender processes its assigned messages in serial. -# For example, say a user with 1000 followers is on an instance with 2 CPUs. With the default multiplier -# of 2, this means 4 senders would be in process at once on this instance. When the user creates a new post, -# each sender would end up iterating through about 250 Create messages + delivering them to remote instances. +# Int. CPU multiplier for the fixed number of goroutines to spawn in order to send messages via ActivityPub. +# Messages will be batched and pushed to a singular queue, from which multiplier * CPU count goroutines will +# pull and attempt deliveries. This can be tuned to limit concurrent posting to remote inboxes, preventing +# your instance CPU usage skyrocketing when accounts with many followers post statuses. # # If you set this to 0 or less, only 1 sender will be used regardless of CPU count. This may be # useful in cases where you are working with very tight network or CPU constraints. From ad6cfc094d860480f03105e2de8156dfaaa0c756 Mon Sep 17 00:00:00 2001 From: kim Date: Mon, 8 Apr 2024 14:19:06 +0100 Subject: [PATCH 34/35] update to the latest activity library version --- go.mod | 2 +- go.sum | 4 ++-- vendor/modules.txt | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/go.mod b/go.mod index f368c78717..c68ca92de4 100644 --- a/go.mod +++ b/go.mod @@ -49,7 +49,7 @@ require ( github.com/spf13/cobra v1.8.0 github.com/spf13/viper v1.18.2 github.com/stretchr/testify v1.9.0 - github.com/superseriousbusiness/activity v1.6.0-gts.0.20240221151241-5d56c04088d4 + github.com/superseriousbusiness/activity v1.6.0-gts.0.20240408131430-247f7f7110f0 github.com/superseriousbusiness/httpsig v1.2.0-SSB github.com/superseriousbusiness/oauth2/v4 v4.3.2-SSB.0.20230227143000-f4900831d6c8 github.com/tdewolff/minify/v2 v2.20.19 diff --git a/go.sum b/go.sum index 7885ace6ef..1afc699e1f 100644 --- a/go.sum +++ b/go.sum @@ -623,8 +623,8 @@ github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8 github.com/subosito/gotenv v1.6.0 h1:9NlTDc1FTs4qu0DDq7AEtTPNw6SVm7uBMsUCUjABIf8= github.com/subosito/gotenv v1.6.0/go.mod h1:Dk4QP5c2W3ibzajGcXpNraDfq2IrhjMIvMSWPKKo0FU= github.com/sunfish-shogi/bufseekio v0.0.0-20210207115823-a4185644b365/go.mod h1:dEzdXgvImkQ3WLI+0KQpmEx8T/C/ma9KeS3AfmU899I= -github.com/superseriousbusiness/activity v1.6.0-gts.0.20240221151241-5d56c04088d4 h1:kPjQR/hVZtROTzkxptp/EIR7Wm58O8jppwpCFrZ7sVU= -github.com/superseriousbusiness/activity v1.6.0-gts.0.20240221151241-5d56c04088d4/go.mod h1:AZw0Xb4Oju8rmaJCZ21gc5CPg47MmNgyac+Hx5jo8VM= +github.com/superseriousbusiness/activity v1.6.0-gts.0.20240408131430-247f7f7110f0 h1:zPdbgwbjPxrJqme2sFTMQoML5ukNWRhChOnilR47rss= +github.com/superseriousbusiness/activity v1.6.0-gts.0.20240408131430-247f7f7110f0/go.mod h1:AZw0Xb4Oju8rmaJCZ21gc5CPg47MmNgyac+Hx5jo8VM= github.com/superseriousbusiness/go-jpeg-image-structure/v2 v2.0.0-20220321154430-d89a106fdabe h1:ksl2oCx/Qo8sNDc3Grb8WGKBM9nkvhCm25uvlT86azE= github.com/superseriousbusiness/go-jpeg-image-structure/v2 v2.0.0-20220321154430-d89a106fdabe/go.mod h1:gH4P6gN1V+wmIw5o97KGaa1RgXB/tVpC2UNzijhg3E4= github.com/superseriousbusiness/go-png-image-structure/v2 v2.0.1-SSB h1:8psprYSK1KdOSH7yQ4PbJq0YYaGQY+gzdW/B0ExDb/8= diff --git a/vendor/modules.txt b/vendor/modules.txt index eb45321f4e..41b09c98ea 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -628,7 +628,7 @@ github.com/stretchr/testify/suite # github.com/subosito/gotenv v1.6.0 ## explicit; go 1.18 github.com/subosito/gotenv -# github.com/superseriousbusiness/activity v1.6.0-gts.0.20240221151241-5d56c04088d4 +# github.com/superseriousbusiness/activity v1.6.0-gts.0.20240408131430-247f7f7110f0 ## explicit; go 1.18 github.com/superseriousbusiness/activity/pub github.com/superseriousbusiness/activity/streams From 65a54b47700075ee4ee9e77cb456a697f1ae4cf8 Mon Sep 17 00:00:00 2001 From: kim Date: Mon, 8 Apr 2024 16:25:52 +0100 Subject: [PATCH 35/35] add comment about not using httptest.Server{} --- internal/transport/delivery/delivery_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/internal/transport/delivery/delivery_test.go b/internal/transport/delivery/delivery_test.go index b45eccaea9..852c6f6f37 100644 --- a/internal/transport/delivery/delivery_test.go +++ b/internal/transport/delivery/delivery_test.go @@ -60,6 +60,10 @@ func test( defer l.Close() // Start the HTTP server. + // + // specifically not using httptest.Server{} here as httptest + // links that server with its own http.Client{}, whereas we're + // using an httpclient.Client{} (well, delivery routine is). srv := new(http.Server) srv.Addr = "http://" + l.Addr().String() srv.Handler = handler