Repository navigation
[fix](regression) Use local HTTP resources in TVF test - #65299
Merged
Merged
Conversation
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Gabriel39
marked this pull request as ready for review
July 7, 2026 03:34
Contributor
Author
|
run buildall |
### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary: The HTTP TVF regression test depended on hard-coded GitHub raw URLs for checked-in test data, making the P0 case sensitive to external network availability. This change serves regression data from a temporary local HTTP server during the suite and points HTTP/file TVF URLs at that server, while preserving HuggingFace-specific URI coverage. In distributed test environments, the local HTTP server host can be set through httpTvfHost to an address that every BE can reach.
### Release note
None
### Check List (For Author)
- Test: Regression test
- ./run-regression-test.sh --run -d external_table_p0/tvf -s test_http_tvf -jdbc "jdbc:mysql://127.0.0.1:59030/?useLocalSessionState=true&allowLoadLocalInfile=true&zeroDateTimeBehavior=round" -ha 127.0.0.1:58030 -conf httpTvfHost=10.26.20.3 (on [email protected]:/mnt/disk3/gabriel/Workspace/dev2/doris, passed; local HTTP URLs used 10.26.20.3)
- Behavior changed: No
- Does this need documentation: No
Gabriel39
force-pushed
the
codex/local-http-tvf
branch
from
July 7, 2026 03:41
e1e3d36 to
a59a310
Compare
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
Contributor
There was a problem hiding this comment.
Automated review summary for PR 65299:
I reviewed the single changed regression suite and the related HTTP TVF FE/BE paths. I did not find a substantiated blocking issue, so I am leaving this as a comment-only review with no inline comments.
Critical checkpoint conclusions:
- Goal and test proof: the change replaces raw GitHub data URLs in
test_http_tvfwith a per-suite local HTTP server while preserving HuggingFace URI coverage. The referenced local data files exist, and the local blobs match the original GitHub refs, including the previously pinned4.0.2-rc02CSV. The existing.outfile does not include the substituted URLs. - Scope: the PR is focused on one regression test file and does not alter product code paths.
- Concurrency and lifecycle: the only new lifecycle is the per-suite
HttpServer; the SQL checks are synchronous and the server is stopped infinally. - Config/session propagation:
httpTvfHostis accepted through the existing-conf key=valuepath intootherConfigs; fallback toexternalEnvIpmatches existing regression conventions. - Parallel paths and compatibility:
http(...)andfile(... fs.http.support=true)still flow through the same FE TVF property handling and BE HTTP reader. FE HEAD size discovery and BE range/non-range reads are covered by the local server behavior. - Test outputs: no generated output change was needed; added patch lines have no trailing whitespace.
- Validation limit: I did not run the full regression suite in this checkout because
.worktree_initialized,thirdparty/installed, andthirdparty/installed/bin/protocare missing. The PR body reports a successful targeted regression run by the author.
Subagent conclusions:
optimizer-rewrite: no optimizer/rewrite candidate; convergence round 1 repliedNO_NEW_VALUABLE_FINDINGS.tests-session-config: no tests/session/config candidate; convergence round 1 repliedNO_NEW_VALUABLE_FINDINGS.- No subagent candidate became an inline comment, and no duplicate candidate needed merging.
User focus: no additional user-provided review focus was supplied.
morningman
approved these changes
Jul 7, 2026
Contributor
|
PR approved by at least one committer and no changes requested. |
Contributor
|
PR approved by anyone and no changes requested. |
This was referenced Jul 7, 2026
Gabriel39
added a commit
to Gabriel39/incubator-doris
that referenced
this pull request
Jul 7, 2026
(cherry picked from commit 553c23d)
yiguolei
pushed a commit
that referenced
this pull request
Jul 8, 2026
…65313) ## Proposed changes Backport #65299 to branch-4.1. This updates the HTTP TVF regression test to serve local `regression-test/data` files through an in-process HTTP server instead of fetching raw GitHub-hosted URLs. ## Notes Resolved the suite-tag conflict by preserving branch-4.1's existing `p2` classification. ## Validation - `git diff --check origin/branch-4.1..codex/backport-65299-branch-4.1` - Verified all local `regression-test/data` files referenced by `httpUrl(...)` exist on the backport branch.
morningman
added a commit
that referenced
this pull request
Nov 5, 2026
…loopback hosts (#70232) ### Background The `http()` table-valued function reads CSV, JSON, Parquet, and ORC files from HTTP(S) URLs; for an `hf://` URI the FE expands the HuggingFace path into download URLs. `external_table_p0/tvf/test_http_tvf` (group `p0`, so every P0 run executes it) serves most of its fixtures from an HTTP server it starts inside the regression runner, and builds their URLs from `httpTvfHost`, else `externalEnvIp`, which is `127.0.0.1` in every pipeline config. Its last seven queries read public HuggingFace datasets through `hf://`. ### What problem does this PR solve? Issue Number: None Related PR: #65877, #66013, #66027 (branch-4.1 fixes by @hello-stephen, ported here), #65299 (added the in-runner HTTP server and its host lookup), #60915 (moved the suite into P0 and relaxed sql15 / sql16 to `count > 0`) PR Type: Test Problem Summary: On master the suite still depends on huggingface.co and on HuggingFace's mutable `main` branches, and it advertises the runner as `127.0.0.1`, which a BE on another host resolves to itself. This PR ports the three branch-4.1 fixes: a loopback or wildcard `externalEnvIp` falls back to the runner's host address (an explicit `httpTvfHost` is still used as given), the seven `hf://` reads use byte-identical mirrors in the regression OSS bucket pinned to a dataset revision, and the mirrored `prompts.csv` skips its header line. With the mirror pinned, sql15 / sql16 assert the exact count 204 again. ### Details - **Failure:** None on master CI today: P0 passed 66 of 66 runs on 2026-11-04, though the HuggingFace queries took about 146 of the suite's 149 seconds (build 1092587). On branch-4.1 the suite failed with `Connection refused` on `http://127.0.0.1:<port>` with remote BEs (#65877), with `java.net.SocketTimeoutException: Connect timed out` to HuggingFace (#66013), and then with sql15 getting 205 instead of 204 on the mirror (#66027). - **Cause:** The suite assumed that every BE runs on the runner's host, true in P0 only, and that huggingface.co is reachable with fixed `main` branches: `prompts.csv` on `fka/awesome-chatgpt-prompts` `main` now has five columns and is over six times the 204-record file the golden came from. The mirrored CSV keeps a header line that `hf://` hid, hence `skip_lines = 1`. - **Stability evidence:** No query reaches huggingface.co any more. The only remote host left is `doris-regression-hk.oss-cn-hongkong.aliyuncs.com` (public reads), which P0 already uses in `query_p0/test_parquet_dict` (28 of 28 recent runs passed). Since master's P0 does not fail today, there is no before-and-after failure rate. ### Testing - `-forceGenOut` run of the suite on a local 1 FE + 1 BE cluster from master `4530f761fa1` (the FE carries local patches that don't touch the HTTP TVF), with P0's `externalEnvIp = 127.0.0.1` and no `httpTvfHost`: the runner advertised `http://192.168.1.164:<port>`, and the 22 blocks generated for sql01 to sql18 and both `replaced_*` checks are byte-identical to master's `.out`, with sql15 / sql16 now after `replaced_after`, where they run. sql19 then hit the 900 s query timeout: this machine's BE reached the 20 MB Parquet mirror through a local proxy at about 10 KB/s (a direct GET from the same machine runs at about 600 KB/s). - The suite without sql19 to sql21, in compare mode against the committed `.out`: passed. With `skip_lines` removed from sql15 it fails with `Expect cell is: 204 But real is: 205`. - sql19 to sql21 did not run locally. Each mirrored Parquet file has the same size as on HuggingFace and a byte-identical footer (schema, row groups, statistics), and each footer's minimum `text` is the first row the `.out` expects. This PR's P0 (build 1092831) then ran the whole suite against the mirror: it passed in 8.9 s, with sql19, sql20 and the two sql21 queries taking 1.3 to 1.8 s each, and the runner advertised `http://172.16.0.146:<port>`; the 39 P0 runs before it, with the HuggingFace version, took 149 to 170 s. ### Notes for Reviewers - Apart from a one-line comment on the loopback check, the suite now matches branch-4.2's except for master's own parts: the `p0` label (#60915) and the `mutable/` cases (#68847). - `hf://` loses its only end-to-end coverage (real HuggingFace URLs, tree-API glob expansion, `@revision`); `HFUtilsTest` still covers parsing, URL conversion, glob matching, and one live `*` expansion against the HuggingFace tree API. - sql16, sql20, and the second sql21 now repeat sql15 / sql19, since plain HTTP has no globbing; they stay so that the goldens and branch-4.x line up. - In P0 the runner now advertises its host address instead of `127.0.0.1`; both work there, but branch-4.x labels the suite `p2`, so P0 runs this fallback for the first time. - `ai_p0/test_ai_profile` copied the old host lookup and has the same latent loopback problem; it is not changed here. Co-authored-by: Dongyang Li <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix Use local HTTP resources in TVF test
What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary: The HTTP TVF regression test depended on hard-coded GitHub raw URLs for checked-in test data, making the P0 case sensitive to external network availability. This change serves regression data from a temporary local HTTP server during the suite and points HTTP/file TVF URLs at that server, while preserving HuggingFace-specific URI coverage. In distributed test environments, the local HTTP server host can be set through httpTvfHost to an address that every BE can reach.
Release note
None
Check List (For Author)