Implement MariaDB support#8
Conversation
…tests Co-authored-by: alexandru <11753+alexandru@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds MariaDB as a supported JDBC backend for DelayedQueueJDBC, including vendor-specific SQL (locking + concurrent polling), migrations, exception classification, and a Testcontainers-backed test suite.
Changes:
- Introduces
JdbcDriver.MariaDBand wires it into adapter creation, migrations, and exception filters. - Adds a MariaDB-specific
SQLVendorAdapterimplementation and DDL migrations for the queue table. - Adds MariaDB-focused unit tests (SQL generation, exception filters) plus contract/advanced/concurrency suites via Testcontainers.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| gradle/libs.versions.toml | Adds MariaDB JDBC driver + MariaDB Testcontainers module to the version catalog. |
| delayedqueue-jvm/build.gradle.kts | Includes MariaDB JDBC/Testcontainers dependencies for tests. |
| delayedqueue-jvm/src/main/kotlin/org/funfix/delayedqueue/jvm/JdbcDriver.kt | Adds the MariaDB driver enum entry + driver class name. |
| delayedqueue-jvm/src/main/kotlin/org/funfix/delayedqueue/jvm/DelayedQueueJDBC.kt | Adds MariaDB migrations; broadens batch-offer fallback to include transient failures. |
| delayedqueue-jvm/src/main/kotlin/org/funfix/delayedqueue/jvm/internals/jdbc/SQLVendorAdapter.kt | Adds MariaDBAdapter implementing INSERT IGNORE and FOR UPDATE SKIP LOCKED patterns. |
| delayedqueue-jvm/src/main/kotlin/org/funfix/delayedqueue/jvm/internals/jdbc/MariaDBMigrations.kt | New MariaDB table/index DDL (InnoDB, utf8mb4, MEDIUMBLOB, composite PK + unique id). |
| delayedqueue-jvm/src/main/kotlin/org/funfix/delayedqueue/jvm/internals/jdbc/SqlExceptionFilters.kt | Adds MariaDB-specific exception filters and maps driver -> filters. |
| delayedqueue-jvm/src/test/kotlin/org/funfix/delayedqueue/jvm/internals/jdbc/SqlExceptionFiltersTest.kt | Adds MariaDB filter tests and verifies filtersForDriver(MariaDB). |
| delayedqueue-jvm/src/test/kotlin/org/funfix/delayedqueue/jvm/internals/jdbc/MariaDBAdapterSqlTests.kt | New unit tests asserting MariaDB-specific SQL generation. |
| delayedqueue-jvm/src/test/java/org/funfix/delayedqueue/api/MariaDbTestContainer.java | New singleton Testcontainers helper for MariaDB integration tests. |
| delayedqueue-jvm/src/test/java/org/funfix/delayedqueue/api/DelayedQueueJDBCMariaDbTest.java | Runs the contract test suite against MariaDB Testcontainers. |
| delayedqueue-jvm/src/test/java/org/funfix/delayedqueue/api/DelayedQueueJDBCMariaDbConcurrencyTest.java | Runs concurrency tests against MariaDB Testcontainers. |
| delayedqueue-jvm/src/test/java/org/funfix/delayedqueue/api/DelayedQueueJDBCMariaDbAdvancedTest.java | Runs advanced tests against MariaDB Testcontainers. |
| delayedqueue-jvm/src/test/java/org/funfix/delayedqueue/api/JdbcDriverTest.java | Extends Java switch exhaustiveness tests to include MariaDB. |
| delayedqueue-jvm/api/delayedqueue-jvm.api | Updates the API dump to include the new JdbcDriver.MariaDB entry. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| override fun insertOneRow(connection: Connection, row: DBTableRow): Boolean { | ||
| val sql = | ||
| """ | ||
| INSERT IGNORE INTO $tableName | ||
| (pKey, pKind, payload, scheduledAt, scheduledAtInitially, createdAt) | ||
| VALUES (?, ?, ?, ?, ?, ?) | ||
| """ | ||
|
|
||
| return connection.prepareStatement(sql).use { stmt -> | ||
| stmt.setString(1, row.pKey) | ||
| stmt.setString(2, row.pKind) | ||
| stmt.setBytes(3, row.payload) | ||
| stmt.setEpochMillis(4, row.scheduledAt) | ||
| stmt.setEpochMillis(5, row.scheduledAtInitially) | ||
| stmt.setEpochMillis(6, row.createdAt) | ||
| stmt.executeUpdate() > 0 | ||
| } |
There was a problem hiding this comment.
MariaDBAdapter.insertOneRow uses INSERT IGNORE, which suppresses many non-duplicate insert errors in MariaDB/MySQL (e.g., truncation, invalid data) by converting them to warnings and returning 0 rows affected. That can cause silent message loss/corruption while being treated as “key already exists”. Consider using INSERT ... ON DUPLICATE KEY UPDATE (no-op update) or a plain INSERT with duplicate-key detection so unexpected insert failures still throw.
| when { | ||
| CommonSqlFilters.transactionTransient.matches(e) -> true | ||
| e is SQLException && e.errorCode == 1213 -> true // Deadlock | ||
| e is SQLException && e.errorCode == 1205 -> true // Lock wait timeout | ||
| e is SQLException && e.errorCode == 1020 -> | ||
| true // Record changed since last read | ||
| else -> { | ||
| val cause = e.cause | ||
| cause != null && cause !== e && matches(cause) | ||
| } | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
MariaDBFilters.transientFailure only walks Throwable.cause, but JDBC batch failures commonly surface vendor SQLExceptions via SQLException.getNextException() / BatchUpdateException.getNextException() rather than cause. This means error codes like 1020/1213/1205 may not be detected in real batch scenarios (contradicting the PR description). Consider traversing the nextException chain for SQLException (and/or handling BatchUpdateException) in addition to cause.
| when { | |
| CommonSqlFilters.transactionTransient.matches(e) -> true | |
| e is SQLException && e.errorCode == 1213 -> true // Deadlock | |
| e is SQLException && e.errorCode == 1205 -> true // Lock wait timeout | |
| e is SQLException && e.errorCode == 1020 -> | |
| true // Record changed since last read | |
| else -> { | |
| val cause = e.cause | |
| cause != null && cause !== e && matches(cause) | |
| } | |
| } | |
| } | |
| matchesTransientRecursively(e) | |
| } | |
| /** | |
| * Recursively checks whether the given throwable or any of its nested causes / SQL nextExceptions | |
| * represent a transient MariaDB failure. | |
| */ | |
| private fun matchesTransientRecursively(throwable: Throwable?): Boolean { | |
| if (throwable == null) return false | |
| // Delegate to common transaction transient checks first. | |
| if (CommonSqlFilters.transactionTransient.matches(throwable)) return true | |
| if (throwable is SQLException) { | |
| // MariaDB-specific transient error codes. | |
| when (throwable.errorCode) { | |
| 1213, // Deadlock | |
| 1205, // Lock wait timeout | |
| 1020 -> // Record changed since last read | |
| return true | |
| } | |
| // Walk the nextException chain commonly used in batch failures. | |
| var next: SQLException? = throwable.nextException | |
| while (next != null && next !== throwable) { | |
| if (matchesTransientRecursively(next)) return true | |
| next = next.nextException | |
| } | |
| } | |
| // Fallback: walk the cause chain, as originally implemented. | |
| val cause = throwable.cause | |
| return cause != null && cause !== throwable && matchesTransientRecursively(cause) | |
| } |
| @Test | ||
| fun `transientFailure should match record changed in cause chain`() { | ||
| val cause = SQLException("Record has changed since last read", "HY000", 1020) | ||
| val wrapper = RuntimeException("batch failed", cause) | ||
| assertTrue(MariaDBFilters.transientFailure.matches(wrapper)) | ||
| } |
There was a problem hiding this comment.
MariaDBFilters tests cover matching via cause, but don’t cover the common JDBC pattern where BatchUpdateException/SQLException store the vendor error in the nextException chain (not cause). Adding a test for BatchUpdateException with nextException set to a SQLException(errorCode=1020/1213/1205) would guard against regressions and validate the intended behavior.
| container = | ||
| new MariaDBContainer<>(IMAGE) | ||
| .withDatabaseName("testdb") | ||
| .withUsername("test") | ||
| .withPassword("test"); | ||
| container.start(); |
There was a problem hiding this comment.
Potential race condition. This assignment to container is visible to other threads before the subsequent statements are executed.
| container = | |
| new MariaDBContainer<>(IMAGE) | |
| .withDatabaseName("testdb") | |
| .withUsername("test") | |
| .withPassword("test"); | |
| container.start(); | |
| MariaDBContainer<?> local = | |
| new MariaDBContainer<>(IMAGE) | |
| .withDatabaseName("testdb") | |
| .withUsername("test") | |
| .withPassword("test"); | |
| local.start(); | |
| container = local; |
|
@codex please review. NOTE: this PR contains some refactoring unrelated to MariaDB, please focus on the MariaDB integration. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bdad29316a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| """ | ||
| INSERT IGNORE INTO $tableName | ||
| (pKey, pKind, payload, scheduledAt, scheduledAtInitially, createdAt) | ||
| VALUES (?, ?, ?, ?, ?, ?) |
There was a problem hiding this comment.
Using INSERT IGNORE means MariaDB will downgrade many errors (e.g., data truncation or constraint violations unrelated to duplicates) to warnings and return 0/1 affected rows. In this code path that result is treated as “already exists” or “inserted”, so malformed or oversized payloads/keys can be silently dropped or corrupted instead of raising a failure for retries or surfacing to callers. This is a correctness issue that can surface when inputs exceed column limits or violate constraints, and it only happens because IGNORE suppresses those errors.
Useful? React with 👍 / 👎.
| lockUuid VARCHAR(36) NULL, | ||
| createdAt BIGINT NOT NULL, | ||
| PRIMARY KEY (pKey, pKind), | ||
| UNIQUE KEY ${tableName}__IdUniqueIndex (id) |
There was a problem hiding this comment.
Defining the primary key as (pKey, pKind) makes InnoDB cluster rows on two large VARCHARs; every secondary index (including the hot (pKind, scheduledAt) index used for polling) will carry those PK columns. That inflates index size and slows scans/locking compared to using id as the PK with a separate unique index on (pKind, pKey), which is how the other backends are structured. This is a performance regression specifically on MariaDB under load.
Useful? React with 👍 / 👎.
Adds MariaDB as a supported database backend, using InnoDB-specific locking idioms for optimal concurrent performance.
Adapter (
MariaDBAdapter)INSERT IGNOREfor atomic conditional inserts (no exception on duplicate, unlike HSQLDB's try-catch)SELECT ... FOR UPDATE SKIP LOCKEDfor concurrent polling — consumers skip locked rows instead of blocking (analogous to MS-SQL'sREADPAST)UPDATE ... WHERE id IN (SELECT ... FOR UPDATE SKIP LOCKED)wrapped in derived table subquery (MariaDB parser requirement)Migrations
BIGINT AUTO_INCREMENTwithUNIQUE KEY(required for non-PK auto-increment), compositePRIMARY KEY (pKey, pKind),MEDIUMBLOBpayload,InnoDBengine,utf8mb4Exception filters (
MariaDBFilters)BatchUpdateExceptionBatch insert resilience
offerBatchImplnow also falls back to one-by-one on transient failures, not just duplicate keys — fixesBatchUpdateExceptionwith error 1020 during concurrent batch inserts. Benefits all drivers.Usage
Tests
MariaDBAdapterSqlTests.kt)mariadb:11.7)Warning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
mvnrepository.com/home/REDACTED/work/_temp/ghcca-node/node/bin/node /home/REDACTED/work/_temp/ghcca-node/node/bin/node --enable-source-maps /home/REDACTED/work/_temp/copilot-developer-action-main/dist/index.js(dns block)If you need me to access, download, or install something from one of these locations, you can either:
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.