Skip to main content

The Soul of the Machine: OMGT Performance Debate

Topic: Strategic Action Plan: OMGT Performance Stabilization

Participants:

  • 🏛️ The Architect: Guardian of structure and long-term maintainability.
  • 🏺 The Archeologist: Keeper of historical context and "why things are the way they are".
  • 🎖️ The Veteran: Pragmatic survivor of past deployments; focused on risk.
  • 🔨 The Builder: Focused on execution and solving the immediate pain.
  • ⚖️ The Critic: The skeptic; demands proof and identifies logical gaps.

Round 1: The Search Optimization (Stream 1)​

🔨 Builder: "Let's be real. The search code is a mess. That loop iterating through Postcode ranges and doing AddRange in memory? It's indefensible. Changing that to a single SQL query is a quick win. And those .Join() calls? Redundant. The View already has the data. I can strip 50% of that query builder code."

🎖️ Veteran: "Slow down, kid. You say 'redundant joins', but have you checked why they are there? Sometimes the View OrderTravelProductSearchViewList uses INNER JOIN while the code manual join might be doing a left join or filtering subtly differently. If you blindly remove them, you might accidentally filter out valid records that have incomplete data (e.g., missing Order Header)."

🏛️ Architect: "I agree with Veteran. Also, relying heavily on a Database View (...SearchViewList) couples our application logic tightly to the SQL schema. However, the current state of OrderAdvancedSearchQueryBuilder (God Class) is worse. My concern is the 'Mitigation B' (Any vs Distinct). If we remove .Distinct(), we trust the join logic perfectly. But OrderTravelProduct is the child of Order. If the View joins Order -> OrderTravelProduct, we get one row per product. If we filter on Order.Customer.Name, we still get one row per product. But if we join Order -> Payments, we get duplicates. Are we sure the View doesn't join on 1:N relations?"

🏺 Archeologist: "Actually, the OrderAdvancedSearchQueryBuilder was written that way because the View was originally much simpler. The devs added manual joins as requirements grew because they didn't have permission to alter the SQL View. Now the View has caught up, but the code remains. So Builder is likely right about redundancy, but Veteran's fear of 'Inner vs Left' join mismatches is the real ghost here."

⚖️ Critic: "You're all missing the biggest risk. The Postcode 'fix'. You want to turn a loop of ranges into a single LINQ predicate. (Zip >= 1000 AND Zip <= 2000) OR (Zip >= 3000 AND ...) . If a user selects 50 different remote ranges, that SQL query becomes a monster. Have we tested the limit of the WHERE clause length in the target SQL Server version? It might crash the query planner just as hard as the loop crashes memory."

🔨 Builder: "Fair. But the loop crashes the server memory. The SQL query crashes... what? milliseconds? I'd take the SQL complexity over fetching 50k rows into RAM any day."

Round 2: The Export Optimization (Stream 2)​

🔨 Builder: "Now, the Export. It's CPU bound. Reflection everywhere. EITTextTemplate for every single cell of every single row. I want to build a 'Fast Path'. If the column is just 'Customer Name', we read otp.Customer.Name directly. No reflection, no parsing. Boom, 10x speed."

🏛️ Architect: "Objection. You are proposing to fork the export logic. You'll have the 'Legacy Slow Template Path' for complex stuff, and a 'Hardcoded Fast Path' for simple stuff. What happens when a business user changes the 'Customer Name' column to include a conditional formatting rules? Your Fast Path breaks or ignores it. You are violating the 'Single Source of Truth' for export definitions."

🏺 Archeologist: "Architect is right. The entire MassActionHandler infrastructure was built to be 'Configurable-First'. The logic lives in the Database (Template patterns). If you hardcode 'Fast Path' in C#, you kill the ability for admins to tweak exports without a deployment."

⚖️ Critic: "I have a bigger problem. Builder, you said the bottleneck is 'Template Resolution'. Prove it. Look at the code MassActionHandler.cs: List<OrderTravelProductOMGT> selected = _dataCtx.OrderTravelProductOMGTs.Where(...).ToList(); It fetches the main entity. It does not include Customer, OrderHeader, or Payments. Then, inside the loop: otp.OrderOMGT.OrderHeader.Customer.FirstName. That is a database query. Per row. Per relationship. If you export 1000 orders, you are executing 1000 queries for Order, 1000 for Customer, 1000 for Payments... Your 'Fast Path' saves CPU cycles on reflection, but the network I/O from 3,000+ database calls will still make the export take forever. You are optimizing the engine while the car has no gas."

🎖️ Veteran: "Ouch. Critic nailed it. That's the classic N+1 Select death spiral. We see it every time someone writes a 'Mass Action' that reuses logic meant for a 'Single Item' view."

🔨 Builder: "Okay... so we need Include()? But the query is dynamic! We don't know what we need to include until we parse the template!"

🏛️ Architect: "Which reinforces my point. The architecture is flawed. We are trying to use an ORM for a Bulk Reporting task. We should project to a DTO before the loop. But since the DTO structure is dynamic (defined by the user's template), we are stuck."

Round 3: The Combinatorial Explosion (The "1200 Line" Revelation)​

⚖️ Critic: "Wait. I just pulled the metrics on the input DTO. SearchOrderOMGTDto is 1,227 lines long. It handles roughly 30 distinct filter categories—from basic Global info to nested School, Travel, and Accounting details. Builder, you're talking about refactoring a single method, but this is a combinatorial explosion problem. There are $2^30$ possible ways a user can combine these filters."

🏛️ Architect: "That is astronomical. It explains why the SQL execution plans are so unstable. If the OrderAdvancedSearchQueryBuilder is just chaining IF statements for 30 categories, the resulting LINQ-to-SQL tree must be a nightmare for the optimizer. One category might add a Join that 'poisons' the filter efficiency of another."

🎖️ Veteran: "I've seen this before. Usually, a dev adds a filter for 'Flights' and joins the Travel table. Then another dev adds 'Invoices' and joins Accounting. If a user filters for both, they get a cross-join if the internal logic doesn't bridge them correctly. In this system, we rely on the OrderTravelProductSearchViewList to solve some of this, but it can't solve everything."

🔨 Builder: "30 categories... Okay, my plan to just 'Refactor' FinaliseQuery is just the tip of the iceberg. If we have 30 categories, we can't just keep adding .Join() or even .Any(). We need a strategy to 'Prune' the query. If a user only selects 'Customer', we shouldn't even touch the OrderTravelProduct details if possible."

🏺 Archeologist: "But we always start from OrderTravelProduct because the grid returns products. The problem is that the OrderAdvancedSearchQueryBuilder is a monolithic 5,700-line beast trying to be everything to everyone. It's not just an anti-pattern; it's a structural failure under the weight of its own requirements."

Round 4: The Holistic Review (UX & Out-of-the-Box)​

🏺 Archeologist: "We are fixing the engine, but are we fixing the journey? We are preserving the '30 categories' because they exist, but do users need them all visible at once?"

🎖️ Veteran: "Probably not. Most agents search by Name, Date, or School. The 'Compliance' filter is likely used by 2 people in the Back Office once a month."

🏛️ Architect: "Agreed. We should implement Progressive Disclosure (Stream 3). Show the top 5 fields by default. Hide the rest behind an 'Advanced' accordion. This isn't just UX; it's performance defense. Using 'hidden' fields discourages their accidental use."

⚖️ Critic: "What about the export? We are obsessing over making it fast. But even 5 seconds is too long for a synchronous web request. The browser spins, the user refreshes, and we get a double query string."

💡 Idea: "Why don't we just make the Export Asynchronous? The user clicks 'Export', we return 'Your download will be ready in a moment', and we send it via email or a notification bell. Then we don't care if it takes 30 seconds."

🔨 Builder: "Async export? That requires a background worker, a queue, a notification system. Do we have that infrastructure? Or are we building a space shuttle to cross the street?"

🏛️ Architect: "We have Task.Run (risky) or Hangfire (if installed). But Builder is right, that's a scope creep. Let's stick to the 'Data Prefetching' fix first. If that brings it under 5s, we are good. If not, we pivot to Async."

Verdict & Recommendations​

1. The Search Strategy (Rounds 1 & 3)​

  • Approved: Refactoring Postcode loop is mandatory. The WHERE clause limit risk is acceptable compared to the current OOM risk.
  • New Directive: Query Pruning: Shift to "Demand-Driven" queries. If a filter category is not selected, the query builder must guarantee that no related tables/joins are added to the LINQ structure.
  • Short-term Guardrail: Implement a "Complexity Budget" warning if a user selects more than 5 high-impact categories simultaneously.
  • Structural Debt: Refactor OrderAdvancedSearchQueryBuilder (5.7k LOC) into isolated Filter Strategy classes.
  • Caution: Removing manual Joins requires line-by-line comparison of "Inner vs Left" semantics against the View.
  • Denied: Removing .Distinct() is too risky for now; keep it but optimize the WHERE tree first.

2. The Export Strategy (Round 2)​

  • Rejected: The "Fast Path" hardcoding (Mitigation C) is dangerous and creates maintenance debt.
  • Pivot: Bottleneck is Data Access (N+1), not Template Resolution.
  • New Directive: Data Prefetching:
    • Parse Template once at start to identify required relationships.
    • Execute a bulk loading query with .Include() for the current batch.
    • Feed pre-hydrated entities to the template engine.
  • Plan B: If prefetching doesn't hit performance targets, migrate to Asynchronous Export (Round 4).

3. The UX Strategy (Round 4)​

  • New Directive: Progressive Disclosure: Show only the "Top 5" categories by default. Hide low-usage/high-complexity filters (e.g., Compliance, PEC) behind an "Advanced" accordion.
  • Telemetry: Use the Search Usage Analytics plan to identify the "Top 5" fields.

Action Plan Status: ⚠️ REQUIRES REVISION (Focus on Data Access for Export and Pruning for Search).