abhinav-phi opened a new pull request, #2111:
URL: https://github.com/apache/stormcrawler/pull/2111
Fixes #2107
## What happens
`Metadata.addValue(key, value)` is a copy-on-append write path: it allocates
a new array of size *n + 1* and copies the existing *n* values on every call.
`CommaSeparatedToMultivaluedMetadata.filter` removes the key, splits the
value on commas and then calls `addValue` once per token, so splitting *n*
tokens costs n²/2 element copies and the same order of transient allocation.
The key was just removed on the line above, so the whole token set could be
stored with one `setValues` call instead.
`Metadata.addValues(String, String[])` has the same shape: when the key
already exists it loops over `addValue`.
## Where
-
`core/src/main/java/org/apache/stormcrawler/parse/filter/CommaSeparatedToMultivaluedMetadata.java`
— the per-token append loop in `filter`.
- `core/src/main/java/org/apache/stormcrawler/Metadata.java` —
`addValue(String, String)` (the copy-on-append write path) and
`addValues(String, String[])` (loops over `addValue` when the key exists).
## Why it matters
The archetype wires this filter onto page content by default:
`jsoupfilters.json` maps `parse.keywords` to
`//META[@name="keywords"]/@content`, and `parsefilters.json` runs this filter
on `parse.keywords`. So the token list comes from the crawled page, not from
trusted configuration.
At the archetype's 65536 byte `http.content.limit`, the filter costs
hundreds of milliseconds per page on a comma-heavy value (measured below),
which is well inside Storm's default 30 second tuple timeout but is still a
large multiple of normal parse cost, paid again on every replay. Because the
cost is quadratic in the number of tokens, raising `http.content.limit` — which
is routine — multiplies it by the square of the increase, and the library
default of `-1` has no bound at all.
## What this PR changes
1. **Single bulk write in the filter.**
`CommaSeparatedToMultivaluedMetadata.filter` now stores all tokens with a
single `setValues` call. Blank tokens (e.g. between consecutive commas) are
still dropped, exactly as `Metadata.addValue` did, so the stored result is
identical for every input.
2. **Bounded token count.** The number of tokens taken from one value is
capped by a new optional `maxTokens` filter parameter and a warning is logged
when the cap trims, so that a page cannot decide how much metadata it produces.
The default of 65536 is above what a page at the archetype's default 65536 byte
content limit can produce (~32k tokens), while still bounding metadata size
when operators raise `http.content.limit` or leave it unbounded; it can be
lowered per deployment in `parsefilters.json`:
```json
{
"class":
"org.apache.stormcrawler.parse.filter.CommaSeparatedToMultivaluedMetadata",
"name": "CommaSeparatedToMultivaluedMetadata",
"params": {
"keys": ["parse.keywords"],
"maxTokens": 1024
}
}
```
3. **Single-copy append in `Metadata.addValues(String, String[])`.** When
the key already exists it previously looped over `addValue`, i.e. one
whole-array copy per value. It now builds one array and copies each element
once, so the quadratic pattern cannot come back through another caller. As a
side effect it no longer skips blank values in that case — which matches what
it already did when the key was absent, so the method is now self-consistent.
`addValue(String, String)` itself is unchanged and keeps skipping blank values.
## Performance
The reproduction test from the issue (`timeAtArchetypeContentLimit`: 32768
tokens, 65536 chars, the archetype's content limit), run on Windows, JDK 25, in
a fresh surefire JVM, before and after:
- **Before:** 1223 ms (the issue measured 162 ms on faster hardware; the
figure is dominated by ~5·10⁸ element copies either way).
- **After:** 272 ms under identical cold-JVM conditions, and ~30 ms once the
JVM is warm — the remaining cost is the regex split and one linear pass over
the tokens.
More important than the constant factor is the shape: the cost now grows
linearly with the number of tokens instead of quadratically, so raising
`http.content.limit` no longer multiplies the filter cost by the square of the
increase.
## Tests
New
`core/src/test/java/org/apache/stormcrawler/parse/filter/CommaSeparatedToMultivaluedMetadataTest.java`:
- `splittingUsesABulkAppend` counts calls to the copy-on-append path through
a `Metadata` subclass; it fails on main (1000 calls to `addValue` for 1000
tokens) and passes with this change.
- `timeAtArchetypeContentLimit` prints the cost at the archetype content
limit and asserts all 32768 tokens survive.
- `splittingHandlesBlankTokensAndCap` pins down the behaviour of the fix:
blank tokens are still dropped, an all-blank value leaves no entry behind, and
values beyond `maxTokens` are trimmed.
Verified locally:
- `mvn -pl core test` — 417 tests, 0 failures, 0 errors.
- `mvn checkstyle:check test-compile` over the whole reactor — clean.
## Note for reviewers
Unrelated to this change: running the full build locally, the editorconfig
and RAT checks currently flag two pre-existing files on main
(`external/opensearch/dashboards` and
`external/solr/archetype/src/main/resources/archetype-resources/configsets` —
missing final newline, no license header). Both are untouched by this PR.
--
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]
