Skip to content

Commit ae3a7f1

Browse files
committed
Reduce long-running federation transactions
Stop wrapping remote federation post persistence in explicit transactions in inbox handlers and remote actor post imports. This avoids holding database transactions open while fetching remote ActivityPub objects, preview cards, and media attachments, which should reduce PostgreSQL INSERT waiting pile-ups when remote servers are slow or unresponsive. Document the change in the 0.7.9 changelog. Refs #411
1 parent fde0c06 commit ae3a7f1

3 files changed

Lines changed: 47 additions & 53 deletions

File tree

CHANGES.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,16 @@ Version 0.7.9
66

77
To be released.
88

9+
- Reduced the risk of long-running PostgreSQL transactions during federation
10+
processing. Federation inbox handlers and remote actor post imports no
11+
longer wrap remote post/account persistence in explicit transactions, which
12+
could otherwise stay open while fetching remote ActivityPub objects,
13+
preview cards, and media attachments. This should reduce `INSERT waiting`
14+
pile-ups and improve resilience when remote servers are slow or
15+
unresponsive. [[#411]]
16+
17+
[#411]: https://github.com/fedify-dev/hollo/issues/411
18+
919

1020
Version 0.7.8
1121
-------------

src/federation/account.ts

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -246,16 +246,14 @@ export async function persistAccountPosts(
246246
} else if (activity instanceof Announce) {
247247
const item = await activity.getObject(options);
248248
if (!isPost(item)) continue;
249-
await db.transaction(async (tx) => {
250-
const post = await persistSharingPost(tx, activity, item, baseUrl, {
251-
...options,
252-
account,
253-
});
254-
if (post?.sharingId != null) {
255-
await updatePostStats(tx, { id: post.sharingId });
256-
}
257-
if (post != null) i++;
249+
const post = await persistSharingPost(db, activity, item, baseUrl, {
250+
...options,
251+
account,
258252
});
253+
if (post?.sharingId != null) {
254+
await updatePostStats(db, { id: post.sharingId });
255+
}
256+
if (post != null) i++;
259257
}
260258
if (i >= fetchPosts) break;
261259
}

src/federation/inbox.ts

Lines changed: 30 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -424,13 +424,12 @@ export async function onPostCreated(
424424
): Promise<void> {
425425
const object = await create.getObject();
426426
if (!isPost(object)) return;
427-
const post = await db.transaction(async (tx) => {
428-
const post = await persistPost(tx, object, ctx.origin, ctx);
429-
if (post?.replyTargetId != null) {
430-
await updatePostStats(tx, { id: post.replyTargetId });
431-
}
432-
return post;
433-
});
427+
// Avoid wrapping persistPost() in an explicit transaction.
428+
// It may fetch remote ActivityPub objects, preview cards, and media files.
429+
const post = await persistPost(db, object, ctx.origin, ctx);
430+
if (post?.replyTargetId != null) {
431+
await updatePostStats(db, { id: post.replyTargetId });
432+
}
434433

435434
// Create status notification for reply target author (if this is a reply)
436435
// and mention notifications for other mentioned local users
@@ -586,19 +585,10 @@ export async function onPostShared(
586585
): Promise<void> {
587586
const object = await announce.getObject();
588587
if (!isPost(object)) return;
589-
const post = await db.transaction(async (tx) => {
590-
const post = await persistSharingPost(
591-
tx,
592-
announce,
593-
object,
594-
ctx.origin,
595-
ctx,
596-
);
597-
if (post?.sharingId != null) {
598-
await updatePostStats(tx, { id: post.sharingId });
599-
}
600-
return post;
601-
});
588+
const post = await persistSharingPost(db, announce, object, ctx.origin, ctx);
589+
if (post?.sharingId != null) {
590+
await updatePostStats(db, { id: post.sharingId });
591+
}
602592
if (post?.sharing?.account?.owner != null) {
603593
await ctx.forwardActivity(
604594
{ username: post.sharing.account.owner.handle },
@@ -669,16 +659,14 @@ export async function onPostPinned(
669659
const accountList = await db.query.accounts.findMany({
670660
where: eq(accounts.featuredUrl, add.targetId.href),
671661
});
672-
await db.transaction(async (tx) => {
673-
const post = await persistPost(tx, object, ctx.origin, ctx);
674-
if (post == null) return;
675-
for (const account of accountList) {
676-
await tx.insert(pinnedPosts).values({
677-
postId: post.id,
678-
accountId: account.id,
679-
} satisfies NewPinnedPost);
680-
}
681-
});
662+
const post = await persistPost(db, object, ctx.origin, ctx);
663+
if (post == null) return;
664+
for (const account of accountList) {
665+
await db.insert(pinnedPosts).values({
666+
postId: post.id,
667+
accountId: account.id,
668+
} satisfies NewPinnedPost);
669+
}
682670
}
683671

684672
export async function onPostUnpinned(
@@ -691,20 +679,18 @@ export async function onPostUnpinned(
691679
const accountList = await db.query.accounts.findMany({
692680
where: eq(accounts.featuredUrl, remove.targetId.href),
693681
});
694-
await db.transaction(async (tx) => {
695-
const post = await persistPost(tx, object, ctx.origin, ctx);
696-
if (post == null) return;
697-
for (const account of accountList) {
698-
await tx
699-
.delete(pinnedPosts)
700-
.where(
701-
and(
702-
eq(pinnedPosts.postId, post.id),
703-
eq(pinnedPosts.accountId, account.id),
704-
),
705-
);
706-
}
707-
});
682+
const post = await persistPost(db, object, ctx.origin, ctx);
683+
if (post == null) return;
684+
for (const account of accountList) {
685+
await db
686+
.delete(pinnedPosts)
687+
.where(
688+
and(
689+
eq(pinnedPosts.postId, post.id),
690+
eq(pinnedPosts.accountId, account.id),
691+
),
692+
);
693+
}
708694
}
709695

710696
export async function onLiked(

0 commit comments

Comments
 (0)