AshharAhmadKhan commented on PR #6494: URL: https://github.com/apache/fineract/pull/6494#issuecomment-6594419460
Hey @adamsaghy, before I open the ticket and start, I'd like your input on one thing. I tested the `jdbcTransactionManager` + `SET TRANSACTION READ ONLY` approach and I don't think it works for report paths that run inside a JPA transaction. I reproduced it on PostgreSQL in a test modeled on `BatchJpaTransactionTest`. After I persist and flush a row in a JPA transaction, a read-only JDBC `TransactionTemplate` with `REQUIRES_NEW` doesn't get a new connection. It reuses the JPA one. The inner query saw the outer's uncommitted row, and after the outer rolled back the row was still there, so the inner commit flushed business writes early. It's the same thing the comment in `ScheduledJobRunnerConfig` warns about. This matters for `UpdateEmailOutboundWithCampaignMessageTasklet`, which runs under the `@Primary` JPA manager. I haven't traced the campaign domain-service paths fully yet. The rest of your analysis held up when I checked it locally. On MariaDB, a writing function called from a SELECT in a read-only transaction fails with error 1792 and nothing gets written. On PostgreSQL, `SELECT 1; COMMIT; DELETE FROM t` through pgjdbc does escape read-only and the DELETE runs, and a `$$` version does the same. `Parser.parseJdbcSql` splits both into 3-4 statements and agreed with the server on quotes, `E'..'`, `$$`, nested comments and a trailing `;`. The one catch is that `standardConformingStrings` has to come from the connection and not be hard-coded, otherwise `'\'; COMMIT; ...'` gets parsed differently from what the server does. What I'd like to do instead is run the report SQL on its own connection from the two execution sites in `ReadReportingServiceImpl`, in one small helper class. It would take `dataSource.getConnection()`, turn autocommit off, run `SET TRANSACTION READ ONLY`, and on PostgreSQL only reject anything the parser splits into more than one statement. Then it executes, builds the result, rolls back and closes. I tested this shape too. It gets a separate connection, doesn't see the outer uncommitted rows, rejects an INSERT, and behaves the same inside or outside a JPA transaction. The downsides are that it needs a second pooled connection while an outer transaction holds one (with a pool of 1 it fails with "Connection is not available"), and reports stop seeing the outer transaction's uncommitted writes. So what do you think? Are you ok with this instead of `jdbcTransactionManager` + `REQUIRES_NEW`, or do you prefer another way to get a separate connection? Is the extra pool slot ok for the campaign paths, or should I only do the REST and export paths first? Are you fine with depending on pgjdbc's internal `Parser`? I'd only check that there's exactly one statement and not filter by command type, since it types `VALUES` as BLANK and a DML CTE as SELECT (the read-only transaction already stops the CTE). And I'd leave the two `wrapSQL` parameter lookup callers out, since I can't find a way to write to `stretchy_parameter` through the API. I'll open the ticket and start once I hear back from you, thanks! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
