adamsaghy commented on PR #6494: URL: https://github.com/apache/fineract/pull/6494#issuecomment-6609539595
> Thanks @adamsaghy , I tried the reporting datasource and it works where the JDBC manager did not. > > On PostgreSQL I used a separate Hikari pool with a plain DataSourceTransactionManager (enforceReadOnly) and a read-only TransactionTemplate, inside a JPA transaction modeled on BatchJpaTransactionTest. It saw none of the outer transaction's uncommitted rows. It rejected an INSERT with "cannot execute INSERT in a read-only transaction", and the same INSERT works on that pool without read-only, so read-only is the cause. Nothing was left behind after the outer rollback, and it still worked with the main pool limited to one connection. So there is no shared-connection leak and no extra slot from the main pool. > > On MariaDB through Connector/J, both SET TRANSACTION READ ONLY and Connection.setReadOnly(true) make a writing function fail with error 1792. I have not tested MySQL yet, that will be a test case. > > I would keep the read-only transaction and the PostgreSQL single-statement check as the baseline even with a SELECT-only user: > > * On MariaDB, a SELECT-only user still ran a writing function with its definer's rights. Only the read-only transaction blocked it. > * On PostgreSQL, a SECURITY DEFINER function does the same for a SELECT-only role (tested through psql), and again only the read-only transaction blocked it. > * On PostgreSQL, a session-level read-only default can be undone with SET default_transaction_read_only = off plus COMMIT. > * On PostgreSQL through pgjdbc, neither Hikari's setReadOnly(true) nor SET TRANSACTION READ ONLY stops SELECT 1; COMMIT; DELETE FROM t. preferQueryMode=simple changes nothing. Only the statement count blocks it, and it counts 3 statements in every case. > > I also checked the rest of the list. All 87 seeded reports parse as exactly one statement with Parser.parseJdbcSql, using the connection's standardConformingStrings. ReportParameterRepository is only used with findById, and I found no SQL in main code that writes stretchy_parameter, so I will leave the two parameter lookup wrapSQL callers out. > > Four questions: > > 1. Should the pool reuse the existing readonly_schema_* settings when they are set? They are blank by default and currently mean read-only instance mode, so reports would go to that host and could show replication lag. Or do you prefer new reporting-specific properties? > 2. When they are blank, is falling back to the normal credentials on a separate small pool ok, instead of failing closed? Failing closed would break reports on every default install. > 3. Are you fine with depending on pgjdbc's internal Parser? I would only count statements, not filter by command type. > 4. The campaign paths can run the report inside the caller's JPA transaction (the tasklet does for sure, I have not traced the domain-service callers). With a separate pool those reports would not see that transaction's uncommitted rows. Should campaigns use the reporting pool too, or should I start with REST and export only? > > The reporting pool would also get its own small size property (the per-tenant defaults would add up to 40 connections per tenant) and its own pool name, since the current _pool name would collide on JMX registration. > > I'll open the ticket and start once I hear back, thanks! I prefer generic, straightforward solutions, so from my point of view: dedicated read-only datasource is the way to go. It should use the read replicate properties if set, otherwise use the default settings, but still open read only connections. We dont even need pool for that either, 1 connection should be just fine -- 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]
