chore(bigquery-jdbc): add the bridge Thread wrap ahead of Executor migration#13482
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.
|
|
||
| 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)
There was a problem hiding this comment.
We use sleep for NEW threads because join returns immediately on unstarted threads, which would cause the get() loop to busy-wait and consume 100% CPU until the thread is started. Additionally, checking every 50ms allows the thread calling get() to detect a cancellation request quickly and throw a CancellationException without remaining blocked for a long join timeout.
|
/gcbrun |
|
/gcbrun |
2 participants
No description provided.