github-starred/pangolin
2
0
Fork 0
mirror of https://github.com/fosrl/pangolin.git synced 2026-07-17 07:04:36 -05:00
Code Issues 1.6k Packages Projects Releases 79 Wiki Activity

[PR #3148] [MERGED] fix(audit-logs): route request audit log reads through logsDb #34632

New Issue
Closed
opened 2026-06-17 04:14:47 -05:00 by GiteaMirror · 0 comments
GiteaMirror commented 2026-06-17 04:14:47 -05:00
Owner
Copy Link

📋 Pull Request Information

Original PR: https://github.com/fosrl/pangolin/pull/3148
Author: @bishnubista
Created: 5/25/2026
Status: ✅ Merged
Merged: 5/25/2026
Merged by: @oschwartz10612

Base: main ← Head: fix-audit-log-replica-routing

📝 Commits (1)

  • 817e848 fix(audit-logs): route request audit log reads through logsDb

📊 Changes

2 files changed (+14 additions, -14 deletions)

View changed files

📝 server/routers/auditLogs/queryRequestAnalytics.ts (+5 -5)
📝 server/routers/auditLogs/queryRequestAuditLog.ts (+9 -9)

📄 Description

Community Contribution License Agreement

By creating this pull request, I grant the project maintainers an unlimited,
perpetual license to use, modify, and redistribute these contributions under any terms they
choose, including both the AGPLv3 and the Fossorial Commercial license terms. I
represent that I have the right to grant this license for all contributed content.

Description

Route the read paths in server/routers/auditLogs/queryRequestAuditLog.ts and server/routers/auditLogs/queryRequestAnalytics.ts through logsDb instead of primaryLogsDb, matching the existing private audit log routes (queryActionAuditLog, queryAccessAuditLog, queryConnectionAuditLog all already use logsDb).

In Postgres deployments configured with read replicas via withReplicas (server/db/pg/logsDriver.ts), this lets high-volume audit log reads use the replica pool instead of contending with writes on the primary. No-op on SQLite where logsDb === primaryDb.

Why not also rewrite the facet queries?

While here, I benchmarked a candidate rewrite for the // TODO: SOMEONE PLEASE OPTIMIZE THIS!!!!! at queryRequestAuditLog.ts:246. Replacing the six parallel `selectDistinct` queries with a single `UNION ALL` over six `GROUP BY ... LIMIT 500` arms is 48-61% slower on SQLite at 100k / 300k / 1M rows (20 runs per impl per size):

Size OLD `SELECT DISTINCT LIMIT 501` `UNION ALL` rewrite 6 × `Promise.all` `GROUP BY`
100k 103.5 ms 153.4 ms (+48.1%) 153.6 ms (+48.3%)
300k 629.7 ms 1015.1 ms (+61.2%) 972.6 ms (+54.4%)
1M 3152.1 ms 4821.9 ms (+53.0%) 4825.9 ms (+53.1%)

`EXPLAIN QUERY PLAN` shows each grouped arm does `USE TEMP B-TREE FOR GROUP BY` — every distinct value is materialized before `LIMIT` truncates. The current DISTINCT+LIMIT short-circuits via hash dedup with early exit, which wins for the low-cardinality dropdown shape this endpoint produces. The long-term fix is likely a materialized facets table refreshed by a background job, not a query-shape rewrite.

How to test?

  1. `npx tsc --noEmit` — no new errors introduced in the changed files (same pre-existing error count before and after).
  2. Manual check on an OSS SQLite stack: load `/[orgId]/settings/logs/request` in the dashboard; confirm the response shape (logs, pagination, filter dropdowns) is unchanged.
  3. SaaS verification: in a Postgres deployment with `postgres_logs.replicas` configured, observe that requests to `GET /org/{orgId}/logs/request` route through the replica pool instead of incrementing query counters on the primary log DB.

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/fosrl/pangolin/pull/3148 **Author:** [@bishnubista](https://github.com/bishnubista) **Created:** 5/25/2026 **Status:** ✅ Merged **Merged:** 5/25/2026 **Merged by:** [@oschwartz10612](https://github.com/oschwartz10612) **Base:** `main` ← **Head:** `fix-audit-log-replica-routing` --- ### 📝 Commits (1) - [`817e848`](https://github.com/fosrl/pangolin/commit/817e848d084ecbf314fac24b13b853ca48dd99a6) fix(audit-logs): route request audit log reads through logsDb ### 📊 Changes **2 files changed** (+14 additions, -14 deletions) <details> <summary>View changed files</summary> 📝 `server/routers/auditLogs/queryRequestAnalytics.ts` (+5 -5) 📝 `server/routers/auditLogs/queryRequestAuditLog.ts` (+9 -9) </details> ### 📄 Description ## Community Contribution License Agreement By creating this pull request, I grant the project maintainers an unlimited, perpetual license to use, modify, and redistribute these contributions under any terms they choose, including both the AGPLv3 and the Fossorial Commercial license terms. I represent that I have the right to grant this license for all contributed content. ## Description Route the read paths in `server/routers/auditLogs/queryRequestAuditLog.ts` and `server/routers/auditLogs/queryRequestAnalytics.ts` through `logsDb` instead of `primaryLogsDb`, matching the existing private audit log routes (`queryActionAuditLog`, `queryAccessAuditLog`, `queryConnectionAuditLog` all already use `logsDb`). In Postgres deployments configured with read replicas via `withReplicas` (`server/db/pg/logsDriver.ts`), this lets high-volume audit log reads use the replica pool instead of contending with writes on the primary. No-op on SQLite where `logsDb === primaryDb`. ### Why not also rewrite the facet queries? While here, I benchmarked a candidate rewrite for the `// TODO: SOMEONE PLEASE OPTIMIZE THIS!!!!!` at `queryRequestAuditLog.ts:246`. Replacing the six parallel \`selectDistinct\` queries with a single \`UNION ALL\` over six \`GROUP BY ... LIMIT 500\` arms is **48-61% slower** on SQLite at 100k / 300k / 1M rows (20 runs per impl per size): | Size | OLD \`SELECT DISTINCT LIMIT 501\` | \`UNION ALL\` rewrite | 6 × \`Promise.all\` \`GROUP BY\` | |---|---:|---:|---:| | 100k | 103.5 ms | 153.4 ms (+48.1%) | 153.6 ms (+48.3%) | | 300k | 629.7 ms | 1015.1 ms (+61.2%) | 972.6 ms (+54.4%) | | 1M | 3152.1 ms | 4821.9 ms (+53.0%) | 4825.9 ms (+53.1%) | \`EXPLAIN QUERY PLAN\` shows each grouped arm does \`USE TEMP B-TREE FOR GROUP BY\` — every distinct value is materialized before \`LIMIT\` truncates. The current DISTINCT+LIMIT short-circuits via hash dedup with early exit, which wins for the low-cardinality dropdown shape this endpoint produces. The long-term fix is likely a materialized facets table refreshed by a background job, not a query-shape rewrite. ## How to test? 1. \`npx tsc --noEmit\` — no new errors introduced in the changed files (same pre-existing error count before and after). 2. Manual check on an OSS SQLite stack: load \`/[orgId]/settings/logs/request\` in the dashboard; confirm the response shape (logs, pagination, filter dropdowns) is unchanged. 3. SaaS verification: in a Postgres deployment with \`postgres_logs.replicas\` configured, observe that requests to \`GET /org/{orgId}/logs/request\` route through the replica pool instead of incrementing query counters on the primary log DB. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
GiteaMirror added the pull-request label 2026-06-17 04:14:47 -05:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
api
authentication
bug
config
dependencies
docker
documentation
enhancement
good first issue
help wanted
Improvement
Look Into
needs investigating
networking
new feature
non-critical bug
potential bug
pull-request
Mirrored from GitHub Pull Request
 question
reverse proxy
Security
stale
ui
wontfix
No Label pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
GiteaMirror ninjasurge
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: github-starred/pangolin#34632
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?