You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@texera.apache.org by "kz930 (via GitHub)" <gi...@apache.org> on 2026/08/02 01:04:56 UTC
[PR] fix(jsonl-scan): count the worker slice from the offset, not the file [texera]
kz930 opened a new pull request, #7247:
URL: https://github.com/apache/texera/pull/7247
### What changes were proposed in this PR?
`JSONLScanSourceOpExec.open()` applies the requested offset twice. It drops `offset` lines from the reader, so the iterator it keeps already begins at the requested row; it then computes the worker's slice bounds as `offsetValue + …`, which are positions in the original file, and applies those bounds to that already-offset iterator. The rows in between are skipped, and when what remains is shorter than the offset the operator emits nothing at all. Neither case raises an error or writes a log line, so the operator reports success with rows missing.
This PR resolves the bounds against the iterator they are actually applied to, by dropping the `offsetValue +` from both expressions. That is the first of the two options the issue describes; it keeps the reader-side `drop` doing the offset and leaves the slice to do only what it exists for, which is dividing the window across workers.
### Any related issues, documentation, discussions?
Closes #7245
### How was this PR tested?
Two cases were added to `JSONLScanSourceOpExecSpec`, which previously covered the row limit but never set an offset: an offset on its own, and an offset together with a limit — the combination that used to yield no rows at all. Both fail against the current code and pass with the change, and the three existing cases are unaffected.
Beyond those, the operator was driven over a five-line file across every combination of offset (unset, 0, 1, 2, 4, 5, 7), limit (unset, 0, 2, 3, 9) and worker count (1, 2, 3), with each result compared against `drop(offset).take(limit)` — the reading that `ArrowSourceOpExec` and this operator's own `sourceSchema()` already follow. 36 of the 105 combinations were wrong before the change and none after. Every one of the 36 had an offset of 1 or more, which is why an unset offset was unaffected and the gap went unnoticed. That sweep was a scratch harness for checking the fix and is not part of the diff; the two added cases are the representatives of it that are worth keeping.
### Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Opus 5)
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(jsonl-scan): count the worker slice from the offset, not the file [texera]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #7247:
URL: https://github.com/apache/texera/pull/7247#issuecomment-5154360713
<!-- texera-benchmarks-comment -->
## ⚠️ Benchmark changes need a look
🟢 0 better · 🔴 4 worse · ⚪ 11 noise (<±5%) · 0 without baseline
> Compared against main `33cc5d1` benchmarked on **this same runner**, so the delta is largely free of cross-runner hardware noise. The "7d avg" column still reflects the gh-pages dashboard. Treat <±5% as noise unless repeated.
[Dashboard](https://apache.github.io/texera/dev/bench/) · [Run](https://github.com/apache/texera/actions/runs/30726413232)
| | config | throughput | MB/s | latency | max Δ latest / 7d |
|---|---|---:|---:|---:|---:|
| 🔴 | bs=10 sw=10 sl=64 | 373 | 0.227 | 25,604/34,994/34,994 us | 🔴 -9.6% / 🔴 +123.0% |
| 🔴 | bs=100 sw=10 sl=64 | 775 | 0.473 | 126,272/153,919/153,919 us | 🔴 +6.5% / 🔴 +43.3% |
| ⚪ | bs=1000 sw=10 sl=64 | 899 | 0.548 | 1,116,342/1,155,503/1,155,503 us | ⚪ within ±5% / 🔴 +12.8% |
<details><summary>Baseline details</summary>
Latest main `33cc5d1` from same runner
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---:|---:|---:|---:|---:|
| bs=10 sw=10 sl=64 | throughput | 373 tuples/sec | 411 tuples/sec | 782.98 tuples/sec | -9.2% | -52.4% |
| bs=10 sw=10 sl=64 | MB/s | 0.227 MB/s | 0.251 MB/s | 0.478 MB/s | -9.6% | -52.5% |
| bs=10 sw=10 sl=64 | p50 | 25,604 us | 24,644 us | 12,467 us | +3.9% | +105.4% |
| bs=10 sw=10 sl=64 | p95 | 34,994 us | 34,239 us | 15,695 us | +2.2% | +123.0% |
| bs=10 sw=10 sl=64 | p99 | 34,994 us | 34,239 us | 18,950 us | +2.2% | +84.7% |
| bs=100 sw=10 sl=64 | throughput | 775 tuples/sec | 804 tuples/sec | 996.92 tuples/sec | -3.6% | -22.3% |
| bs=100 sw=10 sl=64 | MB/s | 0.473 MB/s | 0.491 MB/s | 0.608 MB/s | -3.7% | -22.3% |
| bs=100 sw=10 sl=64 | p50 | 126,272 us | 122,648 us | 100,899 us | +3.0% | +25.1% |
| bs=100 sw=10 sl=64 | p95 | 153,919 us | 144,547 us | 107,385 us | +6.5% | +43.3% |
| bs=100 sw=10 sl=64 | p99 | 153,919 us | 144,547 us | 115,226 us | +6.5% | +33.6% |
| bs=1000 sw=10 sl=64 | throughput | 899 tuples/sec | 919 tuples/sec | 1,024 tuples/sec | -2.2% | -12.2% |
| bs=1000 sw=10 sl=64 | MB/s | 0.548 MB/s | 0.561 MB/s | 0.625 MB/s | -2.3% | -12.3% |
| bs=1000 sw=10 sl=64 | p50 | 1,116,342 us | 1,086,762 us | 989,300 us | +2.7% | +12.8% |
| bs=1000 sw=10 sl=64 | p95 | 1,155,503 us | 1,157,895 us | 1,031,689 us | -0.2% | +12.0% |
| bs=1000 sw=10 sl=64 | p99 | 1,155,503 us | 1,157,895 us | 1,058,394 us | -0.2% | +9.2% |
</details>
<details><summary>Raw CSV</summary>
```csv
config_idx,batch_size,schema_width,string_len,num_batches,total_ms,total_tuples,total_bytes,tuples_per_sec,mb_per_sec,lat_p50_us,lat_p95_us,lat_p99_us
0,10,10,64,20,536.86,200,128000,373,0.227,25604.39,34994.24,34994.24
1,100,10,64,20,2581.58,2000,1280000,775,0.473,126272.43,153918.83,153918.83
2,1000,10,64,20,22257.94,20000,12800000,899,0.548,1116341.82,1155503.25,1155503.25
```
</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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(jsonl-scan): count the worker slice from the offset, not the file [texera]
Posted by "kz930 (via GitHub)" <gi...@apache.org>.
kz930 commented on PR #7247:
URL: https://github.com/apache/texera/pull/7247#issuecomment-5161376311
Added two, both in the gap you pointed at: an offset split evenly across two workers, and an offset with a limit across three, where the division is uneven and the remainder falls to the last worker. Against the old bounds the first loses a row off the front and the second leaves the last worker empty.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(jsonl-scan): count the worker slice from the offset, not the file [texera]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #7247:
URL: https://github.com/apache/texera/pull/7247#issuecomment-5154332433
<!-- texera-reviewer-suggestion -->
### Automated Reviewer Suggestions
Based on the `git blame` history of the changed files, we recommend the following reviewers:
- **Contributors with relevant context:** `@Ma77Ball`, `@aglinxinyuan`
You can notify them by mentioning `@Ma77Ball`, `@aglinxinyuan` in a comment.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(jsonl-scan): count the worker slice from the offset, not the file [texera]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #7247:
URL: https://github.com/apache/texera/pull/7247#issuecomment-5154344960
## [Codecov](https://app.codecov.io/gh/apache/texera/pull/7247?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 82.63%. Comparing base ([`33cc5d1`](https://app.codecov.io/gh/apache/texera/commit/33cc5d16096aa05095f483ee8724e799c95d7898?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`76712d3`](https://app.codecov.io/gh/apache/texera/commit/76712d3ee43a7ab775df1de6066894ec8cd85737?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #7247 +/- ##
============================================
- Coverage 82.63% 82.63% -0.01%
Complexity 4079 4079
============================================
Files 1162 1162
Lines 46275 46272 -3
Branches 5160 5159 -1
============================================
- Hits 38241 38238 -3
Misses 6343 6343
Partials 1691 1691
```
| [Flag](https://app.codecov.io/gh/apache/texera/pull/7247/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | | *Carryforward flag |
|---|---|---|---|
| [access-control-service](https://app.codecov.io/gh/apache/texera/pull/7247/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `70.00% <ø> (ø)` | | |
| [agent-service](https://app.codecov.io/gh/apache/texera/pull/7247/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `83.65% <ø> (ø)` | | Carriedforward from [33cc5d1](https://app.codecov.io/gh/apache/texera/commit/33cc5d16096aa05095f483ee8724e799c95d7898?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [amber](https://app.codecov.io/gh/apache/texera/pull/7247/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `79.30% <ø> (-0.01%)` | :arrow_down: | Carriedforward from [33cc5d1](https://app.codecov.io/gh/apache/texera/commit/33cc5d16096aa05095f483ee8724e799c95d7898?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [computing-unit-managing-service](https://app.codecov.io/gh/apache/texera/pull/7247/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `43.60% <ø> (ø)` | | Carriedforward from [33cc5d1](https://app.codecov.io/gh/apache/texera/commit/33cc5d16096aa05095f483ee8724e799c95d7898?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [config-service](https://app.codecov.io/gh/apache/texera/pull/7247/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `65.97% <ø> (ø)` | | Carriedforward from [33cc5d1](https://app.codecov.io/gh/apache/texera/commit/33cc5d16096aa05095f483ee8724e799c95d7898?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [file-service](https://app.codecov.io/gh/apache/texera/pull/7247/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `66.80% <ø> (ø)` | | Carriedforward from [33cc5d1](https://app.codecov.io/gh/apache/texera/commit/33cc5d16096aa05095f483ee8724e799c95d7898?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [frontend](https://app.codecov.io/gh/apache/texera/pull/7247/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `83.53% <ø> (ø)` | | Carriedforward from [33cc5d1](https://app.codecov.io/gh/apache/texera/commit/33cc5d16096aa05095f483ee8724e799c95d7898?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [notebook-migration-service](https://app.codecov.io/gh/apache/texera/pull/7247/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `78.89% <ø> (ø)` | | |
| [pyamber](https://app.codecov.io/gh/apache/texera/pull/7247/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `97.36% <ø> (ø)` | | Carriedforward from [33cc5d1](https://app.codecov.io/gh/apache/texera/commit/33cc5d16096aa05095f483ee8724e799c95d7898?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [workflow-compiling-service](https://app.codecov.io/gh/apache/texera/pull/7247/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `26.31% <ø> (ø)` | | Carriedforward from [33cc5d1](https://app.codecov.io/gh/apache/texera/commit/33cc5d16096aa05095f483ee8724e799c95d7898?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
*This pull request uses carry forward flags. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/texera/pull/7247?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>
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(jsonl-scan): count the worker slice from the offset, not the file [texera]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #7247:
URL: https://github.com/apache/texera/pull/7247#issuecomment-5186047382
Backport PR opened: draft #7316 (https://github.com/apache/texera/pull/7316) to `release/v1.2`, assigned to @kz930 — needs manual work because the cherry-pick conflicts.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(jsonl-scan): count the worker slice from the offset, not the file [texera]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #7247:
URL: https://github.com/apache/texera/pull/7247#issuecomment-5154331573
<!-- backport-auto-label-report -->
### Backport auto-label report
This `fix:` PR was checked against each actively-supported release branch. `release/*` labels drive the post-merge backport, so add or remove one to change where this fix lands.
| Release branch | Analysis |
| --- | --- |
| ✅ `release/v1.2` | Change detected on this branch — label added; this fix is queued to backport here. Requested review from @xuang7. |
_[Auto-label run](https://github.com/apache/texera/actions/runs/30726407947)._
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(jsonl-scan): count the worker slice from the offset, not the file [texera]
Posted by "chenlica (via GitHub)" <gi...@apache.org>.
chenlica commented on PR #7247:
URL: https://github.com/apache/texera/pull/7247#issuecomment-5160320997
@eugenegujing Can you also take a look as you recently looked at a similar issue in the CSV File Scan operator?
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(jsonl-scan): count the worker slice from the offset, not the file [texera]
Posted by "eugenegujing (via GitHub)" <gi...@apache.org>.
eugenegujing commented on PR #7247:
URL: https://github.com/apache/texera/pull/7247#issuecomment-5161084643
I think the change LGTM. This problem does not overlap with the problem I found before. Also, it would be better to add more tests for this change, like tests combining offset with multiple workers
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(jsonl-scan): count the worker slice from the offset, not the file [texera]
Posted by "xuang7 (via GitHub)" <gi...@apache.org>.
xuang7 merged PR #7247:
URL: https://github.com/apache/texera/pull/7247
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org