ENGAGEMENT.md Phase 14, the last phase of the workstream. Four engagement
tables grew on every fire and nothing had ever deleted from any of them.
Three of them now have a horizon, swept nightly by one worker
(utils/engagementRetentionPrune.js — setInterval + unref + stop(), batched
1000 x 50, each table's failure caught on its own so a lock timeout on one
does not leave the other two unbounded):
engagement_sends 180 days engagement_sends_retain_days (7-3650)
engagement_cooldowns 30 days engagement_cooldowns_retain_days (2-3650)
engagement_outbox 30 days engagement_outbox_retain_days (2-3650)
The fourth, engagement_suppressions, does not expire, and that is the
recorded decision rather than an omission: a suppression is a standing
decision, and ageing out a hard bounce re-mails an address that already
bounced. The way out stays deliberate, and is now reachable per row.
Six decisions were settled by the org lead before any code. Two of them
widened the phase past what was offered:
* the send-log horizon is admin-configurable, so retention got a SCREEN
(Admin -> Engagement -> Retention) where team_activity and
user_notifications keep theirs in invisible settings rows. The send-log
horizon changes what an operator-facing page is able to show, so it has
to be visible; the other two came with it, because "what does this
deployment keep" is one question.
* the suppression purge, which cost a Phase 9 decision. The list
deliberately stripped address_hash from every row, so the only way out
was a window.prompt asking the operator to retype an address the screen
has never shown them. The row had no handle at all. The hash is now
returned: this route is admin-only and an admin can already suppress and
unsuppress any address they can name, so it grants no capability they
lack. GET /sends still strips its own.
The outbox sweep is TERMINAL-ONLY and that is a correctness rule: a
scheduled row is a send this deployment still intends to make (delay_seconds
can put one a day out) and a sending row may be mid-flight.
One shipped defect had to be fixed for the sweep to be a bound at all.
reclaimStale returned every stale sending row to scheduled, and MAX_ATTEMPTS
is consulted only on a graceful retry outcome — so a send that killed the
process mid-flight cycled sending -> scheduled -> sending forever, never
terminal, therefore never eligible for any sweep. It now fails an exhausted
row BEFORE reclaiming the rest; the order is the fix.
Two indexes (idx_engo_sweep, idx_engs_sweep): every existing index on those
tables has created_at in second position, which serves a per-rule window and
is useless to a whole-table horizon.
Proved twice: engagementRetentionSql.test.js against a real MariaDB (7
tests, incl. the acceptance case and the wrong reclaim order run
deliberately), and the live stack, where a 90-day-old cancelled row was
swept and a 90-day-old scheduled row survived.
Co-Authored-By: Claude <noreply@anthropic.com>
101 lines
4.9 KiB
JavaScript
101 lines
4.9 KiB
JavaScript
const { query } = require('../../utils/db')
|
|
|
|
/**
|
|
* Claim a fire for (rule, user, subject, channel), or refuse it because that
|
|
* delivery is still cooling. ENGAGEMENT.md §4.1.
|
|
*
|
|
* **`channel` is part of the key, and Phase 11b is where that was settled.** The
|
|
* engine claims inside its per-channel loop, so a key without the channel means
|
|
* the first channel of a two-channel rule claims the cooldown and every later one
|
|
* is refused as cooling - which made every in-universe email body of Phase 11b
|
|
* unreachable behind the in-app one. A cooldown is per delivery.
|
|
*
|
|
* **Two statements, each of which is its own atomic decision** - and it is worth
|
|
* saying why it is not the single `INSERT ... ON DUPLICATE KEY UPDATE` §4.1
|
|
* describes, because that version was written, tested green against an in-memory
|
|
* stub, and disproved by the first run against a real MariaDB.
|
|
*
|
|
* The one-statement form reads its answer out of `affectedRows`, on the usual
|
|
* contract: 1 for an insert, 2 for an update that changed something, and 0 for a
|
|
* duplicate key whose update changed nothing - that 0 being "the guard failed, so
|
|
* this pair is still cooling". **The mariadb Node connector sets `foundRows: true`
|
|
* by default**, which makes `affectedRows` report rows MATCHED rather than rows
|
|
* CHANGED, and `utils/db.js` does not override it. Under that pool the no-op case
|
|
* returns 1, indistinguishable from a fresh insert: every cooldown would have
|
|
* passed, always, and nothing in a stubbed test could have noticed.
|
|
*
|
|
* So the guard moves into a WHERE clause, where a row either matches or does not
|
|
* and `foundRows` has nothing to fold together:
|
|
*
|
|
* 1. UPDATE the row, guarded on the interval. `affectedRows = 1` means this
|
|
* caller moved it and owns the fire.
|
|
* 2. If that matched nothing, the row either does not exist yet or is still
|
|
* cooling. `INSERT IGNORE` separates the two: 1 means we inserted the first
|
|
* fire, 0 means the row was there and step 1 already said it is cooling.
|
|
*
|
|
* It is still race-free, and each race resolves the right way:
|
|
* - two concurrent first fires: neither UPDATEs, both INSERT IGNORE, exactly
|
|
* one gets 1 (the primary key decides). The loser is treated as cooling.
|
|
* - two concurrent fires after expiry: the row is locked by the first UPDATE,
|
|
* and the second re-evaluates its guard against the committed row - which now
|
|
* holds `now`, so it fails and is refused.
|
|
*
|
|
* `cooldown_seconds = 0` always passes, which is the documented meaning of a rule
|
|
* with no cooldown: the guard becomes `last_fired_at <= now`, and it is.
|
|
*/
|
|
async function claim(ruleId, userId, subjectKey, channel, cooldownSeconds, now = new Date()) {
|
|
const moved = await query(
|
|
`UPDATE engagement_cooldowns
|
|
SET last_fired_at = ?, fire_count = fire_count + 1
|
|
WHERE rule_id = ? AND user_id = ? AND subject_key = ? AND channel = ?
|
|
AND last_fired_at <= ? - INTERVAL ? SECOND`,
|
|
[now, ruleId, userId, subjectKey, channel, now, cooldownSeconds],
|
|
)
|
|
if (Number(moved?.affectedRows || 0) === 1) return true
|
|
|
|
const inserted = await query(
|
|
`INSERT IGNORE INTO engagement_cooldowns (rule_id, user_id, subject_key, channel, last_fired_at, fire_count)
|
|
VALUES (?, ?, ?, ?, ?, 1)`,
|
|
[ruleId, userId, subjectKey, channel, now],
|
|
)
|
|
return Number(inserted?.affectedRows || 0) === 1
|
|
}
|
|
|
|
const get = async (ruleId, userId, subjectKey, channel) => {
|
|
const [row] = await query(
|
|
`SELECT * FROM engagement_cooldowns
|
|
WHERE rule_id = ? AND user_id = ? AND subject_key = ? AND channel = ?`,
|
|
[ruleId, userId, subjectKey, channel],
|
|
)
|
|
return row || null
|
|
}
|
|
|
|
/**
|
|
* Drop cooldown rows older than `olderThan`.
|
|
*
|
|
* `idx_engc_sweep (last_fired_at)` exists for this: the table is written on every
|
|
* fire and read once per fire, so without a prune it is the unbounded-growth
|
|
* failure `teamActivityPrune` was written for. A dropped row means the next fire
|
|
* is treated as a first fire, which is correct as long as the retention window is
|
|
* longer than the longest configured cooldown - the caller's job, not this one's.
|
|
* Phase 14 gave it that caller (`engagementRetentionPrune`), which also enforces
|
|
* the horizon-versus-longest-cooldown rule this comment names.
|
|
*
|
|
* `limit` bounds one statement, for `userNotificationsPrune`'s reason: a first
|
|
* sweep after a long outage must be a series of bounded DELETEs rather than one
|
|
* that holds locks over a million rows. Omitting it keeps the original
|
|
* unbounded behaviour, so existing callers and tests are unaffected.
|
|
*
|
|
* @returns {Promise<number>} rows deleted
|
|
*/
|
|
const prune = async (olderThan, limit = 0) => {
|
|
const bounded = Number(limit) > 0
|
|
const result = await query(
|
|
`DELETE FROM engagement_cooldowns WHERE last_fired_at < ?${bounded ? ' LIMIT ?' : ''}`,
|
|
bounded ? [olderThan, Math.floor(limit)] : [olderThan],
|
|
)
|
|
return Number(result?.affectedRows || 0)
|
|
}
|
|
|
|
module.exports = { claim, get, prune }
|