Skip to content

Stop pooled DB connections from serving stale reads on MySQL - #150

Open
ParameswaranSajeenthiran wants to merge 1 commit into
wso2:mainfrom
ParameswaranSajeenthiran:fix/db-utils
Open

Stop pooled DB connections from serving stale reads on MySQL #150
ParameswaranSajeenthiran wants to merge 1 commit into
wso2:mainfrom
ParameswaranSajeenthiran:fix/db-utils

Conversation

@ParameswaranSajeenthiran

Copy link
Copy Markdown
Contributor

Purpose

On MySQL deployments, a comment posted on a complaint frequently did not appear in the timeline afterwards, on both the Data Principal and Complaint Officer surfaces. Status
changes behaved the same way: a complaint resolved by an officer still rendered as unresolved on the next load.

The data was never lost — it was committed correctly every time. The reads were returning an older snapshot of the database.

Root cause: JDBCPersistenceManager.getDBConnection() hands out every connection with setAutoCommit(false), and the read paths never commit or roll back — they only call close().
close() on a pooled connection returns it to the pool rather than closing it, so the connection went back mid-transaction. Under InnoDB's default REPEATABLE READ, a transaction's
read view is fixed at its first read and held until the transaction ends, so that connection stayed pinned to an old snapshot. The next borrower's setAutoCommit(false) is a no-op
(JDBC only commits when the value changes), so the stale transaction simply continued. A POST that committed on one connection was then invisible to a GET served by another.

Tomcat JDBC has cleanup for exactly this — ConnectionPool.shouldClose() → terminateTransaction() — but it runs only when the datasource sets defaultAutoCommit=false. The product's
generated master-datasources.xml sets no pool properties, so that branch never executed.

Measured on a live deployment before the fix: an idle (Sleep) pooled connection holding a RUNNING REPEATABLE READ transaction 239 seconds old while still serving requests.

Resolves: <issue link — I don't have the number>

Goals

  • Reads always reflect data committed before the request, on every supported database.
  • Fix it once, centrally: DatabaseUtils is shared, so consent history and event notifications carried the identical defect, not just complaints.
  • Cover it with a test that fails when the bug is present, so it can't silently return.

Approach

End any open transaction in DatabaseUtils.closeConnection() before handing the connection back to the pool:

if (!connection.getAutoCommit()) {
connection.rollback();
}

For a read path there is nothing to undo; the rollback's only effect is to terminate the transaction and release the read view, so the next statement on that connection starts
fresh. On a write path commit() has already ended the transaction, making this a no-op. The guard also skips connections in autocommit mode. Rollback failure is logged and does
not prevent the close().

Rejected alternative: turning autocommit on for read paths. It would require every read site to opt out of the shared helper, and would still leak a dirty connection whenever a
write path threw before its commit.

Not changed: defaultAutoCommit / rollbackOnReturn in the shipped deployment.toml. Worth adding as defence in depth, but operators edit that file freely, so the code has to hold on
its own.

No UI change.

User stories

  • As a Data Principal, when I reply on my complaint, I see my message in the thread immediately.
  • As a Complaint Officer, when I reply or change a complaint's status, the thread and status reflect it on the next load.
  • As either, the full conversation history stays visible for the life of the complaint.

Release note

Fixed complaint comments, timelines, and status changes intermittently showing stale data on MySQL deployments, caused by pooled database connections being returned to the pool
with an open transaction.

Documentation

N/A — internal defect fix. No configuration option, API contract, or user-facing behaviour is added or changed; the corrected behaviour is what the docs already describe.

Training

N/A

Certification

N/A — no change to product configuration or usage that a certification question could target.

Marketing

N/A

Automation tests

  • Unit tests

    Three TestNG cases added to DatabaseUtilsTest: rollback-before-close ordering (via Mockito InOrder), no rollback when autocommit is on, and a failing rollback still closing the
    connection.

    mvn verify -pl …accelerator.common: 41 tests, 0 failures. DatabaseUtils at 100% instruction coverage (38/38) and 100% branch coverage (4/4); module total 83.5% against the 0.8
    gate. Full reactor re-run green — every module's suite and every jacoco gate — since the shared helper is on every DAO's path.

  • Integration tests

    New Playwright spec 05.09.03 in tests/07-complaints/05.09-end-to-end-scenarios.spec.ts: five rounds of officer↔citizen exchange on one complaint, including two status
    transitions. Each message is read back from both surfaces, and every read asserts the entire thread so far rather than only the newest line — a single reply read once can land
    on the connection that just committed it and pass while broken. It closes with a direct API read of the timeline, bypassing the SPA's query cache.

    Verified as a real regression test against a MySQL-backed IS, same server, one variable:

Build Result
Fixed 2 passed (20.9s) — 05.09.03 in 7.0s
Pre-fix jar 2 failed — 05.09.03 timed out at 180s
Fixed again 2 passed (21.2s) — 05.09.03 in 6.7s
On the pre-fix build the pre-existing 05.09.02 failed too (the resolved banner never appeared) — this bug was already reachable by the existing suite; it had simply never been  
run against MySQL.                                                                                                                                                               
                                                                                                                                                                                 
Caveat: H2 and Postgres are READ COMMITTED, so the spec passes there either way. It guards the fix only where the suite runs against MySQL.

@coderabbitai

coderabbitai Bot commented Sep 4, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: e1227450-d755-41cb-94f4-2c9c67a320f1


Comment @coderabbitai help to get the list of available commands.

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.

1 participant