You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@iotdb.apache.org by "shuwenwei (via GitHub)" <gi...@apache.org> on 2026/07/13 09:37:01 UTC
[PR] [to dev/1.3] Remove incorrect timeout reassignment after query planning [iotdb]
shuwenwei opened a new pull request, #18196:
URL: https://github.com/apache/iotdb/pull/18196
The context.setTimeOut() call in QueryExecution.start() mutated timeOut from the total timeout duration to the remaining time after planning. However, all consumers (checkTimeOutForQuery, cleanUpStaleQueries, timeout exception messages) assume timeOut holds the total duration. This caused stale query cleanup to trigger earlier than expected and incorrect deadline values in timeout error messages.
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [to dev/1.3] Remove incorrect timeout reassignment after query planning [iotdb]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #18196:
URL: https://github.com/apache/iotdb/pull/18196#issuecomment-4956605472
## [](https://sonarcloud.io/dashboard?id=apache_iotdb&pullRequest=18196) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_iotdb&pullRequest=18196&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_iotdb&pullRequest=18196&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_iotdb&pullRequest=18196&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_iotdb&pullRequest=18196&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_iotdb&pullRequest=18196&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_iotdb&pullRequest=18196)
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [to dev/1.3] Remove incorrect timeout reassignment after query planning [iotdb]
Posted by "PDGGK (via GitHub)" <gi...@apache.org>.
PDGGK commented on PR #18196:
URL: https://github.com/apache/iotdb/pull/18196#issuecomment-5035877984
Nice catch — this looks right to me. I traced the `getTimeOut()` consumers to double-check: the runtime deadline checks in `FragmentInstanceDispatcherImpl` (`currentTime - getStartTime() >= getTimeOut()`) and the fragment-timeout computations that do `getTimeOut() - (now - getStartTime())` (`SimpleFragmentParallelPlanner`, `TableModelQueryFragmentPlanner`, …) all treat `getTimeOut()` as the total duration from `getStartTime()`, so the post-planning reassignment was effectively subtracting the planning time twice. Removing it restores a single consistent meaning.
One small thought: since several call sites recompute `getTimeOut() - (now - getStartTime())` themselves, might a short javadoc on `getTimeOut()` noting it's always the total-from-start (never "remaining") help keep someone from re-introducing a reassignment like this later? Non-blocking either way.
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [to dev/1.3] Remove incorrect timeout reassignment after query planning [iotdb]
Posted by "JackieTien97 (via GitHub)" <gi...@apache.org>.
JackieTien97 merged PR #18196:
URL: https://github.com/apache/iotdb/pull/18196
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [to dev/1.3] Remove incorrect timeout reassignment after query planning [iotdb]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #18196:
URL: https://github.com/apache/iotdb/pull/18196#issuecomment-4956990973
## [Codecov](https://app.codecov.io/gh/apache/iotdb/pull/18196?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 43.50%. Comparing base ([`458b547`](https://app.codecov.io/gh/apache/iotdb/commit/458b5479f96688206277e7e1312a1f648a5b8160?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`9735c37`](https://app.codecov.io/gh/apache/iotdb/commit/9735c3749eae8f87a884cbcd13cad0f288afa963?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 67 commits behind head on dev/1.3.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## dev/1.3 #18196 +/- ##
=============================================
+ Coverage 42.61% 43.50% +0.89%
+ Complexity 241 198 -43
=============================================
Files 3622 3630 +8
Lines 240790 243739 +2949
Branches 29341 29741 +400
=============================================
+ Hits 102602 106030 +3428
+ Misses 138188 137709 -479
```
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/iotdb/pull/18196?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
</details>
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [to dev/1.3] Remove incorrect timeout reassignment after query planning [iotdb]
Posted by "PDGGK (via GitHub)" <gi...@apache.org>.
PDGGK commented on PR #18196:
URL: https://github.com/apache/iotdb/pull/18196#issuecomment-5044362064
Thanks for adding this — "total query timeout ... measured from `getStartTime()` ... never represents the remaining timeout" is exactly the intent, and it should keep the next person from re-introducing a post-planning reassignment. LGTM.
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org