Skip to content

fix: optimize notification_log_wallet_address insert performance#1861

Merged
Gr1dlock merged 3 commits intomainfrom
Gr1dlock/optimize-notif-wallet-insert
Apr 9, 2026
Merged

fix: optimize notification_log_wallet_address insert performance#1861
Gr1dlock merged 3 commits intomainfrom
Gr1dlock/optimize-notif-wallet-insert

Conversation

@Gr1dlock
Copy link
Copy Markdown
Contributor

@Gr1dlock Gr1dlock commented Apr 9, 2026

PR Type

  • Regular Task
  • Bug Fix
  • QA Tests

Description

Optimizes the notification_log_wallet_address bulk insert query that was consuming a large percentage of DB query runtime. Two changes:

  1. Batch inserts into groups of 100 — Hasura wraps bulk inserts in a CTE with RETURNING * and constraint checks. Large batches (hundreds of wallet addresses) produce oversized queries with significant planning overhead. Batching keeps each query small and efficient.

  2. Fix v2 schema duplicating wallet address logs per localisation — Previously, for v2 schema notifications, logNotification was called once per localisation entry, each time inserting the same full set of wallet addresses. This multiplied insert volume by the number of localisations. Now wallet addresses are logged once regardless of schema version.

Also adds error handling for insert failures that were previously silently swallowed (fire-and-forget without try/catch).

Checklist

  • I have self-reviewed this PR.
  • I have left comments in the code for clarity.
  • I have added necessary unit tests.
  • I have updated the documentation as needed.

Batch wallet address inserts into groups of 100 to reduce Hasura CTE
query overhead, and fix v2 schema logging wallet addresses once instead
of duplicating per localisation.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@Gr1dlock
Copy link
Copy Markdown
Contributor Author

Gr1dlock commented Apr 9, 2026

@codex review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 406900875e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

const logMessage =
schemaVersion === "v1"
? (parsedParams as SendNotificationBodyV1).message
: (parsedParams as SendNotificationBodyV2).localisations?.[0]?.message;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Use deterministic locale for v2 log message

When schemaVersion is v2, the logged message is taken from localisations?.[0]?.message, which depends on client-supplied array order. This means the same notification payload can produce different notification_log.message values if localisations are reordered, and logs can end up in a non-English language even though validation guarantees an en localisation exists. After consolidating to one log row per send, pick a stable locale (e.g. en with fallback) so log data remains consistent and queryable.

Useful? React with 👍 / 👎.

Pick the "en" localisation for the log message instead of relying on
client-supplied array order, with fallback to the first entry.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@Gr1dlock Gr1dlock merged commit fe76511 into main Apr 9, 2026
13 checks passed
@Gr1dlock Gr1dlock deleted the Gr1dlock/optimize-notif-wallet-insert branch April 9, 2026 21:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants