chore(bigquery-jdbc): add the bridge Thread wrap ahead of Executor migration#13482
chore(bigquery-jdbc): add the bridge Thread wrap ahead of Executor migration#13482Neenu1995 wants to merge 3 commits into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a temporary compatibility bridge method wrapThread in BigQueryDatabaseMetaData to wrap raw Thread objects into Future instances, along with comprehensive unit tests. A critical issue was identified in the overloaded get(long timeout, TimeUnit unit) method: if the wrapped thread is in the NEW state, calling thread.join() will return immediately, resulting in a tight busy-wait loop that consumes 100% CPU. It is recommended to handle the NEW state by sleeping instead of joining, similar to the no-argument get() implementation.
|
|
||
| @Override | ||
| public Object get() throws InterruptedException, CancellationException { | ||
| if (isCancelled()) { |
There was a problem hiding this comment.
Instead of the code duplication, we can call get(timeout, unit) with some absurdly large limit, like 100 days.
|
|
||
| long delay = Math.min(remainingMillis, 50); | ||
| if (thread.getState() == Thread.State.NEW) { | ||
| Thread.sleep(delay); |
There was a problem hiding this comment.
Why the difference sleep vs join?
Also we can just call thread.join(1000);, there is no need to check every 50ms if object was cancelled or not. We have 10-60 second cleanup waiters either way (or worry about < 50 ms remaining time)
2 participants
No description provided.