PDGGK commented on code in PR #17492:
URL: https://github.com/apache/iceberg/pull/17492#discussion_r3785691174
##########
core/src/test/java/org/apache/iceberg/jdbc/TestJdbcCatalog.java:
##########
@@ -1293,4 +1298,50 @@ private boolean tableExists(DatabaseMetaData metadata,
String tableName) throws
return false;
}
+
+ @Test
+ public void testExecuteRestoresInterruptStatus() throws Exception {
+ try (JdbcCatalog interruptibleCatalog =
+ initCatalog(
+ "interrupt_jdbc_catalog",
ImmutableMap.of(CatalogProperties.CLIENT_POOL_SIZE, "1"))) {
+ CountDownLatch connectionHeld = new CountDownLatch(1);
+ CountDownLatch releaseConnection = new CountDownLatch(1);
+ ExecutorService executor = Executors.newSingleThreadExecutor();
+ // Hold the only pooled connection, so that the next caller has to wait
on the pool. That
+ // wait is where a thread that is already interrupted observes the
interrupt.
+ Future<?> holder =
+ executor.submit(
+ () -> {
+ interruptibleCatalog
+ .connectionPool()
+ .run(
+ conn -> {
+ connectionHeld.countDown();
+ try {
+ releaseConnection.await();
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ }
+ return null;
+ });
+ return null;
+ });
+
+ try {
+ assertThat(connectionHeld.await(30, TimeUnit.SECONDS)).isTrue();
+ Thread.currentThread().interrupt();
+ // purge = false keeps this on the execute() path. The default purge =
true would first
+ // read table metadata through fetch(), which already restores the
interrupt status.
+ assertThatThrownBy(
Review Comment:
Good catch, and sorry for the slow reply — I pushed the fix but never came
back to say so.
That was accurate for the commit you reviewed (`8aacaaf0e`), which ended the
chain at `.isInstanceOf(...)`. On the current head it reads:
```java
assertThatThrownBy(
() -> interruptibleCatalog.dropTable(TableIdentifier.of("ns",
"tbl"), false))
.isInstanceOf(UncheckedInterruptedException.class)
.hasMessage("Interrupted in SQL command");
```
`build-checks (17, pull_request)` is green, so
`AssertThatThrownByWithMessageCheck` is satisfied, and it is the only
`assertThatThrownBy` the PR adds.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
