PDGGK opened a new pull request, #9159:
URL: https://github.com/apache/paimon/pull/9159
### Purpose
The `jdbc` package converts an `InterruptedException` into an unchecked
exception in nine places. **Seven re-assert the interrupt before rethrowing;
two do not.** This PR closes those two, so the package stops contradicting
itself.
| site | | |
|---|---|---|
| `JdbcCatalog:131` | constructor → `initializeCatalogTablesIfNeed()` |
**drops the flag** |
| `JdbcUtils:677` | `insertTable` | **drops the flag** |
| `JdbcCatalog` 640, 832, 1018, 1047, 1314 · `JdbcUtils` 627, 655 | |
already restore it |
Why it matters: `LinkedBlockingDeque.pollFirst`, which is what
`ClientPoolImpl.run` blocks on, **clears** the flag when it throws. Whoever
catches the resulting `RuntimeException` — a retry loop, an executor's task
wrapper, a catalog-loader that falls back to another catalog — sees a thread
that looks like it was never cancelled, and every subsequent blocking call on
it behaves accordingly. The two fixed sites are on the catalog-open and
table-create paths, so they run on exactly the threads a shutdown is trying to
stop.
I deliberately kept this to the two deviating sites, matching the shape
already used beside each one rather than introducing a new one:
- `JdbcCatalog:131` is a single `catch (InterruptedException e)` → mirrors
**`JdbcCatalog:1018`**
- `JdbcUtils:677` is a `catch (SQLException | InterruptedException e)` →
mirrors **`JdbcCatalog:1047`**, keeping the `instanceof` guard so a
`SQLException` is unaffected
No exception type or message changes, so existing assertions such as
`JdbcCatalogTest#testInsertTableUtility`'s `hasMessageContaining("Failed to
insert table")` still hold.
### Tests
New `JdbcInterruptStatusTest`, one case per fixed site.
The constructor case needs **no mocking**. `ClientPoolImpl.run` waits on
`LinkedBlockingDeque.pollFirst(10, SECONDS)`, and `lockInterruptibly()` throws
immediately when the calling thread already carries the flag — so setting the
flag and then calling the real constructor drives the real code down its real
interrupt path. `insertTable` is a plain static call, so it takes a mocked
`JdbcClientPool` whose `run` throws. Both cases clear the flag in `@AfterEach`
so it cannot leak into later tests on the same thread.
Checked against master, both fail — and they fail *at the interrupt-status
assertion*, with the exception type and message assertions already passing,
which is what confirms they are exercising the intended path rather than
erroring out early:
```
JdbcInterruptStatusTest.catalogConstructorKeepsTheInterruptStatus:87
JdbcInterruptStatusTest.insertTableKeepsTheInterruptStatus:102
Tests run: 2, Failures: 2
```
With the fix:
```
JdbcClientPoolTest Tests run: 4, Failures: 0
JdbcInterruptStatusTest Tests run: 2, Failures: 0
JdbcCatalogTest Tests run: 77, Failures: 0
```
`spotless:check`, `checkstyle:check` and `apache-rat:check` are clean. Java
8 syntax only, per `AGENTS.md`.
No overlap with the open #7475 — its hunks are elsewhere in both files, and
it only *calls* `insertTable`.
--
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]
