[jira] [Comment Edited] (TIKA-4795) ParseBytes: parse-only entrypoint for callers that already hold the document bytes

["Davide Polato (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/TIKA-4795?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18100952#comment-18100952 ] 

Davide Polato edited comment on TIKA-4795 at 8/1/26 1:07 PM:
-------------------------------------------------------------

[~tallison] [~kristian] there is a decision in here that I do not think should be mine.

h3. The bound on the branch does not actually do anything

PARSE_BYTES_MAX_BYTES is 64 MiB and the handler rejects anything bigger with RESOURCE_EXHAUSTED. But grpc-java's default maxInboundMessageSize is about 4 MiB, so a 10 MB request never gets as far as that check. The transport refuses it first. The 64 MiB number is decoration.

It took me a while to notice because the tests use the in-process transport, which ignores the inbound limit entirely. Whatever we settle on needs at least one test against a real Netty server, otherwise we are only exercising a path no client ever takes.

I went looking for a ceiling further down and did not find one on the way in. For ParseBytes the document bytes reach the worker through the spool file: the pipes request carries only the fetch key, and the worker reads the file from disk. maxIpcPayloadBytes, default 100 MiB, bounds the FINISHED result coming back rather than anything going in, and it is configurable in the pipes section of tika-config.json in any case. So the transport limit is the only thing standing between a caller and a large parse.

The hard ceiling is Integer.MAX_VALUE, since maxInboundMessageSize is an int. [~kristian] pointed that out offline. Worth remembering that a unary message is materialised in full in heap before the handler runs, so whatever we set is also a bound on how much heap a single caller can ask the server for. That is why I would not simply set it to the maximum the API allows.

h3. Why I am asking rather than picking one

maxInboundMessageSize is a per-server setting. Raising it for ParseBytes raises it for SaveFetcher, SavePipesIterator and the rest of the management surface, which only ever carry small JSON. That changes what the server will accept from anyone who can reach it, and it is not the sort of thing I want to slip in quietly inside a feature branch.

Three ways to go:

# Leave the default alone, and operators who want large ParseBytes payloads raise maxInboundMessageSize themselves. The server ships conservative. The cost is that ParseBytes is capped near 4 MiB out of the box, which is small for the crawler-shaped callers this ticket is aimed at.
# Raise the server default to 100 MB. ParseBytes then works without configuration and the code stays simple. The cost is that every other RPC now accepts 100 MB too.
# Raise the default to 100 MB and add a ServerInterceptor that puts the limit back near 4 MiB for every method except ParseBytes. Large payloads only where they are meant to be. The cost is another moving part, and one more thing to remember whenever a method is added.

I lean towards the third, and finding no ceiling downstream does not change that. The question was never how large the number can be, it is whether raising it should be global or per method. The second option is defensible if the view is that a Tika gRPC server never faces untrusted callers, but that is a judgement about how people actually deploy this, and I would rather hear it from someone who has seen more deployments than I have.

h3. What has to happen either way

Right now the bound is validated in two places that answer with two different gRPC statuses, RESOURCE_EXHAUSTED in the v2 handler and INVALID_ARGUMENT further down. Only the first one is reachable today, but they should collapse into a single check with a single answer, lined up with whatever the real transport limit turns out to be. Then the real-transport test. And flowControlWindow, which defaults to 1 MiB and is worth raising once messages run to several MB, or the transfer spends round trips on window updates instead of moving bytes.

h3. This is what is keeping the ticket open

The bound is one of the two things this ticket says it needs. The other is cancellation. An in-flight parse is not cancelled when the client disconnects, and with a shared PipesClient, interrupting one request disturbs the worker that is serving the others. That needs its own ticket and its own fix, and I have not started it.

So TIKA-4795 cannot close on what is on the branch today. Either both land, or we agree here to split cancellation out and say so on the ticket instead of leaving it implied.

I will put a number on the branch so the demo can move, but that is a fork making a proof of concept work, not an answer to the above. The branch is at https://github.com/ai-pipestream/tika/tree/TIKA-4795-parseBytes. It sits on top of the TIKA-4766 branch from #2961, so it cannot be a pull request of its own until that one lands.


was (Author: JIRAUSER312865):
[~tallison] [~krickert] there is a decision in here that I do not think should be mine.

h3. The bound on the branch does not actually do anything

PARSE_BYTES_MAX_BYTES is 64 MiB and the handler rejects anything bigger with RESOURCE_EXHAUSTED. But grpc-java's default maxInboundMessageSize is about 4 MiB, so a 10 MB request never gets as far as that check. The transport refuses it first. The 64 MiB number is decoration.

It took me a while to notice because the tests use the in-process transport, which ignores the inbound limit entirely. Whatever we settle on needs at least one test against a real Netty server, otherwise we are only exercising a path no client ever takes.

There is a second limit above us that no configuration can move. Pipes frames are Smile-encoded, which measures at roughly 1.14x expansion on this payload shape, and PipesMessage.MAX_PAYLOAD_BYTES is 100 MiB checked when the frame is read. So an oversized frame does not fail politely in the server, it kills the worker. Usable content tops out somewhere near 87 MiB no matter what the transport is willing to accept.

h3. Why I am asking rather than picking one

maxInboundMessageSize is a per-server setting. Raising it for ParseBytes raises it for SaveFetcher, SavePipesIterator and the rest of the management surface, which only ever carry small JSON. That changes what the server will accept from anyone who can reach it, and it is not the sort of thing I want to slip in quietly inside a feature branch.

Three ways to go:

# Leave the default alone, and operators who want large ParseBytes payloads raise maxInboundMessageSize themselves. The server ships conservative. The cost is that ParseBytes is capped near 4 MiB out of the box, which is small for the crawler-shaped callers this ticket is aimed at.
# Raise the server default to 100 MB. ParseBytes then works without configuration and the code stays simple. The cost is that every other RPC now accepts 100 MB too.
# Raise the default to 100 MB and add a ServerInterceptor that puts the limit back near 4 MiB for every method except ParseBytes. Large payloads only where they are meant to be. The cost is another moving part, and one more thing to remember whenever a method is added.

I lean towards the third. It is the only one that gives ParseBytes room without also widening what the management RPCs will swallow, and an interceptor says the intent out loud instead of burying it in one global number. The second is defensible if the view is that a Tika gRPC server never faces untrusted callers, but that is a judgement about how people actually deploy this, and I would rather hear it from someone who has seen more deployments than I have.

h3. What has to happen either way

Right now the bound is validated in two places that answer with two different gRPC statuses, RESOURCE_EXHAUSTED in the v2 handler and INVALID_ARGUMENT further down. Only the first one is reachable today, but they should collapse into a single check with a single answer. The constant then needs to line up with whatever the real transport limit turns out to be and with the worker ceiling above it. Plus the real-transport test.

h3. This is what is keeping the ticket open

The bound is one of the two things this ticket says it needs. The other is cancellation. Aelled when the client disconnects, and with a shared PipesClient, interrupting one requestdisturbs the worker that is serving the others. That needs its own ticket and its own fix, and I have not started it.

So TIKA-4795 cannot close on what is on the branch today. Either both land, or we agree here to split cancellation out and say so on the ticket instead of leaving it implied.

The branch is at https://github.com/ai-pipestream/tika/tree/TIKA-4795-parseBytes. It sits on top of the TIKA-4766 branch from #2961, so it cannot be a pull request of its own until that one lands.

> ParseBytes: parse-only entrypoint for callers that already hold the document bytes
> ----------------------------------------------------------------------------------
>
>                 Key: TIKA-4795
>                 URL: https://issues.apache.org/jira/browse/TIKA-4795
>             Project: Tika
>          Issue Type: New Feature
>          Components: tika-pipes
>    Affects Versions: 4.0.0
>            Reporter: Davide Polato
>            Priority: Major
>              Labels: grpc, pipes, protobuf
>
> h4. Goal
> Add a parse-only tika-grpc entrypoint that parses the exact bytes supplied by the caller, without asking Tika to fetch or re-fetch the resource.
> This was split from the typed {{Document}} work in [TIKA-4766|https://issues.apache.org/jira/browse/TIKA-4766] / [PR #2961|https://github.com/apache/tika/pull/2961], whose description states that "{{ParseBytes}} is deferred to its own JIRA."
> h4. Motivation
> {{FetchAndParse}} obtains content through a registered fetcher. Callers such as web crawlers have already acquired the resource bytes. Requiring another acquisition path can duplicate the transfer or parse a different representation from the one the caller observed because of redirects, cookies, authentication, robots and rate-limiting decisions, or transient content.
> The parse result should correspond to the exact representation captured by the caller.
> Apache StormCrawler is a concrete consumer. Its parsing bolts already receive the acquired bytes together with URL and protocol metadata. StormCrawler can contribute crawler requirements and input fixtures including truncated payloads, declared-vs-actual charset mismatches, malformed archives, documents containing embedded resources, HTML with {{<base>}}, and distinct URLs with identical content.
> h4. Proposed observable behavior
> This issue defines observable behavior, not the internal buffering, fetcher, or Pipes implementation.
> The request carries the exact byte sequence. It may also carry:
> * an optional opaque caller correlation id, echoed but never interpreted
> * source URI and effective URI when available, plus a base URI for link resolution; these are provenance metadata and must never be dereferenced by the service
> * resource name
> * declared media type and charset hints
> * optional declared content length and digest
> * a truncation flag
> Input must be bounded, and deadline or cancellation must terminate the associated parse work.
> The reply reuses the typed {{Document}} contract introduced by #2961 rather than defining a second parse-result model.
> The current direction discussed in #2961 is to place this on the experimental v2 surface so that it does not gate the 4.0.0 release.
> h4. Open design decisions
> * bounded unary versus client streaming for the request bytes
> * exact service and package placement within v2
> * which provenance fields and parsing hints are necessary
> * whether and how caller-supplied length and digest are validated
> * maximum payload size and behavior for truncated payloads
> h4. Non-goals
> This issue does not define the structured content tree or its Markdown projection, the embedded-document representation, extension payloads such as {{google.protobuf.Any}}, or downstream NLP and embedding enrichment. Those remain separate TIKA-4766 stages and follow-up issues.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)