[
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)
