tballison commented on PR #2962:
URL: https://github.com/apache/tika/pull/2962#issuecomment-5063614163
I concur with my :robot:
```
Thanks for iterating on this — the threaded limit + `PAYLOAD_LIMIT_EXCEEDED`
status read cleanly. Two small doc mismatches and then I think it's good:
**1. `PipesConfig.setMaxIpcPayloadBytes` javadoc.** It says the limit is
configured "on both ends automatically," but in the final design the configured
value is only consulted on the client's read of server responses (`PipesClient`
line ~376). Server-side reads still use the built-in default. Suggest:
```java
/**
* Sets the maximum IPC payload size in bytes. Must be a positive value.
* This bounds the size of a message the client will accept back from the
* forked server (chiefly the FINISHED result). Request payloads
* (client to server) are small and use the built-in default.
*
* @param maxIpcPayloadBytes positive payload limit in bytes
* @throws IllegalArgumentException if the value is not positive
*/
```
**2. `PayloadLimitExceededException` javadoc.** Two things drift here: it's
thrown for the *configured* per-read limit now, not `MAX_PAYLOAD_BYTES`; and
"the server process itself remains healthy" only holds for a shared server —
with the default per-client forked server the process still exits on the broken
write and the client reconnects next task. Suggest:
```java
/**
* Thrown when an incoming IPC payload's declared length exceeds the
configured
* limit (see {@link
org.apache.tika.pipes.core.PipesConfig#getMaxIpcPayloadBytes()};
* default {@link PipesMessage#MAX_PAYLOAD_BYTES}). The payload bytes were
not
* consumed, so the stream is desynchronized and the connection must be
closed.
* With a shared server the process keeps running (only this connection
ends);
* with the default per-client forked server the process may still exit on
the
* failed write, and the client reconnects on the next task.
*/
```
(The same "server is healthy" wording is in the `PipesClient` catch block —
worth trimming there too.)
Heads up, CI will be red until `PipesParsingHelper.mapStatusToHttpResponse`
(tika-server-core) gets the new `PAYLOAD_LIMIT_EXCEEDED` case — it's an
exhaustive `switch` with no default, so the added enum constant breaks its
compile. Adding it to the `INTERNAL_SERVER_ERROR` arm (alongside the other
TASK_EXCEPTION statuses) is the consistent 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
