[fix](test) pin insert visible timeout case to master FE#65340
[fix](test) pin insert visible timeout case to master FE#65340shuke987 wants to merge 3 commits into
Conversation
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
run buildall |
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Automated review found one blocking regression in the test change.
Critical checkpoints:
- Scope: reviewed the single changed file and related regression framework helpers (
getMasterIp,getMasterPort,connectTo,Config.buildUrlWithDb,DebugPoint) plusPublishVersionDaemon. - Existing review context: no existing inline review threads/comments; no duplicate suppression needed.
- User focus: no additional user-provided review focus.
- Validation: GitHub PR patch/head verified at
b544a8e5842b2659664f934d9e904fed09d7adf6; scopedgit show --checkon the changed file passed. I did not run the regression suite because no matching branch-4.1/master-observers cluster is available in this runner. - AGENTS: the review prompt reported no required
AGENTS.mdfiles for changed ancestors. The repo-local code-review skill was not present after hidden-path search, so I followed the available Doris review workflow instructions.
Subagent conclusions:
optimizer-rewritereported no optimizer/rewrite candidate.tests-session-configproposedTSC-001; I accepted it asMRG-001and submitted it inline.- Final convergence round 1: both live subagents returned
NO_NEW_VALUABLE_FINDINGSfor the current ledger/comment set.
| def feHost = feHttpAddress.split(":")[0] | ||
| def feHttpPort = Integer.parseInt(feHttpAddress.split(":")[1]) | ||
| // PublishVersionDaemon runs on the master FE, so keep both the session and debug point on master. | ||
| def feHost = getMasterIp() |
There was a problem hiding this comment.
This reintroduces the endpoint problem that the removed block was guarding against. getMasterIp() returns Host from frontends(), and this value is now used both for DebugPoint.enableDebugPoint(...) and for the rebuilt JDBC URL. In the pipeline configs the runner-facing endpoints are carried separately, for example jdbcUrl = jdbc:mysql://172.19.0.2:9131/... and feHttpAddress = 172.19.0.2:8131, while the old comment explicitly avoided SHOW FRONTENDS because it can expose loopback/internal addresses. On those deployments the test will fail before reaching the visible-timeout path even though the configured endpoints are reachable. Please keep the master pinning, but resolve it through runner-reachable regression/cluster metadata or preserve the configured host when switching to the master's query/http ports instead of using raw frontends().Host.
|
Addressed review: keep master pinning by using master FE query/http ports, but preserve the configured runner-reachable FE host instead of using raw frontends().Host. New commit: 40e1569. |
|
run buildall |
|
/review |
|
Codex automated review failed and did not complete. Error: You've hit your usage limit. Visit https://chatgpt.com/codex/settings/usage to purchase more credits or try again at Jul 10th, 2026 6:44 AM. Please inspect the workflow logs and rerun the review after the underlying issue is resolved. |
|
/review |
There was a problem hiding this comment.
Automated review found one current-head blocking issue.
Critical checkpoints:
- Scope: reviewed the single changed file and related regression framework helpers (
getMasterPort,connectTo,Config.buildUrlWithDb,DebugPoint) plus the nonConcurrent/p0 pipeline configs that define the runner-facing endpoints. - Existing review context: read the existing inline thread
discussion_r3536645563; the submitted comment is distinct because the prior thread covered the rawfrontends().Hostregression, while current head fixed the host but still uses rawfrontends()ports. - User focus: no additional user-provided review focus.
- Validation: verified the GitHub PR patch at head
40e1569de407209b2728d3e2ab70e6bdefa01565; the patch reverse-applies cleanly against this checkout. I did not run the regression suite because no live Doris regression cluster is available in this runner. - AGENTS/skill: the prompt reported no required
AGENTS.mdfiles. No in-repositorySKILL.mdwas present after hidden-path and full-tree searches, so I followed the explicit review prompt rules.
Subagent conclusions:
optimizer-rewritereported no optimizer/rewrite candidates in the initial pass and returnedNO_NEW_VALUABLE_FINDINGSin convergence round 1.tests-session-configreported no candidates in the initial pass and returnedNO_NEW_VALUABLE_FINDINGSin convergence round 1.- Main-agent candidate
M1was accepted and submitted inline; convergence round 1 ended with all live subagents reportingNO_NEW_VALUABLE_FINDINGSfor this ledger/comment set.
| // PublishVersionDaemon runs on the master FE. Use the configured runner-reachable host and master FE ports, | ||
| // because SHOW FRONTENDS can expose loopback or internal addresses in regression deployments. | ||
| def feHost = context.config.feHttpAddress.split(":")[0] | ||
| def feHttpPort = getMasterPort() |
There was a problem hiding this comment.
The host part is now runner-reachable, but this still swaps in ports from frontends(). In the static nonConcurrent/p0 pipeline configs the reachable endpoints are jdbcUrl = jdbc:mysql://172.19.0.2:9131/... and feHttpAddress = 172.19.0.2:8131, while the FE config does not override query_port/http_port, so getMasterPort() reads the FE-advertised defaults 9030/8030. That rebuilds the JDBC URL as 172.19.0.2:9030 and sends the debug-point request to 172.19.0.2:8030, which are not the configured runner-facing endpoints. Please resolve the master FE through runner-reachable cluster metadata, or keep the configured JDBC/HTTP ports when the discovered master corresponds to the configured FE.
|
Addressed current-head review: removed raw frontends() port usage. The test now preserves the configured runner-facing JDBC endpoint via context.getJdbcUrl() and the configured FE HTTP endpoint via context.config.feHttpAddress, so it no longer rebuilds targets with FE-internal 9030/8030 ports. New commit: 1b2b2a6. |
|
/review |
There was a problem hiding this comment.
Automated review found no new blocking issue at current head.
Critical checkpoints:
- Scope: reviewed the single changed file and related regression helpers/configuration:
SuiteContext.getJdbcUrl,connectTo, andcloseThreadLocal;Config.buildUrlWithDb;DebugPoint;PublishVersionDaemon.stop_publish;OlapInsertExecutortimeout handling; and the static nonConcurrent pipeline endpoints. - Existing review context: read existing inline threads
discussion_r3536645563anddiscussion_r3551549565. Current head no longer uses rawfrontends()host or port values in this test and preserves the configured runner-facing JDBC/HTTP endpoints, so I found no distinct non-duplicate endpoint issue to submit. - User focus: no additional user-provided review focus.
- Validation: verified GitHub PR metadata/patch at head
1b2b2a6f89a153e9181d2cc32f2ad9883022ff8c, fetched exact baseee46005a0423b5f72e691899874274b40dd2e2eb, andgit diff --check ee46005a0423b5f72e691899874274b40dd2e2eb..HEAD -- regression-test/suites/insert_p0/test_insert_visible_timeout_return_mode.groovypassed. I did not run the regression suite because no live Doris regression cluster is available in this runner. - AGENTS/skill: the prompt reported no required
AGENTS.mdfiles for changed ancestors. No in-repositorySKILL.mdwas present after hidden-path and full-tree searches, so I followed the explicit review prompt rules.
Subagent conclusions:
optimizer-rewriteproposed no candidate IDs.tests-session-configproposed no candidate IDs.- Final convergence round 1 ended with both live subagents reporting
NO_NEW_VALUABLE_FINDINGSfor this ledger/comment set.
|
run buildall |
2 participants
Summary
test_insert_visible_timeout_return_modeto the master FE before running the INSERT checks.PublishVersionDaemon.stop_publishdebug point.Testing
git diff --checkFailure evidence: