[
https://issues.apache.org/jira/browse/FINERACT-2668?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18092783#comment-18092783
]
Farooq Ayoade commented on FINERACT-2668:
-----------------------------------------
[https://github.com/apache/fineract/pull/6071]
PR
> Bulk-import download templates load the entire tenant (every client and every
> account) into the .xls — O(tenant) memory and file size for an
> O(rows-entered) operation; can OOM the server
> ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>
> Key: FINERACT-2668
> URL: https://issues.apache.org/jira/browse/FINERACT-2668
> Project: Apache Fineract
> Issue Type: Improvement
> Components: DataImportTool
> Reporter: Farooq Ayoade
> Priority: Major
>
> h3. Observed behavior
> Requesting a bulk-import *download template* for a transaction-type entity
> (e.g. loan repayment) builds an Excel workbook that embeds the {*}entire
> tenant{*}:
> * a hidden *{{Clients}}* sheet containing *every client* in the tenant (one
> row each),
> * a hidden *{{Offices}}* sheet with every office,
> * the data sheet itself carries lookup columns holding *every active
> account* in the tenant, plus a {{VLOOKUP}} array formula pre-written into a
> *hard-coded 3000 rows × 5 columns* block.
> The underlying fetches are {*}unbounded{*}. For loan repayment
> ({{{}BulkImportWorkbookPopulatorServiceImpl.populateLoanRepaymentWorkbook{}}}):
> {{List<ClientData> clients = fetchClients(officeId); // officeId
> null ⇒ retrieveAll(null), no LIMIT
> List<LoanAccountData> loans = fetchLoanAccounts(officeId); // officeId
> null ⇒ retrieveAll(null), no LIMIT}}
> {{ClientReadPlatformServiceImpl.retrieveAll(SearchParameters)}} only appends
> a {{LIMIT}} clause when search parameters request one; called with {{null}}
> it returns {*}all rows{*}. The result set is then materialised as POI
> {{HSSFCell}} objects in heap.
> On a tenant with, say, 100k clients an HQ-scoped user downloading a "simple
> repayment" template pulls 100k client rows + every active loan into memory
> and into a multi-megabyte {{{}.xls{}}}. Multiple concurrent downloads
> multiply the heap cost and can OOM the instance.
> h3. Why this is wasteful — the importer never reads the loaded data
> The {{Clients}} and {{Offices}} sheets exist *only* to drive Excel's
> cascading data-validation dropdowns (Office → Client → Account) during manual
> entry. The {*}import handler does not read them{*}.
> {{LoanRepaymentImportHandler.readLoanRepayment(...)}} resolves the loan
> purely from the typed account number:
> {{String loanaccountInfo =
> ImportHandlerUtils.readAsString(LoanRepaymentConstants.LOAN_ACCOUNT_NO_COL,
> row);
> loanAccountId =
> loanReadPlatformService.retrieveLoanIdByAccountNumber(loanAccountAr.get(0));}}
> So the server already does an authoritative server-side lookup on upload. The
> entire embedded client/office dataset is a data-entry convenience that costs
> O(tenant) to build and is discarded at import time. (Only the small
> {{Extras}} sheet — payment types — is read back, via {{{}getIdByName{}}}.)
> h3. Expected behavior
> * A download template must not be O(tenant size). Lookup data must be
> *bounded* and/or *scoped* (by office hierarchy), and {*}omittable{*}.
> * When a tenant exceeds a configurable lookup threshold, the template should
> *gracefully degrade* to a lean form (plain typed columns, no embedded
> client/office dataset, no 3000-row VLOOKUP block), relying on the server-side
> validation that already runs on upload.
> * Existing callers that depend on the dropdown UX keep working (backward
> compatible by default).
> h3. Steps to reproduce
> # On a tenant with a large client/loan base (or seed several thousand
> clients), authenticate as a user whose office is the head office.
> # {{GET /fineract-provider/api/v1/loans/repayments/downloadtemplate}} (no
> {{{}officeId{}}}).
> # Observe: response is a multi-MB {{{}.xls{}}}; the {{Clients}} sheet
> contains every client in the tenant; opening the file in Excel is slow
> (15,000 pre-baked VLOOKUP formulas recalc). Under concurrent requests, watch
> server heap.
> h3. Root cause
> # *Unbounded fetch.* {{fetchClients(null)}} / {{fetchLoanAccounts(null)}}
> call {{{}retrieveAll(null){}}}, which applies no {{{}LIMIT{}}}.
> # *Lookup sheets are mandatory and tenant-wide.* {{ClientSheetPopulator}} /
> {{OfficeSheetPopulator}} are always built and always span the whole
> (office-hierarchy-visible) tenant, even though the importer ignores them.
> # *Hard-coded 3000-row formula block.* Each transaction populator's
> {{setDefaults(...)}} writes {{VLOOKUP}} formulas into rows 1..3000 regardless
> of how many rows the user will actually enter.
> This pattern is shared across the transaction-type templates (loan repayment,
> guarantor, savings/recurring/ fixed-deposit transactions, shared accounts,
> …), so the fix belongs in the shared populator path, not in any single
> handler.
> h3. Proposed fix
> Backward-compatible, opt-out-of-bloat design (default behaviour unchanged for
> small tenants):
> # *Bound every lookup fetch.* Introduce a configurable cap (e.g.
> {{{}fineract.bulkimport.template.max-lookup-rows{}}}, default ~10,000).
> {{fetchClients}} / {{fetchLoanAccounts}} request at most that many rows;
> honour {{officeId}} scoping that already exists on the endpoint.
> # *Make lookup sheets optional.* Add {{?includeLookups=\{true|false}}} to
> the download endpoints (default {{{}true{}}}). When {{{}false{}}}, skip
> {{ClientSheetPopulator}} / {{OfficeSheetPopulator}} and the in-sheet VLOOKUP
> cascade; emit a lean sheet of plain typed columns ({{{}Loan Account No.*{}}},
> {{{}Amount Repaid*{}}}, {{{}Date*{}}}, {{{}Type*{}}}, payment-detail). Upload
> already validates the account number server-side.
> # *Auto-degrade.* If the bounded count would exceed the cap, automatically
> fall back to the lean template and write a note cell explaining that
> dropdowns were omitted and the account number must be typed.
> # *Stop hard-coding 3000 rows.* Replace the fixed 1..3000 VLOOKUP block with
> a small configurable default (or column/table references), and omit it
> entirely in lean mode.
> Pilot the change on the *loan-repayment* template (smallest, highest-traffic
> reproducer); the shared-path changes then extend to the other transaction
> templates in follow-ups.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
