2097: Isolate tika parsing via tika-pipes to ensure reliable timeouts and protection from OOMs

[tballison]

This modernizes the calls to Tika to use tika-pipes. Parsing is done in a forked process to protect the jvm from timeouts and ooms.

This adds some challenges for provisioning -- number of forks and Xmx for each.

However, this now brings the use of Tika back into Tika's security model. Let me know what you think of my bot's work.

Review this carefully. It is 100% 🤖

[rzo1]

Thanks. I have identified some things, will push directly here and run some test swith my sc-warc-demo ;-) - moving to draft now.

[rzo1]

Thanks @tballison, this works nicely. I pushed a commit on top with a few fixes:

• The fork returns the content as metadata (tk:content), which ended up in parse.* for every document. It's stripped now.
• When the fork hit its write limit, the XML came back cut off and the document failed with a SAXParseException instead of being trimmed. The handlers now keep what they got before the cut, and the fork stops at parser.tika.text.maxlength.
• One fork per bolt instance, since execute() only parses one document at a time. I removed parser.tika.pipes.numclients; the docs also said its default was CPU-based, but it's actually 1.
• The fork is started with the worker's own java.home instead of whatever java is on the PATH.
• log4j-core and log4j-slf4j2-impl are excluded, since the Storm worker provides them.
• The HtmlMapper can't be set through parse-context either. The fork always uses DefaultHtmlMapper, so I updated the docs and the warning.
• The archetypes now merge META-INF/tika/*.idx and META-INF/extensions.idx in the shaded jar. Without that, the Tika bolt fails in any shaded topology because only one parsers.idx survives (tesseract-ocr-parser is not registered). This one also affects the plain Tika 4 path, not only pipes.

I tested it on a Storm 3.1.0 cluster in Docker. Timeouts, an OOM in the fork (reported as parse crash, worker unaffected) and a 13MB PDF all behave as expected; the 13MB file went through a temp file without any plugins dir.

One thing for the Tika side: with TimeoutLimits(t, t) every parse logs "progressTimeoutMillis >= totalTaskTimeoutMillis", and there's no way to switch off the stall detector while keeping a total deadline, since progress=0 is rejected. Could Tika allow progress == total, or treat 0 as off?

[tballison]

https://issues.apache.org/jira/browse/TIKA-4919

[tballison]

Please don't merge until I have a chance to take a look today and respond to your observations. Not sure if it is worth converting to draft?

[rzo1]

@tballison I did change it to draft ;-) - I did tests with my local storm docker environment, which where fine (also with your 6kb parser explosion). But we are not in a hurry - please also check and once you think it is fine, go back to ready state (and merge if you feel alike).

[tballison]

Where are you on sizing etc? How many threads go through a bolt? How do we advise users on numConsumers and -Xmx for forked parsers?

[rzo1]

@tballison Good questions. In Storm every bolt executor is a single thread and execute() handles one tuple at a time, so a ParserBolt never has more than one parse in flight. Concurrency comes from the bolt's parallelism, which is why I went with one fork per bolt instance. A second client would just sit there idle.

For memory, I'd tell users to always set parser.tika.pipes.jvmargs, e.g. -Xmx512m, and size the host as worker heap plus one fork heap (and some JVM overhead) per ParserBolt instance on that host. In my Docker test, 512m was fine for normal documents and for a 13MB PDF with 2,700 pages. A nasty 12MB single-page PDF ran out of memory and came back as parse crash, which is what we want.

One thing I noticed while looking into this: if no heap is set, Tika gives each fork -XX:MaxRAMPercentage=60, since numClients is 1. With a few ParserBolt instances on the same host that's way more than 100% of RAM, and Tika's overcommit warning can't catch it because each bolt has its own parser. I think ParserBolt should set its own default when the user doesn't, either a plain -Xmx512m or a percentage split across the ParserBolt tasks in the worker. Any preference?

Also worth documenting: topology.message.timeout.secs needs some headroom beyond parser.tika.timeout. Tuples queue up behind a slow parse, and restarting the fork after a timeout took about 1.5s here.

[dpol1]

Had a look at what reaches the fork and what comes back.

ConfigMerger drops setSocketTimeoutMillis and setJavaPath: the fork runs java from the PATH with a 60s socket timeout. parse.Content-Type is the raw server header, the tuple metadata overwrites what the fork parsed.

The text comes back as one tk:content string reparsed as XML, up to 20M chars twice in the worker.
Same root cause for the three: settings go as a JSON file and results as a Map<String,String>, so neither side can check what it gets.

Smaller things: the fork starts lazily, so a bad jvmarg makes every URL a parse pipes error; a one-line parse in prepare would fail the bolt instead. PARTIAL_TIMEOUT goes out as a trimmed success while the docs say ERROR, intended? maxfilesperprocess defaults to 10000 on the Tika side.

On the heap I'd append -Xmx512m when there's no heap flag, Storm can't give a per-host split.

[rzo1]

Feel free to enhance it @dpol1 (directly in this branch - should be possible)

[tballison]

https://issues.apache.org/jira/browse/TIKA-4932
https://issues.apache.org/jira/browse/TIKA-4931

Those are small enough to get into our 4.1.0 release. Fixing those now. rc2 should go under vote later today.

Thank you @dpol1 !

[dpol1]

@tballison and @rzo1. had a look at the timeouts against Tika main: TimeoutLimits is what kills the parse, socketTimeoutMillis is only liveness and idle shutdown. with TIKA-4931 our setSocketTimeoutMillis(parseTimeout) starts to count, so 1s or less won't even start the fork and an idle fork exits after T. dropped it in the last commit, so we stay on Tika's 60s default, same as 4.0 today. ok with you?

[rzo1]

I think it is fine. Let's wait for Tika 4.1.0 and I guess we are fine.

[tballison]

socketTimeoutMillis is only liveness and idle shutdown.

Right.

[tballison]

Let's wait for Tika 4.1.0

Yes!

[tballison]

@dpol1 please do share other, ahem, sharp edges. 🤣