[
https://issues.apache.org/jira/browse/TIKA-4793?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18104140#comment-18104140
]
ASF GitHub Bot commented on TIKA-4793:
--------------------------------------
tballison commented on PR #3009:
URL: https://github.com/apache/tika/pull/3009#issuecomment-5269442298
From my claude. :rofl:
```
1. Success clobbering — EMIT_SUCCESS_PASSBACK results (already emitted to
S3/ES) get replaced with a failure status → retries re-emit already-indexed
docs. Fix: degrade emitData only, keep the status.
2. Pre-check false rejection — the "lower-bound" estimate is empirically
~3.6×
over for short-string-heavy metadata (measured), so big archive results
that
fit under the limit get discarded — the PR's own target case regresses.
3. Estimate unit change breaks other consumers — AsyncEmitter's heap budget
and the DYNAMIC threshold now admit ~2× the real heap/content; needs a
separate heap vs. wire estimate.
4. OOM catch hazards — bypasses the module's exit-on-OOM/restart policy;
and
since the giant result is still live on the stack, the fallback
serialization
can OOM again, escaping into catch(Throwable) and hanging the client until
socket timeout.
5. Coverage gaps — writeIntermediate() and writeCrash() have none of the
protections; same desync bug remains on those paths. awaitAck() still reads
with the hardcoded 100 MB default.
6. Config/docs — setMaxIpcPayloadBytes javadoc wrongly claims requests use
the
built-in default (the limit is bidirectional; a lowered limit makes big
requests die as undiagnosable UNSPECIFIED_CRASH); knob missing from
configuration.adoc.
7. Branch hygiene — needs rebase: merge base predates #2962, so the GitHub
diff shows ~463 additions when the real change is 5 files / +272 (I
test-merged: clean, all 26 tests pass), and branch CI never exercises the
TIKA-4813 timeout model.
8. Architecture — one fix subsumes Nick's finding plus #2, #4, #5, and the
post-check's ~2× memory spike: serialize through a size-capped counting
stream
instead of the three-layer estimate/OOM-catch/post-check.
Note the counting-stream fix also resolves Nick's bug naturally (the tiny
error frame is the only thing ever buffered). Minor test/comment nits from
earlier still apply.
```
> Make the Pipes IPC max payload size configurable (currently hard-coded to 100
> MB)
> ---------------------------------------------------------------------------------
>
> Key: TIKA-4793
> URL: https://issues.apache.org/jira/browse/TIKA-4793
> Project: Tika
> Issue Type: Improvement
> Components: tika-pipes
> Reporter: Srinivasarao Daruna
> Priority: Major
>
> PipesMessage.MAX_PAYLOAD_BYTES (tika-pipes-core) is a compile-time constant
> set to 100 MB:
> //
> tika-pipes/tika-pipes-core/src/main/java/org/apache/tika/pipes/core/protocol/PipesMessage.java:44
> public static final int MAX_PAYLOAD_BYTES = 100 * 1024 * 1024;
> This cap is enforced on the read side of every IPC message in the
> PipesClient↔PipesServer socket protocol. It does not limit the file size
> being parsed (files are fetched server-side by a Fetcher); it limits the size
> of the serialized JSON payload — most critically the PipesResult (parsed
> metadata + extracted text) returned in FINISHED messages.
> Problems with the current implementation:
> 1. Hard-coded, not configurable. Users with very large documents that produce
> large parse results (and no MetadataWriteLimiterFactory configured) have no
> way to raise the cap short of forking the code. There is no corresponding
> field in PipesConfig.
> 2. No write-side guard. PipesMessage.write() applies no limit before writing.
> When the server serializes a PipesResult exceeding 100 MB and sends it, the
> client's PipesMessage.read() throws IOException("Payload length X exceeds
> maximum of 104857600 bytes"). This is caught by the catch-all Exception block
> in PipesClient.waitForServer() and surfaced to the caller as
> UNSPECIFIED_CRASH — a misleading status that provides no indication of the
> root cause.
> Proposed fix:
> 1. Add maxIpcPayloadBytes to PipesConfig with a default of 100 * 1024 * 1024,
> loaded from the "pipes" JSON config section (consistent with all other
> PipesConfig fields).
> 2. Thread the configured value through to both PipesMessage.read() and
> PipesMessage.write(), replacing the hard-coded constant.
> 3. Add a write-side guard in PipesMessage.write() so oversized results are
> caught server-side with a descriptive IOException rather than failing
> silently at the client with UNSPECIFIED_CRASH.
> Example config (proposed):
> {
> "pipes": {
> "maxIpcPayloadBytes": 209715200
> }
> }
> Note: Users hitting this limit should first consider configuring a
> MetadataWriteLimiterFactory to bound extracted-text size, which is the right
> long-term solution for very large documents. But the limit should still be
> configurable for cases where the full content is legitimately needed.
> Affected files:
> -
> tika-pipes/tika-pipes-core/src/main/java/org/apache/tika/pipes/core/protocol/PipesMessage.java
> -
> tika-pipes/tika-pipes-core/src/main/java/org/apache/tika/pipes/core/PipesConfig.java
> -
> tika-pipes/tika-pipes-core/src/main/java/org/apache/tika/pipes/core/PipesClient.java
> -
> tika-pipes/tika-pipes-core/src/main/java/org/apache/tika/pipes/core/server/PipesServer.java
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
