Skip to content

Commit

Permalink
Added caching to the LinkRedirectRepository (#20036)
Browse files Browse the repository at this point in the history
ref
https://linear.app/tryghost/issue/ENG-851/implement-a-minimal-but-complete-version-of-redirect-caching-to
ref https://app.incident.io/ghost/incidents/55

Often immediately after sending an email, sites receive a large volume
of requests to LinkRedirect endpoints from members clicking on the links in
the email.

We currently don't cache any of these requests in our CDN, because we
also record click events, update the member's `last_seen_at` timestamp,
and send webhooks in response to these clicks, so Ghost needs to handle
each of these requests itself. This means that each of these LinkRedirect requests
hits Ghost, and currently all these requests hit the database to lookup
where to redirect the member to.

Each one of these requests can make up to 11 database queries, which can
quickly exhaust Ghost's database connection pool. Even though the
LinkRedirect lookup query is fairly cheap and quick, these queries aren't
prioritized over the "record" queries Ghost needs to handle, so they can
get stuck behind other queries in the queue and eventually timeout.

The result is that members are unable to actually reach the destination
of the link they clicked on, instead receiving a 500 error in Ghost, or
it can take a long time (60s+) for the redirect to happen.

This PR uses our existing `adapterManager` to cache the redirect lookups
either in-memory or in Redis (if configured — by default there is no caching). This only removes 1 out of
11 queries per redirect request, so it won't reduce the load on the DB
drastically, but it at least decouples the serving of the LinkRedirect from
the DB so the member can be redirected even if the DB is under heavy
load.

Local load testing results have shown a decrease in response times from
60 seconds to ~50ms for the redirect requests when handling 500 requests
per second, and reduced the 500 error rate to 0.
  • Loading branch information
cmraible authored Apr 26, 2024
1 parent 892b9ab commit dcd65bf
Show file tree
Hide file tree
Showing 3 changed files with 343 additions and 8 deletions.
Original file line number Diff line number Diff line change
@@ -1,45 +1,83 @@
const LinkRedirect = require('@tryghost/link-redirects').LinkRedirect;
const ObjectID = require('bson-objectid').default;
const debug = require('@tryghost/debug')('LinkRedirectRepository');

module.exports = class LinkRedirectRepository {
/** @type {Object} */
#LinkRedirect;
/** @type {Object} */
#urlUtils;
/** @type {Boolean} */
#cacheEnabled;
/** @type {Object} */
#cache;

/**
* @param {object} deps
* @param {object} deps.LinkRedirect Bookshelf Model
* @param {object} deps.LinkRedirect - Bookshelf Model
* @param {object} deps.urlUtils
* @param {object} deps.cacheAdapter - Cache Adapter instance, or null if cache is disabled
* @param {object} deps.EventRegistry
*/
constructor(deps) {
debug('Creating LinkRedirectRepository');
this.#LinkRedirect = deps.LinkRedirect;
this.#urlUtils = deps.urlUtils;
this.#cache = null;
if (deps.cacheAdapter !== null) {
debug('Caching enabled with adapter:', deps.cacheAdapter.constructor.name);
this.#cache = deps.cacheAdapter;
// This is a bit of a blunt instrument, but it's the best we can do for now
// It covers all the cases we would need to invalidate the links cache
// We need to invalidate the cache when:
// - a redirect is edited
// - a site's subdirectory is changed (rare)
// - analytics settings are changed
deps.EventRegistry.on('site.changed', () => {
this.#cache.reset();
});
}
}

/**
* Save a new LinkRedirect to the DB
* @param {InstanceType<LinkRedirect>} linkRedirect
* @returns {Promise<void>}
*/
async save(linkRedirect) {
debug('Saving link redirect', linkRedirect.from.pathname, '->', linkRedirect.to.href);
const model = await this.#LinkRedirect.add({
// Only store the pathname (no support for variable query strings)
from: this.stripSubdirectoryFromPath(linkRedirect.from.pathname),
to: linkRedirect.to.href
}, {});

linkRedirect.link_id = ObjectID.createFromHexString(model.id);
if (this.#cache) {
debug('Caching new link redirect', linkRedirect.from.pathname);
this.#cache.set(linkRedirect.from.pathname, this.#serialize(linkRedirect));
}
}

/**
* Trim the leading slash from a URL path
* @param {string} url
* @returns {string} url without leading slash
*/
#trimLeadingSlash(url) {
return url.replace(/^\//, '');
}

/**
* Returns a LinkRedirect object from a model
* @param {object} model - Bookshelf model instance
* @returns {InstanceType<LinkRedirect>} LinkRedirect
*/
fromModel(model) {
// Store if link has been edited
// Note: in some edge cases updated_at is set directly after created_at, sometimes with a second difference, so we need to check for that
const edited = model.get('updated_at')?.getTime() > (model.get('created_at')?.getTime() + 1000);

return new LinkRedirect({
id: model.id,
from: new URL(this.#trimLeadingSlash(model.get('from')), this.#urlUtils.urlFor('home', true)),
Expand All @@ -48,6 +86,43 @@ module.exports = class LinkRedirectRepository {
});
}

/**
* Create a LinkRedirect object from a JSON object (e.g. from the cache)
* @param {object} serialized
* @param {string} serialized.link_id - string representation of ObjectID
* @param {string} serialized.from - path of the URL
* @param {string} serialized.to - URL to redirect to
* @param {boolean} serialized.edited - whether the link has been edited
* @returns {InstanceType<LinkRedirect>} LinkRedirect
*/
#fromSerialized(serialized) {
return new LinkRedirect({
id: serialized.link_id,
from: new URL(this.#trimLeadingSlash(serialized.from), this.#urlUtils.urlFor('home', true)),
to: new URL(serialized.to),
edited: serialized.edited
});
}

/**
* Serialize a LinkRedirect object to a plain object (e.g. for caching)
* @param {InstanceType<LinkRedirect>} linkRedirect
* @returns {object} - serialized LinkRedirect
*/
#serialize(linkRedirect) {
return {
link_id: linkRedirect.link_id.toHexString(),
from: linkRedirect.from.pathname,
to: linkRedirect.to.href,
edited: linkRedirect.edited
};
}

/**
* Get all LinkRedirects from the DB, with optional filters
* @param {object} options - options passed directly to LinkRedirect.findAll
* @returns {Promise<InstanceType<LinkRedirect>[]>} array of LinkRedirects
*/
async getAll(options) {
const collection = await this.#LinkRedirect.findAll(options);

Expand All @@ -60,6 +135,11 @@ module.exports = class LinkRedirectRepository {
return result;
}

/**
* Get all LinkRedirect IDs from the DB, with optional filters
* @param {object} options - options passed directly to LinkRedirect.getFilteredCollectionQuery
* @returns {Promise<string[]>} array of LinkRedirect IDs
*/
async getFilteredIds(options) {
const linkRows = await this.#LinkRedirect.getFilteredCollectionQuery(options)
.select('redirects.id')
Expand All @@ -68,25 +148,44 @@ module.exports = class LinkRedirectRepository {
}

/**
*
* Get a LinkRedirect by its URL
* @param {URL} url
* @returns {Promise<InstanceType<LinkRedirect>|undefined>} linkRedirect
* @returns {Promise<InstanceType<LinkRedirect>|undefined>} LinkRedirect
*/
async getByURL(url) {
debug('Getting link redirect for', url.pathname);
// Strip subdirectory from path
const from = this.stripSubdirectoryFromPath(url.pathname);

const linkRedirect = await this.#LinkRedirect.findOne({
if (this.#cache) {
const cachedLink = await this.#cache.get(from);
// Cache hit, serve from cache
if (cachedLink) {
debug('Cache hit for', from);
return this.#fromSerialized(cachedLink);
}
}

// Cache miss, fetch from the DB
const linkRedirectModel = await this.#LinkRedirect.findOne({
from
}, {});

if (linkRedirect) {
return this.fromModel(linkRedirect);
if (linkRedirectModel) {
const linkRedirect = this.fromModel(linkRedirectModel);
if (this.#cache) {
debug('Cache miss for', from, '. Caching');
// Cache the link
this.#cache.set(from, this.#serialize(linkRedirect));
}
return linkRedirect;
}
}

/**
* Convert root relative URLs to subdirectory relative URLs
* @param {string} path
* @returns {string} path without subdirectory
*/
stripSubdirectoryFromPath(path) {
// Bit weird, but only way to do it with the urlUtils atm
Expand Down
7 changes: 6 additions & 1 deletion ghost/core/core/server/services/link-redirection/index.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
const urlUtils = require('../../../shared/url-utils');
const LinkRedirectRepository = require('./LinkRedirectRepository');
const adapterManager = require('../adapter-manager');
const config = require('../../../shared/config');
const EventRegistry = require('../../lib/common/events');

class LinkRedirectsServiceWrapper {
async init() {
Expand All @@ -15,7 +18,9 @@ class LinkRedirectsServiceWrapper {

this.linkRedirectRepository = new LinkRedirectRepository({
LinkRedirect: models.Redirect,
urlUtils
urlUtils,
cacheAdapter: config.get('hostSettings:linkRedirectsPublicCache:enabled') ? adapterManager.getAdapter('cache:linkRedirectsPublic') : null,
EventRegistry
});

// Expose the service
Expand Down
Loading

0 comments on commit dcd65bf

Please sign in to comment.