[mgaffigan]

Running MySQL at the default REPEATABLE_READ transaction isolation level causes hangs and errors throughout the application when running at load. These were frequently the cause of flaky tests in #449, and have issues open in the nextgen repo.

Add a warning to startup to check for misconfigured instances, and update the instructions in mirth.properties to have the correct transaction isolation level.

Release note:

MySQL installs should not run at REPEATABLE READ. The server will now warn if you run at REPEATABLE READ, and will block new installs at REPEATABLE READ. Update your connection string per the guidance in mirth.properties.

[github-actions]

Test Results

130 files  ±0  130 suites  ±0   4m 6s ⏱️ + 1m 4s
789 tests ±0  789 ✅ ±0  0 💤 ±0  0 ❌ ±0 
915 runs  ±0  909 ✅ ±0  6 💤 ±0  0 ❌ ±0 

Results for commit de18cb3. ± Comparison against base commit a403039.

♻️ This comment has been updated with latest results.

[pacmano1]

I would refuse to start. Like driving on a bald tire ( mandatory car ananlogy) and protect the user from themselves.

[mgaffigan]

I would refuse to start. Like driving on a bald tire ( mandatory car ananlogy) and protect the user from themselves.

Sure - the less interrupting scenario here is that we could set the transaction isolation level ourselves, but such changes are highly affecting. If we include a "do it anyway", I could get on board with that.

I question making such a big change without the administrator being involved. Channels could be relying upon repeatable read for correctness.

[pacmano1]

I would refuse to start. Like driving on a bald tire ( mandatory car ananlogy) and protect the user from themselves.

Sure - the less interrupting scenario here is that we could set the transaction isolation level ourselves, but such changes are highly affecting. If we include a "do it anyway", I could get on board with that.

I question making such a big change without the administrator being involved. Channels could be relying upon repeatable read for correctness.

maybe for fresh installs do a hard stop?

[mgaffigan]

maybe for fresh installs do a hard stop?

That could work. Sure. Updated.

[NicoPiel]

The check doesn’t match its message. Both checks only reject REPEATABLE_READ, but the messages say READ-COMMITTED is required. SERIALIZABLE, which locks even more, gets through with no block and no warning. The fix is to test for “not READ_COMMITTED” instead.

[mgaffigan]

The check doesn’t match its message. Both checks only reject REPEATABLE_READ, but the messages say READ-COMMITTED is required. SERIALIZABLE, which locks even more, gets through with no block and no warning. The fix is to test for “not READ_COMMITTED” instead.

This is intentional, but I'm open to other phrasing of the message.

A DBA may reasonably configure the system away from READ_COMMITTED - the problem is that the default for MySQL is REPEATABLE_READ, so people are unintentionally running with REPEATABLE_READ. There's not a reasonable way I've found to check that the transaction isolation level is configured away from the default explicitly - which leaves checking for the default.

[tonygermano]

A few minor things suggested inline, but, otherwise, looks good to me. Those were supposed to be part of the same review, but I guess I clicked the wrong button.

Could you also please create the release note and put it in the PR description?

[mgaffigan]

@NicoPiel, I changed the phrasing of the message.