You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@texera.apache.org by "roshiiiz (via GitHub)" <gi...@apache.org> on 2026/07/23 00:17:28 UTC
[PR] Fix/file scan memory limit [texera]
roshiiiz opened a new pull request, #6811:
URL: https://github.com/apache/texera/pull/6811
<!--
Thanks for sending a pull request (PR)! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines:
[Contributing to Texera](https://github.com/apache/texera/blob/main/CONTRIBUTING.md)
2. Ensure you have added or run the appropriate tests for your PR
3. If the PR is work in progress, mark it a draft on GitHub.
4. Please write your PR title to summarize what this PR proposes, we
are following Conventional Commits style for PR titles as well.
5. Be sure to keep the PR description updated to reflect all changes.
-->
### What changes were proposed in this PR?
<!--
Please clarify what changes you are proposing. The purpose of this section
is to outline the changes. Here are some tips for you:
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
3. If it is a refactoring, clarify what has been changed.
3. It would be helpful to include a before-and-after comparison using
screenshots or GIFs.
4. Please consider writing useful notes for better and faster reviews.
-->
This PR introduces a strict 100 MB memory allocation limit when scanning files using the standard `binary` attribute type.
**Why is it needed?**
Previously, when users mistakenly attempted to read massive files (e.g., >8GB) using the `binary` attribute type (instead of the streaming `large binary` type), the `ByteArrayOutputStream` would attempt to allocate the entire file into a single contiguous Java byte array. Since standard `byte[]` arrays max out at ~2.14GB, this inherently fails. Worse, on standard developer laptops or lower-memory deployments, this massive allocation attempt triggered immediate `IllegalArgumentException`s or severe JVM Garbage Collection death spirals, causing the `computing-unit-master` to lock up or crash entirely without reporting a user-friendly error to the frontend UI.
**What was changed:**
- Added a `MAX_SAFE_SIZE` limit of 100 MB to `FileScanUtils.scala`.
- During the `binary` scan loop, if the byte stream exceeds 100 MB, it is immediately intercepted and aborted safely.
- Throws a clean, user-friendly `RuntimeException` directly to the frontend directing the user to use the `large binary` attribute type instead for massive files, completely preventing the JVM from thrashing or freezing.
### Any related issues, documentation, discussions?
<!--
Please use this section to link other resources if not mentioned already.
1. If this PR fixes an issue, please include `Fixes #1234`, `Resolves #1234`
or `Closes #1234`. If it is only related, simply mention the issue number.
2. If there is design documentation, please add the link.
3. If there is a discussion in the mailing list, please add the link.
-->
Closes #3721
### How was this PR tested?
<!--
If tests were added, say they were added here. Or simply mention that if the PR
is tested with existing test cases. Make sure to include/update test cases that
check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how
you tested step by step, ideally copy and paste-able, so that other reviewers can
test and check, and descendants can verify in the future. If tests were not added,
please describe why they were not added and/or why it was difficult to add.
-->
**Manual Verification:**
1. Uploaded an 8.9 GB CSV test file to the workspace.
2. Created a workflow with the `File Scan` operator.
3. Configured the operator to use the `binary` attribute type (which intentionally attempts to load the entire file into memory).
4. Ran the workflow.
**Result:** The workflow aborted instantly (in ~3-4 seconds) the moment it crossed the 100MB threshold. It safely threw the custom user-friendly error message in the UI and immediately cleaned up the worker thread without any memory leaks or GC pauses.
### Was this PR authored or co-authored using generative AI tooling?
<!--
If generative AI tooling has been used in the process of authoring this PR,
please include the phrase: 'Generated-by: ' followed by the name of the tool
and its version. If no, write 'No'.
Please refer to the [ASF Generative Tooling Guidance](https://www.apache.org/legal/generative-tooling.html) for details.
-->
Generated-by: Antigravity (DeepMind)
Changed it to 100 Mb instead of 1gb afterwards
<img width="1916" height="870" alt="image" src="https://github.com/user-attachments/assets/a1df28f5-05cd-47b4-b9f7-20db51210fdc" />
--
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(operator): add 100MB memory limit to binary file scan to prevent OOM [texera]
Posted by "roshiiiz (via GitHub)" <gi...@apache.org>.
roshiiiz commented on PR #6811:
URL: https://github.com/apache/texera/pull/6811#issuecomment-5072105008
> Do you think producing OOM is a bug? I think it's natural to produce OOM error if the operator doesn't support reading more than 2GB file. I am okay if the direction is to make the error message user-friendly and guide to using `large-binary` type but I don't think introducing a threshold is a good solution.
I completely understand your point, and I agree that an OOM is natural when hitting that 2GB limit.
The main reason I initially leaned toward a threshold was actually to solve a user experience issue. The original use case that sparked this involved someone uploading a 9GB file. They ended up waiting a really long time for it to process, only for the whole thing to crash at the very end with a generic error. My goal was to introduce a way to 'fail fast' so a new user unaware of the limit doesn't waste 30 minutes waiting on a 10GB file just to get an OOM.
--
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] feat(operator): provide user-friendly error message when binary file scan hits memory limit [texera]
Posted by "aglinxinyuan (via GitHub)" <gi...@apache.org>.
aglinxinyuan merged PR #6811:
URL: https://github.com/apache/texera/pull/6811
--
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/file scan memory limit [texera]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #6811:
URL: https://github.com/apache/texera/pull/6811#issuecomment-5052947377
<!-- 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:** `@aglinxinyuan`
You can notify them by mentioning `@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(operator): add 100MB memory limit to binary file scan to prevent OOM [texera]
Posted by "roshiiiz (via GitHub)" <gi...@apache.org>.
roshiiiz commented on PR #6811:
URL: https://github.com/apache/texera/pull/6811#issuecomment-5083686361
> @roshiiiz Can you confirm #3721 (about login) is related to this PR?
>
> Also if we want to go with this design, it will be good to make this size configurable by the admins.
Nope, I can confirm this PR is completely unrelated to #3721. My frontend changes just strip technical exception prefixes from operator warnings in the global error handler so it doesn't touch the login/registration notification durations at all.
--
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(operator): add 100MB memory limit to binary file scan to prevent OOM [texera]
Posted by "kunwp1 (via GitHub)" <gi...@apache.org>.
kunwp1 commented on PR #6811:
URL: https://github.com/apache/texera/pull/6811#issuecomment-5137389757
Can you update the PR title and description? It's outdated.
--
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(operator): add 100MB memory limit to binary file scan to prevent OOM [texera]
Posted by "chenlica (via GitHub)" <gi...@apache.org>.
chenlica commented on PR #6811:
URL: https://github.com/apache/texera/pull/6811#issuecomment-5082232503
@roshiiiz Can you confirm https://github.com/apache/texera/pull/3721 (about login) is related to this PR?
Also if we want to go with this design, it will be good to make this size configurable by the admins.
--
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(operator): add 100MB memory limit to binary file scan to prevent OOM [texera]
Posted by "roshiiiz (via GitHub)" <gi...@apache.org>.
roshiiiz commented on PR #6811:
URL: https://github.com/apache/texera/pull/6811#issuecomment-5083666206
> > > If the Java limit is about 2 GB, should we set it to 2 GB instead of 100 MB? What if I want to upload a 200MB as binary?
> >
> >
> > Great question! While the theoretical Java byte limit is 2GB, ByteArrayOutputStream doubles its internal buffer capacity every time it resizes. This means reading a 1GB file actually requires >3GB of free heap space just to perform the array copy. On standard local deployments, this instantly triggers OutOfMemoryErrors or severe GC death spirals that crash the worker JVM before it ever hits the 2GB limit.
> > The 100MB limit was chosen as a conservative safety net to guarantee the worker never crashes, while actively nudging users to select the large binary attribute type instead for bigger files (since large binary streams the data chunk-by-chunk and creates zero heap pressure).
> > However, I am completely open to bumping this limit to 500MB if you feel that's a better threshold for standard use cases! Let me know what limit you'd prefer and I'll gladly update the PR.
>
> Do you have some data to support this? For example, can you test the maximum possible file size for this operator? We want to set this limit to upper bound instead of lower bound.
Thanks for the feedback. I actually had a discussion with @kunwp1 in the thread above relating somewhat to this exact concern. We agreed that instead of trying to find the perfect artificial upper bound, it is much cleaner to just remove the hardcoded threshold entirely.
I am updating the PR to let the operator naturally hit the Java array limit on its own, and we can simply catch the resulting OutOfMemoryError or IllegalArgumentException and wrap it in the user-friendly large-binary hint.
--
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(operator): add 100MB memory limit to binary file scan to prevent OOM [texera]
Posted by "aglinxinyuan (via GitHub)" <gi...@apache.org>.
aglinxinyuan commented on PR #6811:
URL: https://github.com/apache/texera/pull/6811#issuecomment-5053288085
If the Java limit is about 2 GB, should we set it to 2 GB instead of 100 MB? What if I want to upload a 200MB as binary?
--
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(operator): provide user-friendly error message when binary file scan hits memory limit [texera]
Posted by "roshiiiz (via GitHub)" <gi...@apache.org>.
roshiiiz commented on PR #6811:
URL: https://github.com/apache/texera/pull/6811#issuecomment-5150512605
> Can you update the PR title and description? It's outdated.
sorry for the delay, Updated.
--
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(operator): add 100MB memory limit to binary file scan to prevent OOM [texera]
Posted by "kunwp1 (via GitHub)" <gi...@apache.org>.
kunwp1 commented on PR #6811:
URL: https://github.com/apache/texera/pull/6811#issuecomment-5071245129
Do you think producing OOM is a bug? I think it's natural to produce OOM error if the operator doesn't support reading more than 2GB file.
--
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(operator): add 100MB memory limit to binary file scan to prevent OOM [texera]
Posted by "roshiiiz (via GitHub)" <gi...@apache.org>.
roshiiiz commented on PR #6811:
URL: https://github.com/apache/texera/pull/6811#issuecomment-5073605400
> If you want a faster fail path, then I wonder why we have to run the operator to check the file size. Did you consider a way to check the file size before running the operator? Also, exceeding 100MB doesn't necessarily mean the operator will cause an OOM. Can you reconsider the idea?
That is a great suggestion, but checking the file size upfront introduces some edge cases. For instance, if a user uploads a .zip file, we cannot determine the uncompressed size of the extracted entries until the operator is actively streaming and unzipping them on the fly. (I might be overthinking this as well)
However, I completely agree that your original suggestion is 100% the cleanest and best approach. I will go ahead and remove the threshold entirely. I'll update the PR to let it hit the Java array limit, and we can simply catch the resulting OutOfMemoryError or IllegalArgumentException and wrap it in user-friendly large-binary hint.
Thanks for the feedback I will update the PR with this approach shortly.
--
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(operator): add 100MB memory limit to binary file scan to prevent OOM [texera]
Posted by "roshiiiz (via GitHub)" <gi...@apache.org>.
roshiiiz commented on PR #6811:
URL: https://github.com/apache/texera/pull/6811#issuecomment-5053352458
> If the Java limit is about 2 GB, should we set it to 2 GB instead of 100 MB? What if I want to upload a 200MB as binary?
Great question! While the theoretical Java byte limit is 2GB, ByteArrayOutputStream doubles its internal buffer capacity every time it resizes. This means reading a 1GB file actually requires >3GB of free heap space just to perform the array copy. On standard local deployments, this instantly triggers OutOfMemoryErrors or severe GC death spirals that crash the worker JVM before it ever hits the 2GB limit.
The 100MB limit was chosen as a conservative safety net to guarantee the worker never crashes, while actively nudging users to select the large binary attribute type instead for bigger files (since large binary streams the data chunk-by-chunk and creates zero heap pressure).
However, I am completely open to bumping this limit to 500MB or even 1GB if you feel that's a better threshold for standard use cases! Let me know what limit you'd prefer and I'll gladly update the PR.
--
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/file scan memory limit [texera]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #6811:
URL: https://github.com/apache/texera/pull/6811#issuecomment-5052983215
<!-- texera-benchmarks-comment -->
## ⚠️ Benchmark changes need a look
🟢 0 better · 🔴 7 worse · ⚪ 8 noise (<±5%) · 0 without baseline
> Compared against main `87ec5f2` 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/29968692891)
| | config | throughput | MB/s | latency | max Δ latest / 7d |
|---|---|---:|---:|---:|---:|
| 🔴 | bs=10 sw=10 sl=64 | 425 | 0.259 | 21,897/33,240/33,240 us | 🔴 +12.9% / 🔴 +106.2% |
| ⚪ | bs=100 sw=10 sl=64 | 955 | 0.583 | 102,484/135,507/135,507 us | ⚪ within ±5% / 🔴 +26.2% |
| 🔴 | bs=1000 sw=10 sl=64 | 1,106 | 0.675 | 895,395/1,078,033/1,078,033 us | 🔴 +14.2% / 🟢 -9.0% |
<details><summary>Baseline details</summary>
Latest main `87ec5f2` from same runner
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---:|---:|---:|---:|---:|
| bs=10 sw=10 sl=64 | throughput | 425 tuples/sec | 482 tuples/sec | 760.2 tuples/sec | -11.8% | -44.1% |
| bs=10 sw=10 sl=64 | MB/s | 0.259 MB/s | 0.294 MB/s | 0.464 MB/s | -11.9% | -44.2% |
| bs=10 sw=10 sl=64 | p50 | 21,897 us | 20,572 us | 12,781 us | +6.4% | +71.3% |
| bs=10 sw=10 sl=64 | p95 | 33,240 us | 29,451 us | 16,118 us | +12.9% | +106.2% |
| bs=10 sw=10 sl=64 | p99 | 33,240 us | 29,451 us | 19,074 us | +12.9% | +74.3% |
| bs=100 sw=10 sl=64 | throughput | 955 tuples/sec | 964 tuples/sec | 988.68 tuples/sec | -0.9% | -3.4% |
| bs=100 sw=10 sl=64 | MB/s | 0.583 MB/s | 0.588 MB/s | 0.603 MB/s | -0.9% | -3.4% |
| bs=100 sw=10 sl=64 | p50 | 102,484 us | 103,940 us | 101,448 us | -1.4% | +1.0% |
| bs=100 sw=10 sl=64 | p95 | 135,507 us | 132,704 us | 107,410 us | +2.1% | +26.2% |
| bs=100 sw=10 sl=64 | p99 | 135,507 us | 132,704 us | 120,258 us | +2.1% | +12.7% |
| bs=1000 sw=10 sl=64 | throughput | 1,106 tuples/sec | 1,107 tuples/sec | 1,023 tuples/sec | -0.1% | +8.1% |
| bs=1000 sw=10 sl=64 | MB/s | 0.675 MB/s | 0.676 MB/s | 0.624 MB/s | -0.1% | +8.1% |
| bs=1000 sw=10 sl=64 | p50 | 895,395 us | 909,780 us | 983,888 us | -1.6% | -9.0% |
| bs=1000 sw=10 sl=64 | p95 | 1,078,033 us | 944,086 us | 1,030,100 us | +14.2% | +4.7% |
| bs=1000 sw=10 sl=64 | p99 | 1,078,033 us | 944,086 us | 1,056,513 us | +14.2% | +2.0% |
</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,470.74,200,128000,425,0.259,21897.25,33239.60,33239.60
1,100,10,64,20,2094.93,2000,1280000,955,0.583,102483.61,135507.28,135507.28
2,1000,10,64,20,18090.66,20000,12800000,1106,0.675,895394.87,1078033.09,1078033.09
```
</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/file scan memory limit [texera]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #6811:
URL: https://github.com/apache/texera/pull/6811#issuecomment-5052964567
### :warning: JUnit XML file not found
The CLI was unable to find any JUnit XML files to upload.
For more help, visit our [troubleshooting guide](https://docs.codecov.com/docs/test-analytics#troubleshooting).
--
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(operator): add 100MB memory limit to binary file scan to prevent OOM [texera]
Posted by "aglinxinyuan (via GitHub)" <gi...@apache.org>.
aglinxinyuan commented on PR #6811:
URL: https://github.com/apache/texera/pull/6811#issuecomment-5073863181
> > If the Java limit is about 2 GB, should we set it to 2 GB instead of 100 MB? What if I want to upload a 200MB as binary?
>
>
>
> Great question! While the theoretical Java byte limit is 2GB, ByteArrayOutputStream doubles its internal buffer capacity every time it resizes. This means reading a 1GB file actually requires >3GB of free heap space just to perform the array copy. On standard local deployments, this instantly triggers OutOfMemoryErrors or severe GC death spirals that crash the worker JVM before it ever hits the 2GB limit.
>
>
>
> The 100MB limit was chosen as a conservative safety net to guarantee the worker never crashes, while actively nudging users to select the large binary attribute type instead for bigger files (since large binary streams the data chunk-by-chunk and creates zero heap pressure).
>
>
>
> However, I am completely open to bumping this limit to 500MB if you feel that's a better threshold for standard use cases! Let me know what limit you'd prefer and I'll gladly update the PR.
Do you have some data to support this? For example, can you test the maximum possible file size for this operator? We want to set this limit to upper bound instead of lower bound.
--
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(operator): add 100MB memory limit to binary file scan to prevent OOM [texera]
Posted by "kunwp1 (via GitHub)" <gi...@apache.org>.
kunwp1 commented on PR #6811:
URL: https://github.com/apache/texera/pull/6811#issuecomment-5073649316
Sounds good! I haven't thought about a zip file but your opinion makes sense that it's not possible to cover all the cases and check the file size beforehand. You can go ahead with the fix.
--
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(operator): add 100MB memory limit to binary file scan to prevent OOM [texera]
Posted by "kunwp1 (via GitHub)" <gi...@apache.org>.
kunwp1 commented on PR #6811:
URL: https://github.com/apache/texera/pull/6811#issuecomment-5073381320
If you want a faster fail path, then I wonder why we have to run the operator to check the file size. Did you consider a way to check the file size before running the 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(operator): add 100MB memory limit to binary file scan to prevent OOM [texera]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #6811:
URL: https://github.com/apache/texera/pull/6811#discussion_r3634977204
##########
common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/source/scan/file/FileScanUtils.scala:
##########
@@ -39,6 +39,33 @@ import scala.collection.mutable
import scala.jdk.CollectionConverters.IteratorHasAsScala
private[file] object FileScanUtils {
+
+ private val MAX_SAFE_SIZE = 100L * 1024L * 1024L // 100 MB
+
+ private def safeToByteArray(entry: InputStream, attributeType: FileAttributeType): Array[Byte] = {
+ val out = new ByteArrayOutputStream()
+ val buffer = new Array[Byte](8192)
+ var bytesRead = entry.read(buffer)
+ var totalBytes = 0L
+
+ while (bytesRead != -1) {
+ totalBytes += bytesRead
+ if (totalBytes > MAX_SAFE_SIZE) {
+ val largeBinaryHint = attributeType match {
+ case FileAttributeType.BINARY => "Please use 'large binary' attribute type instead."
+ case FileAttributeType.SINGLE_STRING =>
+ "Please split the file or use a chunked reading method."
+ case _ => "File is too large to fit in memory."
+ }
+ throw new RuntimeException(
+ s"File exceeds maximum safe memory size of 100MB for '${attributeType.getName}' type. $largeBinaryHint"
+ )
+ }
Review Comment:
`MAX_SAFE_SIZE` is used as a byte count, but the name doesn’t indicate units and the exception message hard-codes "100MB" separately. This can drift if the limit is ever changed, and the constant is less self-explanatory than other size limits in the codebase (which typically include a `_BYTES` suffix).
##########
common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/source/scan/file/FileScanUtils.scala:
##########
@@ -39,6 +39,33 @@ import scala.collection.mutable
import scala.jdk.CollectionConverters.IteratorHasAsScala
private[file] object FileScanUtils {
+
+ private val MAX_SAFE_SIZE = 100L * 1024L * 1024L // 100 MB
+
+ private def safeToByteArray(entry: InputStream, attributeType: FileAttributeType): Array[Byte] = {
+ val out = new ByteArrayOutputStream()
+ val buffer = new Array[Byte](8192)
+ var bytesRead = entry.read(buffer)
+ var totalBytes = 0L
Review Comment:
The new 100MB guard in `safeToByteArray` changes behavior in a safety-critical way (preventing OOM / GC thrash), but there’s no regression/unit test covering the new failure mode. `FileScanUtilsSpec` already exists; adding a test that creates a >100MB file/zip entry and asserts `createTuplesFromFile` fails fast with the expected message would prevent accidental regressions (e.g., limit removal or message drift).
--
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(operator): add 100MB memory limit to binary file scan to prevent OOM [texera]
Posted by "chenlica (via GitHub)" <gi...@apache.org>.
chenlica commented on PR #6811:
URL: https://github.com/apache/texera/pull/6811#issuecomment-5085745333
> Closes #3721
> > @roshiiiz Can you confirm #3721 (about login) is related to this PR?
> > Also if we want to go with this design, it will be good to make this size configurable by the admins.
>
> Nope, I can confirm this PR is completely unrelated to #3721. My frontend changes just strip technical exception prefixes from operator warnings in the global error handler so it doesn't touch the login/registration notification durations at all.
The description mentioned #3721. Please fix it.
--
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(operator): provide user-friendly error message when binary file scan hits memory limit [texera]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #6811:
URL: https://github.com/apache/texera/pull/6811#issuecomment-5150499961
<!-- 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` | Already labeled — this fix is queued to backport here. |
_[Auto-label run](https://github.com/apache/texera/actions/runs/30690634400)._
--
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