Skip to content

9211 csv import perf fix 3 - #6483

Open
ttoomey wants to merge 52 commits into
productionfrom
9211-csv-import-perf-fix-3
Open

9211 csv import perf fix 3#6483
ttoomey wants to merge 52 commits into
productionfrom
9211-csv-import-perf-fix-3

Conversation

@ttoomey

@ttoomey ttoomey commented May 17, 2026

Copy link
Copy Markdown
Contributor

Merging this PR

  • use the squash-merge strategy for PRs targeting a release-X branch
    Here's the PR summary:

Description

Improve process_existing query performance by materializing in-scope warehouse rows into a shared temp table and replacing correlated Arel EXISTS subqueries with indexed INNER JOINs.

  • process_existing orchestration: Replace three independent methods (mark_unchanged, mark_incoming_older, apply_updates) with a shared temp table pattern (with_delete_pending_rows) that evaluates existing_destination_data_scope once instead of three times. Each sub-pass drains resolved rows from the temp table so subsequent passes operate on a shrinking set.
  • Unchanged / incoming-older detection: Replace Arel-built correlated EXISTS subqueries against the full warehouse table with raw SQL INNER JOINs against the small indexed temp table (unchanged_ids_sql, incoming_older_ids_sql). Results are materialized into an inner temp table to avoid re-running unindexed staging joins per batch.
  • apply_updates: Paginate through the shared temp table via keyset pagination instead of re-querying existing_destination_data_scope with ActiveRecord in_batches.
  • Guard clause hoisting: Move ExportID and custom_augmentation? early returns from individual methods into process_existing, avoiding temp table creation for skipped file types.
  • Removed methods: mark_unchanged, mark_incoming_older, local_date_cast_arel, batch_clear_pending_deletion — replaced by with_delete_pending_rows, unchanged_ids_sql, incoming_older_ids_sql, resolve_matched_rows.
  • Comment cleanup: Fix typos, update class-level and method-level comments to reflect the new architecture.

Type of Change

Refactor

Checklist before requesting review

  • I have performed a self-review of my code
  • I have run the code that is being changed under ideal conditions, and it doesn't fail
  • I have updated the documentation (or not applicable)
  • I have added spec tests (or not applicable)
  • I have provided testing instructions in this PR or the related issue (or not applicable)

gigxz and others added 2 commits May 15, 2026 09:49
* Fix deadlocks between concurrent cohort_clients writers by coordinating all three write paths behind a per-cohort advisory lock.
* Also adds more test coc's, our factories are exhausting them
@ttoomey
ttoomey marked this pull request as draft May 17, 2026 00:47
ttoomey and others added 17 commits May 18, 2026 10:21
Adds a rails hmis_csv:qa:compare[DS_A_ID,DS_B_ID] rake task to diff two independently-imported data sources table-by-table.
* Log and expose threshold crossing emails

* Include scenarios to help reason about threshold monitoring; Fix and expand notes on tables

* Hand role path

* Update hints to better describe valid values; edge case tests

* Threshold monitoring display and collection fixes; regression tests and tests for calculator run class

* Test fixes

* Test fixes
- Retires LSA generators for FY2018–FY2024. Deletes the pre-driver FY2018/2019/2021 code entirely.
- Extracts `db_up.rb` so `Rds#wait_for_database!` no longer depends on a retired FY's SQL Server models.
- Fixes the `lsa_shut_down` rake task guard, which was querying the legacy `Report`/`ReportResult` tables.
- Fixes `hic?` string/integer comparison bug via `.to_i` coercion and adds a `Rds` stub class so environments without AWS credentials don't crash.
- Adds spec coverage for the FY2026 generator (state machine, scopes, preflight, filter, date ranges).

---------

Co-authored-by: Dave Greiner <dgreiner@greenriver.org>
…port (#6488)

* fix export money fields with two decimal places in FY2026 HMIS CSV export

* Move money formatting to ExportConcern#process by converting the AR row to a plain hash before rounding, removing the post-write formatting from CsvDestination

* Guard against double conversion in ExportConcern#process for custom exporters that convert row to a plain hash inside adjust_keys
When the form fails validation, the Project Types dropdown is populated with the generator's allowed project type codes
* use columns instead of papertrail for assmt fields

* respond to PR feedback
* preserve original IDs and round-trip jsonb columns correctly in report archival reload

* add tests
Update main with hotfixes merged into staging
Release-214: Main to Staging
@ttoomey

ttoomey commented May 21, 2026

Copy link
Copy Markdown
Contributor Author
Helpful AI artifact # CSV Importer Refactor: `process_existing` (commit `73ec1cd`)

Context: Where process_existing sits in the pipeline

By the time process_existing runs, two earlier passes have already executed:

Pass Method Effect
0 mark_tree_as_dead Sets pending_date_deleted on all in-scope warehouse rows ("guilty until proven innocent")
1 add_new_data Upserts staging rows whose hud_key has no match in the warehouse (truly new records)

State on entry to process_existing: Every in-scope warehouse row still has pending_date_deleted set. The rows that were brand new have been inserted. What remains are warehouse rows that do have a matching key in the staging data — process_existing must decide what to do with each one.

State after process_existing: Every warehouse row that should be kept has had its pending_date_deleted cleared. Updated rows have been overwritten from staging. The only rows with pending_date_deleted still set are ones the CSV no longer contains — those get soft-deleted by the subsequent remove_pending_deletes pass.

There are three possible dispositions for each matched row:

Disposition Condition Action
Unchanged source_hash in staging matches warehouse Clear pending_date_deleted — no data update needed
Incoming older Staging DateUpdated < warehouse DateUpdated (by local-tz date) Clear pending_date_deleted — keep the newer warehouse version
Genuine update Everything else (hash mismatch, incoming same or newer) Overwrite warehouse from staging, clear pending_date_deleted, mark dirty flags

1) The original flow (before the commit)

process_existing called three independent methods sequentially, each of which independently queried existing_destination_data_scope(klass) — the ActiveRecord scope representing in-scope, delete-pending warehouse rows.

Methods

mark_unchanged(klass, file_name)

  • Built an Arel EXISTS subquery: correlated the staging table with the warehouse table on hud_key AND source_hash equality.
  • Used existing_destination_data_scope(klass).where(exists) to find matching warehouse rows.
  • Called batch_clear_pending_deletion which materialized those IDs into a temp table and batch-updated pending_date_deleted = NULL.

mark_incoming_older(klass, file_name)

  • Same pattern, but the EXISTS subquery compared DateUpdated (with timezone-aware date casting via the local_date_cast_arel helper) instead of source_hash.
  • Again called batch_clear_pending_deletion on the matched scope.

apply_updates(klass, file_name)

  • Iterated existing_destination_data_scope(klass).in_batches(of: SELECT_BATCH_SIZE).
  • For each batch, plucked the hud_key values, looked up matching staging rows, transformed them via prepare_destination_for_update, and upserted.
  • Also handled ExportID early-return and custom augmentation dispatch internally.

Helper methods

batch_clear_pending_deletion(klass, matched_scope, file_name)

  • Materialized matched_scope IDs into a temp table.
  • Batch-updated pending_date_deleted = NULL via keyset pagination on the temp table.

local_date_cast_arel(conn, arel_column)

  • Built an Arel node for CAST(col AT TIME ZONE 'UTC' AT TIME ZONE '<local>' AS DATE).
flowchart TD
    subgraph PE["process_existing (per file)"]
        direction TB
        A["mark_unchanged"]
        B["mark_incoming_older"]
        C["apply_updates"]
        A --> B --> C
    end

    subgraph MU["mark_unchanged"]
        direction TB
        A1["Evaluate existing_destination_data_scope<br/>(query #1 against warehouse)"]
        A2["Build Arel EXISTS subquery:<br/>staging.hud_key = wh.hud_key<br/>AND staging.source_hash = wh.source_hash"]
        A3["batch_clear_pending_deletion<br/>↳ materialize IDs into temp table<br/>↳ batch UPDATE<br/>  pending_date_deleted = NULL<br/>↳ drop temp table"]
        A1 --> A2 --> A3
    end

    subgraph MIO["mark_incoming_older"]
        direction TB
        B1["Evaluate existing_destination_data_scope<br/>(query #2 against warehouse)"]
        B2["Build Arel EXISTS subquery:<br/>staging.hud_key = wh.hud_key<br/>AND staging.DateUpdated<br/>  < wh.DateUpdated"]
        B3["batch_clear_pending_deletion<br/>↳ materialize IDs into temp table<br/>↳ batch UPDATE<br/>  pending_date_deleted = NULL<br/>↳ drop temp table"]
        B1 --> B2 --> B3
    end

    subgraph AU["apply_updates"]
        direction TB
        C1["Evaluate existing_destination_data_scope<br/>(query #3 against warehouse)"]
        C2["in_batches → pluck hud_keys<br/>↳ find staging rows<br/>↳ prepare_destination_for_update<br/>↳ upsert"]
        C1 --> C2
    end

    %% Connect summary to details to align columns
    A -.-> A1
    B -.-> B1
    C -.-> C1

    %% Force subgraphs to stack vertically 
    A3 ---> B1
    B3 ---> C1
Loading

Key characteristics

  1. existing_destination_data_scope is evaluated 3 times. Each call produces a query that scans/filters the warehouse table by data_source_id, project_ids, date_range, and pending_date_deleted.

  2. Correlated EXISTS subqueries. mark_unchanged and mark_incoming_older use WHERE EXISTS (SELECT 1 FROM staging WHERE staging.hud_key = wh.hud_key AND ...). These are semi-joins — for each warehouse row, Postgres must probe the staging table. On large tables, the planner may choose a nested-loop strategy that's expensive.

  3. Each temp table is independent. batch_clear_pending_deletion creates a one-shot temp table, uses it, and drops it. There's no sharing between passes — each pass starts from scratch.

  4. No pruning between passes. After mark_unchanged clears pending_date_deleted on some rows, those rows may naturally fall out of existing_destination_data_scope for the next query (if the scope filters on pending_date_deleted IS NOT NULL). But the scope itself must still be re-evaluated — Postgres must re-scan to discover which rows remain.


2) How the original flow served its purpose

Despite the performance concerns, the original flow was correct:

  • Ordering guarantee: Unchanged runs first, so hash-matched rows never reach the date comparison or update logic. Incoming-older runs second, so only hash-mismatched rows get date-compared. Apply_updates runs last and only sees rows that failed both prior checks.

  • Independence: Each method was self-contained with its own scope evaluation and temp table lifecycle. This made each method easy to reason about in isolation.

  • Correctness of the semi-joins: The Arel EXISTS subqueries correctly expressed "there exists a staging row with matching hud_key and matching source_hash" (or date condition). NULL source_hash on the warehouse side correctly failed the equality check, routing those rows through to apply_updates.

The trade-off was performance: three full scope evaluations and three rounds of correlated subquery execution, with no shared intermediate state between passes.


3) The new flow (after the commit)

The refactor replaces the three independent methods with a shared temp table pattern and progressive draining.

New orchestration in process_existing

with_delete_pending_rows(conn, klass, file_name) do |pending_rows|
  # Sub-pass 1: unchanged
  resolve_matched_rows(conn, klass, ..., unchanged_ids_sql(..., pending_rows), pending_rows, label: 'unchanged')

  # Sub-pass 2: incoming_older (skipped if most recent export)
  resolve_matched_rows(conn, klass, ..., incoming_older_ids_sql(..., pending_rows), pending_rows, label: 'incoming_older')

  # Sub-pass 3: apply_updates (uses the now-drained pending_rows directly)
  apply_updates(klass, file_name, pending_rows, conn: conn)
end

Early returns for ExportID and custom_augmentation? are hoisted into process_existing itself, before the temp table is created.

New methods

with_delete_pending_rows(conn, klass, file_name) — single materialization

  • Evaluates existing_destination_data_scope(klass) once.
  • Materializes the result into a temp table with four columns: id, hud_key, source_hash, DateUpdated.
  • Creates indexes on id and hud_key.
  • Yields the quoted temp table name. Drops it in ensure.

unchanged_ids_sql(klass, conn, pending_rows) — raw SQL, no Arel

  • Returns a SQL string: SELECT DISTINCT t.id FROM <pending_rows> t INNER JOIN <staging> s ON s.hud_key = t.hud_key AND s.source_hash = t.source_hash WHERE ...
  • This is a regular INNER JOIN on the small, indexed temp table — not a correlated EXISTS against the full warehouse table.

incoming_older_ids_sql(klass, conn, pending_rows) — raw SQL, no Arel

  • Same pattern but with the timezone-aware date comparison inlined as raw SQL (replacing the local_date_cast_arel Arel helper).
  • Joins temp table to staging on hud_key, filters where staging date < temp table date.

resolve_matched_rows(conn, klass, ..., matched_sql, pending_rows, label:) — replaces batch_clear_pending_deletion

  • Materializes the matched IDs (from the SQL string) into an inner temp table.
  • Batch-updates the warehouse to clear pending_date_deleted (same keyset pagination pattern as before).
  • New step: Deletes resolved rows from the shared pending_rows temp table: DELETE FROM <pending_rows> WHERE id IN (SELECT id FROM <inner>).
  • Drops the inner temp table in ensure.

apply_updates(klass, file_name, pending_rows, conn:) — now paginating from the temp table

  • Instead of re-querying existing_destination_data_scope(klass).in_batches(...), paginates through the shared temp table: SELECT id, hud_key FROM <pending_rows> WHERE id > last_id ORDER BY id LIMIT batch_size.
  • For each batch of hud_key values, looks up staging rows, transforms, and upserts (same logic as before).
flowchart TD
    subgraph PE["process_existing (per file)"]
        direction TB

        M["with_delete_pending_rows\nEvaluate existing_destination_data_scope ONCE\n→ CREATE TEMP TABLE with id, hud_key, source_hash, DateUpdated\n→ CREATE INDEX on id, hud_key"]

        subgraph PASS1["Sub-pass 1: unchanged"]
            U1["unchanged_ids_sql\nJOIN temp ↔ staging ON hud_key + source_hash"]
            U2["resolve_matched_rows\n→ materialize matched IDs into inner temp\n→ batch UPDATE wh SET pending_date_deleted = NULL\n→ DELETE matched rows from shared temp"]
            U1 --> U2
        end

        subgraph PASS2["Sub-pass 2: incoming_older"]
            IO1["incoming_older_ids_sql\nJOIN temp ↔ staging ON hud_key\nWHERE staging.date < temp.date"]
            IO2["resolve_matched_rows\n→ materialize matched IDs into inner temp\n→ batch UPDATE wh SET pending_date_deleted = NULL\n→ DELETE matched rows from shared temp"]
            IO1 --> IO2
        end

        subgraph PASS3["Sub-pass 3: apply_updates"]
            AU1["Paginate remaining rows in shared temp\nSELECT id, hud_key WHERE id > last_id"]
            AU2["For each batch of hud_keys:\nfind staging rows → prepare → upsert"]
            AU1 --> AU2
        end

        M --> PASS1 --> PASS2 --> PASS3

        DROP["DROP shared temp table (ensure)"]
        PASS3 --> DROP
    end
Loading

What changed (summary)

Aspect Before After
Scope evaluation existing_destination_data_scope queried 3 times (once per method) Queried once, materialized into a shared temp table
Join strategy Correlated EXISTS subqueries (Arel semi-joins against full warehouse table) INNER JOIN against small, indexed temp table
Inter-pass coordination None — each pass independently re-queries the warehouse Progressive draining — resolved rows are deleted from the shared temp table, so each subsequent pass operates on a shrinking set
apply_updates iteration existing_destination_data_scope.in_batches (ActiveRecord batching against warehouse) Keyset pagination on the shared temp table (WHERE id > last_id)
Guard clauses Each method individually checked ExportID / custom_augmentation? Hoisted into process_existing before temp table creation
Arel vs raw SQL mark_unchanged and mark_incoming_older used Arel for EXISTS + date casting unchanged_ids_sql and incoming_older_ids_sql use hand-written SQL strings
Removed methods mark_unchanged, mark_incoming_older, local_date_cast_arel, batch_clear_pending_deletion
Added methods with_delete_pending_rows, unchanged_ids_sql, incoming_older_ids_sql, resolve_matched_rows

Where the performance win comes from

  1. Single scope evaluation. The existing_destination_data_scope query — which filters a potentially very large warehouse table by data_source_id, project_ids, date_range, and pending_date_deleted — runs once instead of three times.

  2. Temp-table JOINs are cheaper. The unchanged and incoming-older checks now join the staging table against a small, indexed temp table rather than executing correlated EXISTS subqueries against the full warehouse table. Postgres can use hash joins or merge joins on the temp table.

  3. Progressive draining reduces work. Each sub-pass deletes its resolved rows from the shared temp table. If 90% of rows are unchanged (common in steady-state imports), the incoming-older pass only evaluates the remaining 10%, and apply_updates only iterates the small residual.

  4. Keyset pagination on temp table. apply_updates no longer uses ActiveRecord's in_batches (which can be unpredictable with large offset-based pagination). Instead it uses simple WHERE id > last_id ORDER BY id LIMIT N on the temp table — fast with an index.

@ttoomey
ttoomey force-pushed the 9211-csv-import-perf-fix-3 branch 3 times, most recently from 04ee986 to f8ad943 Compare May 22, 2026 01:51
@ttoomey
ttoomey changed the base branch from main to staging May 22, 2026 03:57
@ttoomey
ttoomey requested review from dtgreiner and eanders May 22, 2026 03:58
@ttoomey
ttoomey marked this pull request as ready for review May 22, 2026 04:05
martha and others added 2 commits May 22, 2026 07:47
* Add basic filter for CE steps assigned to you

* omit assigned_to_user from schema for now
…6506)

* fix: prevent nil employment translator values rendering as ()

* exclude enrollments without an in-range entry or exit from employment DQ checks
@ttoomey
ttoomey force-pushed the 9211-csv-import-perf-fix-3 branch from f30bfc4 to a8435e6 Compare May 22, 2026 23:53
@ttoomey
ttoomey changed the base branch from staging to production May 22, 2026 23:53
ttoomey and others added 28 commits May 25, 2026 12:00
These keys expire and could cause issues with s3 access
* Update AssessmentAccess and deprecate unused fields

* Use new permission field canDelete

* resolve canDeleteAssessment instead of canDelete
* add basic demo workflow

* update with feedback

* self-review

* move ce demo step forms into default dir

* add data_source to delete_template_and_associated_data

* rename demo => standard

* add workflow template readme and fix mermaid generation

* fix quoting to avoid unneeded change

* use force_recreate flag more like PH

* update readme

* fix sys test failure
* feat: add release tag to system status and non-production header

* remove git release info from dev environment
When archival purge fails, record purge_failed_at and purge_failure_reason
in archival_metadata so the report is excluded from future purge_eligible
scopes and won't be retried indefinitely.
…6530)

Replaces the two separate `archive_and_purge_simple_reports` and
`archive_and_purge_hud_reports` tasks with the combined
`archive_and_purge_eligible` task, removing the non-production-only
trigger now that the unified task is ready for production.
* Add structured expression translator

* Simplify

* simplify and add comments

* add clarifying comment

Co-authored-by: Gig <gig@greenriver.org>

* replace struct with data

---------

Co-authored-by: Gig <gig@greenriver.org>
Adjust destination bucketing on the Outcomes report to classify homeless exits in a dedicated bucket instead of "other or unknown outcome"
Add configurable daily-rate-based leasing cost computation for HOPWA CAPER FBH sheets.
* better handling for ActiveStorage::FileNotFoundError in AnalyzeJob
* ensure we aren't prematurely deleting the s3 object on soft-delete of files
* Avoids creating orphan s3 objects in image cache
* Cleanup file soft-deletion into a single method
* Add PurgeSoftDeletedClientFilesJob to clean up soft-deleted files
* add RuleChangeImpactCalculator

* Add CeMatchExpressionValidator

* AI review

* memoize for performance

* update rule impact calculator for updates

* add comments

* only check impact for unit groups with ce waitlists enabled

* add unit_groups assoc to owners

* update comments

* fix n+1

* move length validation to the model
* Add notes field to project groups

Adds a free-text notes column to project groups, surfaced in the edit
form, index table, and Excel download. Also exposes notes through the
analytics view for downstream Superset reporting.

* Render project group notes as markdown

Adds GrdaWarehouse::ProjectGroup#markdown_notes which renders the notes
field through Redcarpet with filter_html: true to strip raw HTML from
user-entered content before marking output safe.
PR #5947 removed the hmis_csv_twenty_twenty project extension to stop
convert_to_aggregated! from shadowing the unversioned importer version.
That also dropped imported_items_2020/loaded_items_2020, causing
NoMethodError on Source Data for projects. Re-add those associations
only; aggregate conversion stays on hmis_csv_importer. Add model specs
for FY2020 wiring and unversioned convert_to_aggregated! behavior.
…6545)

Pre-filter indexes by relpages in the innermost subquery so indexes
that cannot exceed SIZE_CUTOFF bytes of bloat are excluded before
expensive per-index statistics are computed. Also fix typo in the
MIN_PCT_NOT_ANALZYED constant name.
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.

5 participants