fix(csharp/src/Drivers/BigQuery): Fix the bug about large result and timeout #2810
Conversation
|
|
||
| await ExecuteWithRetriesAsync<BigQueryJob>(checkJobStatus); | ||
|
|
||
| // We can't checkJobStatus, Otherwise, the timeout in QueryResultsOptions is meaningless. |
There was a problem hiding this comment.
Would removing the polling loop not break the case where there's a longer-running job?. To the extent that there's a problem with the query timeout, should that not instead be reflected in the "while true" logic that doesn't currently take a timeout into account?
@davidhcoe, waiting for your feedback on this.
There was a problem hiding this comment.
The change in timeout behavior is caused by this commit: feat(csharp/src/Drivers/BigQuery): Add support for AAD/Entra authenti… · apache/arrow-adbc@5aa9f1e.
Let me explain the changes before and after this commit based on my understanding. In the previous timeout feature, it was completely specified by the timeout parameter in QueryResultOptions (default is five minutes, if the job is not completed after 5 minutes, it will prompt timeout), but customers can freely set it (such as setting timeout after 1 minute, timeout after 15 minutes, etc.). But after the modification, it became a constant wait and completely uncontrollable.
I don't think it's necessary to add relevant logic in while true, because the official QueryResultOptions already has a timeout parameter to implement this function, and we have been using it before without encountering any problems.
Of course, this is my viewpoint, and I look forward to your opinions.
There was a problem hiding this comment.
This looks good to me @CurtHagenlocher . I validated the long running query will still succeed without that block in place, and it will time out when it is supposed to time out.
3 participants
No description provided.