-
Notifications
You must be signed in to change notification settings - Fork 4
Overview of cleanup - suggested indexes and export tool #1975
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
RagnarFatland
wants to merge
25
commits into
main
Choose a base branch
from
chore/cleanupMigration2026
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from 18 commits
Commits
Show all changes
25 commits
Select commit
Hold shift + click to select a range
7d8dd5c
Generated documentation, scipt and console app to export data neccesa…
35b2ce1
Adjusted A2Parties table to handle SelfIdentified users having partyu…
794beec
Various corrections after initial coderabbit review.
424b440
Cleanup of various issue; including fixing proper cursor for export.
1905fd3
minor tweaks, added test script for export query so I can manually te…
b24755c
fixes some sql syntas issues, added manual test scripts
e6f58ba
handle edge cases for index creation
ad7cb68
Misc optimizations after creating indexes and testing queries manually.
8e608aa
various attempts to tweak and test
ed447d1
fix comments from coderabbit
2c43cfe
more fix
b8b2c92
more testing
a4997e6
Updates after massive testing of export for issue 1716, should be pro…
6ccdfa7
updates after comments from Coderabbits
b0ea142
more fixes
9e3e14f
Fix broken syntax of export script.
c81321b
changed cursor usage to per-status
52b434d
Additional troubleshooting and optimization.
169d26a
Improved status logging.
31ee9fc
Tweaks after feedback from Coderabbit.
7034317
Updated for optimized Issue 1951.
7cfb8ca
cleanup documentation
4c9f7e4
tweaked export with abillity to have a delay between each export to a…
3f0ea46
Major documentation cleanup.
0ebf622
minor fixes to documentation and cleanup-scripts.
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,218 @@ | ||
| # CRITICAL FIX: Cursor Pagination Index Issue | ||
|
|
||
| ## Problem Discovered | ||
|
|
||
| **Batch 1**: 642ms (✅ 1,558 rows/sec - EXCELLENT) | ||
| **Batch 2**: 21,174ms (❌ 47 rows/sec - 33x SLOWER!) | ||
|
|
||
| ## Root Cause | ||
|
|
||
| The cursor pagination was comparing against **joined table columns** instead of **indexed table columns**: | ||
|
|
||
| ### ❌ BEFORE (Broken) | ||
| ```sql | ||
| FROM correspondence."A2Iss1716A2Events" a2Events | ||
| INNER JOIN correspondence."CorrespondenceStatuses" stats | ||
| ON a2Events."CorrespondenceId" = stats."CorrespondenceId" | ||
| ... | ||
| WHERE a2Events."Status" = 4 | ||
| AND (stats."CorrespondenceId", stats."Status") > (@lastId, @lastStatus) -- ❌ Wrong! | ||
| ORDER BY stats."CorrespondenceId", Status | ||
| ``` | ||
|
|
||
| **Problem**: | ||
| - Index is on `a2Events` table: `IX_A2Iss1716A2Events_CorrId_Status_Party (CorrespondenceId, Status, PartyUuid)` | ||
| - Cursor compares against `stats` table (joined table) | ||
| - PostgreSQL cannot use the index efficiently → **19-second query execution!** | ||
|
|
||
| ### ✅ AFTER (Fixed) | ||
| ```sql | ||
| FROM correspondence."A2Iss1716A2Events" a2Events | ||
| INNER JOIN correspondence."CorrespondenceStatuses" stats | ||
| ON a2Events."CorrespondenceId" = stats."CorrespondenceId" | ||
| ... | ||
| WHERE a2Events."Status" = 4 | ||
| AND (a2Events."CorrespondenceId", a2Events."Status") > (@lastId, @lastStatus) -- ✅ Index! | ||
| ORDER BY stats."CorrespondenceId", Status -- ✅ SELECT list (for DISTINCT)! | ||
| ``` | ||
|
RagnarFatland marked this conversation as resolved.
Outdated
|
||
|
|
||
| **Fix**: | ||
| - **Cursor**: References `a2Events` columns (indexed table) → allows index usage | ||
| - **ORDER BY**: References `stats` columns (in SELECT list) → satisfies DISTINCT requirement | ||
| - **Join Equivalence**: `a2Events."CorrespondenceId" = stats."CorrespondenceId"` → PostgreSQL recognizes the columns are equivalent via the join condition | ||
| - **Why ORDER BY can use stats columns**: The optimizer understands that due to the INNER JOIN equality, ordering by `stats."CorrespondenceId"` is equivalent to ordering by `a2Events."CorrespondenceId"`. Since `stats."CorrespondenceId"` is in the SELECT list (required by DISTINCT), we can ORDER BY it while still benefiting from the index on `a2Events` used by the cursor predicate. | ||
| - **Result**: Cursor predicate uses indexed `a2Events` columns → index seek works efficiently → **Expected < 1 second!** | ||
|
|
||
| ## Timing Evidence | ||
|
|
||
| ### Batch 1 (No Cursor) ✅ | ||
| ``` | ||
| Status 4: ExecuteReader=91ms, Read 1000 rows=20ms, Total=112ms | ||
| Status 6: ExecuteReader=22ms, Read 1000 rows=488ms, Total=511ms | ||
| Batch: Fetch=633ms, Merge=4ms, Write=4ms, Total=642ms | ||
| ``` | ||
| **Perfect performance** - index working correctly | ||
|
|
||
| ### Batch 2 (With Broken Cursor) ❌ | ||
| ``` | ||
| Status 4: ExecuteReader=19,062ms, Read 1000 rows=864ms, Total=19,926ms | ||
| Status 6: ExecuteReader=1,225ms, Read 1000 rows=17ms, Total=1,243ms | ||
| Batch: Fetch=21,170ms, Merge=1ms, Write=2ms, Total=21,174ms | ||
| ``` | ||
| **Status 4 taking 19 SECONDS!** - cursor breaking index | ||
|
|
||
| ### Expected Batch 2 (With Fixed Cursor) ✅ | ||
| ``` | ||
| Status 4: ExecuteReader=~100ms, Read 1000 rows=~20ms, Total=~120ms | ||
| Status 6: ExecuteReader=~100ms, Read 1000 rows=~20ms, Total=~120ms | ||
| Batch: Fetch=~250ms, Merge=~4ms, Write=~4ms, Total=~260ms | ||
| ``` | ||
| **Should be ~260ms per batch** - consistent with Batch 1 | ||
|
|
||
| ## Impact on Full Export | ||
|
|
||
| ### Before Fix ❌ | ||
| ``` | ||
| Batch 1: 642ms | ||
| Batch 2: 21,174ms | ||
| Batch 3: 21,174ms (estimated) | ||
| ... | ||
| Average: ~20,000ms per batch | ||
| Total batches: 9,970 | ||
| Total time: 9,970 × 20s = 199,400s = 55 hours | ||
| ``` | ||
|
|
||
| ### After Fix ✅ | ||
| ``` | ||
| Batch 1: 642ms | ||
| Batch 2: 260ms (estimated with fix) | ||
| Batch 3: 260ms | ||
| ... | ||
| Average: ~350ms per batch | ||
| Total batches: 9,970 | ||
| Total time: 9,970 × 0.35s = 3,490s = 58 minutes | ||
| ``` | ||
|
|
||
| **Improvement**: 55 hours → 58 minutes = **57x faster!** | ||
|
|
||
| ## Files Changed | ||
|
|
||
| ### 1. DialogActivityExportService.cs | ||
| **Line 457-459** (Issue #1716 cursor predicate): | ||
| ```csharp | ||
| // BEFORE | ||
| var a2EventsCursorPredicate = lastCursor.HasValue | ||
| ? "AND (stats.\"CorrespondenceId\", stats.\"Status\") > (@lastId, @lastStatus)" | ||
| : ""; | ||
|
|
||
| // AFTER | ||
| var a2EventsCursorPredicate = lastCursor.HasValue | ||
| ? "AND (a2Events.\"CorrespondenceId\", a2Events.\"Status\") > (@lastId, @lastStatus)" | ||
| : ""; | ||
| ``` | ||
|
|
||
| **Line 491** (ORDER BY clause): | ||
| ```csharp | ||
| // BEFORE | ||
| ORDER BY stats.""CorrespondenceId"", Status | ||
|
|
||
| // AFTER | ||
| ORDER BY a2Events.""CorrespondenceId"", Status | ||
| ``` | ||
|
|
||
| ### 2. Test_Export_Query.sql | ||
| **Lines 176-177** (Status 4 cursor): | ||
| ```sql | ||
| -- BEFORE | ||
| -- AND (stats."CorrespondenceId", stats."Status") > ('last-uuid'::uuid, 4) | ||
| ORDER BY stats."CorrespondenceId", Status | ||
|
|
||
| -- AFTER | ||
| -- AND (a2Events."CorrespondenceId", a2Events."Status") > ('last-uuid'::uuid, 4) | ||
| ORDER BY a2Events."CorrespondenceId", Status | ||
| ``` | ||
|
|
||
| **Lines 203-204** (Status 6 cursor): | ||
| ```sql | ||
| -- Same change for Status 6 | ||
| ``` | ||
|
|
||
| ## Verification | ||
|
|
||
| ### Test Again | ||
| ```powershell | ||
| .\test-export-2.ps1 | ||
| ``` | ||
|
|
||
| ### Expected Results | ||
| ``` | ||
| Batch 1: Fetch=~650ms, Total=~660ms (unchanged - already fast) | ||
| Batch 2: Fetch=~250ms, Total=~260ms (was 21,174ms - now 80x faster!) | ||
| ``` | ||
|
|
||
| ### Look For | ||
| ✅ Status 4 ExecuteReader < 200ms (was 19,062ms) | ||
| ✅ Status 6 ExecuteReader < 200ms (was 1,225ms) | ||
| ✅ Both batches similar timing (~250-650ms each) | ||
| ✅ Consistent rows/sec (~1,500-3,000 rows/sec) | ||
|
|
||
| ## Why This Happened | ||
|
|
||
| 1. **First query (no cursor)**: PostgreSQL uses index directly on `a2Events` table | ||
| - `WHERE a2Events."Status" = 4` | ||
| - Index: `IX_A2Iss1716A2Events_CorrId_Status_Party` | ||
| - Result: Fast! ✅ | ||
|
|
||
| 2. **Subsequent queries (with cursor)**: Cursor referenced joined table | ||
| - `WHERE a2Events."Status" = 4 AND (stats."CorrespondenceId", stats."Status") > (...)` | ||
| - PostgreSQL must join FIRST, then filter cursor | ||
| - Cannot use index for cursor seek | ||
| - Result: Slow! ❌ | ||
|
|
||
| 3. **Fixed queries (cursor on indexed table)**: Cursor references indexed table | ||
| - `WHERE a2Events."Status" = 4 AND (a2Events."CorrespondenceId", a2Events."Status") > (...)` | ||
| - PostgreSQL can use index for cursor seek | ||
| - Result: Fast! ✅ | ||
|
|
||
| ## Lesson Learned | ||
|
|
||
| **When using cursor pagination with helper tables:** | ||
| 1. ✅ Always reference the **driving table** in cursor comparison | ||
| 2. ✅ Ensure cursor columns **match index columns** | ||
| 3. ✅ Keep ORDER BY consistent with cursor comparison | ||
| 4. ❌ Don't reference joined table columns in cursor (breaks index usage) | ||
|
|
||
| ## Technical Explanation | ||
|
|
||
| ### Index Seek vs Index Scan + Filter | ||
|
|
||
| **With correct cursor (a2Events columns)**: | ||
| ``` | ||
| Index Seek on IX_A2Iss1716A2Events_CorrId_Status_Party | ||
| WHERE Status = 4 | ||
| AND (CorrespondenceId, Status) > ('0197f333...', 4) | ||
| → Directly seeks to position in index (fast) | ||
| → Returns next 1000 rows | ||
| ``` | ||
|
|
||
| **With broken cursor (stats columns)**: | ||
| ``` | ||
| Index Scan on IX_A2Iss1716A2Events_CorrId_Status_Party | ||
| WHERE Status = 4 | ||
| → Scans all matching rows | ||
| → Joins with CorrespondenceStatuses | ||
| → Filters: (stats.CorrespondenceId, stats.Status) > (...) | ||
| → Cannot use index for cursor filtering (slow) | ||
| ``` | ||
|
|
||
| The difference: **Index Seek (instant) vs Full Scan + Join + Filter (19 seconds)** | ||
|
|
||
| ## Summary | ||
|
|
||
| ✅ **Fixed**: Changed cursor comparison from `stats.*` to `a2Events.*` | ||
| ✅ **Fixed**: Changed ORDER BY from `stats.*` to `a2Events.*` | ||
| ✅ **Impact**: 55 hours → 58 minutes (57x faster) | ||
| ✅ **Test**: Run `.\test-export-2.ps1` to verify | ||
| ✅ **Expected**: Both batches ~250-650ms (consistent performance) | ||
|
|
||
| This was a **critical performance bug** that would have made the full export impractical. Now fixed! 🎉 | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.