Skip to content

Commit

Permalink
Improve handling of interrupts in TrinoResultSet
Browse files Browse the repository at this point in the history
Differentiate between interrupts happening in the calling thread and
interrupts of the background thread.

- use different message
- the interrupts happening in the calling thread should result in full
  cancellation of the background thread. Previously they would close
  client, but wouldn't set `cancelled` flag.
  • Loading branch information
findepi committed Sep 19, 2022
1 parent 993e6ba commit 3e01a3e
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 14 deletions.
23 changes: 10 additions & 13 deletions client/trino-jdbc/src/main/java/io/trino/jdbc/TrinoResultSet.java
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,9 @@ public AsyncIterator(Iterator<T> dataIterator, StatementClient client)
}
}
catch (InterruptedException e) {
handleInterrupt(e);
client.close();
rowQueue.clear();
throw new RuntimeException(new SQLException("ResultSet thread was interrupted", e));
}
finally {
semaphore.release();
Expand All @@ -196,18 +198,6 @@ public void cancel()
{
cancelled = true;
future.cancel(true);
cleanup();
}

private void handleInterrupt(InterruptedException e)
{
cleanup();
Thread.currentThread().interrupt();
throw new RuntimeException(new SQLException("ResultSet thread was interrupted", e));
}

private void cleanup()
{
// When thread interruption is mis-handled by underlying implementation of `client`, the thread which
// is working for `future` may be blocked by `rowQueue.put` (`rowQueue` is full) and will never finish
// its work. It is necessary to close `client` and drain `rowQueue` to avoid such leaks.
Expand Down Expand Up @@ -253,6 +243,13 @@ protected T computeNext()
}
return rowQueue.poll();
}

private void handleInterrupt(InterruptedException e)
{
cancel();
Thread.currentThread().interrupt();
throw new RuntimeException(new SQLException("Interrupted", e));
}
}

private static class ResultsPageIterator
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -906,7 +906,7 @@ public void testQueryCancelByInterrupt()
assertTrue(queryFinished.await(10, SECONDS));
assertThat(queryFailure.get())
.isInstanceOf(SQLException.class)
.hasMessage("ResultSet thread was interrupted");
.hasMessage("Interrupted");
assertEquals(getQueryState(queryId.get()), FAILED);
}

Expand Down

0 comments on commit 3e01a3e

Please sign in to comment.