[Davidding4718]

Description

#7146 raised Jackson's StreamReadConstraints.maxStringLength to 64 MB for JacksonEvent. However, the opensearch source deserializes search responses (PIT / search_after / scroll) with its own JacksonJsonpMapper instances, created via the no-arg constructor in OpenSearchClientFactory. Those mappers still use Jackson's default limit of 20,000,000 characters, so a document with a large string field fails before an Event is ever built:

Caused by: com.fasterxml.jackson.core.exc.StreamConstraintsException: String value length (20054016) exceeds the maximum allowed (20000000, from `StreamReadConstraints.getMaxStringLength()`)
    ...
    at org.opensearch.client.json.jackson.JacksonJsonpMapper$JacksonValueParser.deserialize(JacksonJsonpMapper.java:140)
    ...
    at org.opensearch.client.transport.aws.AwsSdk2Transport.performRequest(AwsSdk2Transport.java:222)
    at org.opensearch.client.opensearch.OpenSearchClient.search(OpenSearchClient.java:1386)
    at org.opensearch.dataprepper.plugins.source.opensearch.worker.client.OpenSearchAccessor.searchWithSearchAfter(OpenSearchAccessor.java:284)
    at org.opensearch.dataprepper.plugins.source.opensearch.worker.PitWorker.processIndex(PitWorker.java:192)

Because this happens on read, PitWorker retries the index indefinitely and the pipeline makes no progress.

Changes

• OpenSearchClientFactory now creates its mappers via createJacksonJsonpMapper() / createElasticsearchJacksonJsonpMapper(), which keep each client's default JacksonJsonpMapper configuration and only raise maxStringLength on the mapper's JsonFactory to 64 MB (same as JacksonEvent).
  • Applied to the AwsSdk2Transport (SigV4 / AOS / AOSS), RestClientTransport, and Elasticsearch client paths.
  • JacksonJsonpMapper builds its JacksonJsonProvider from objectMapper.getFactory(), so the constraint applies to response parsing.

Testing

New OpenSearchClientFactoryJsonpMapperTest:

• default JacksonJsonpMapper rejects a 20,000,001-char string (reproduces the issue)
• factory mapper deserializes a 32 MB string
• factory mapper still rejects 64 MB + 1
• Elasticsearch mapper has the raised limit

./gradlew :data-prepper-plugins:opensearch:test --tests 'org.opensearch.dataprepper.plugins.source.opensearch.*' and checkstyle pass.

Open questions

• Should the OpenSearch sink client get the same treatment?
• Longer term, should the source skip/record an unparseable page instead of retrying the whole index?

Issues Resolved

Follow-up to #7146.

Check List

• New functionality includes testing.
• Commits are signed with a real name per the DCO

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[github-actions]

✅ License Header Check Passed

All newly added files have proper license headers. Great work! 🎉

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 7a41c78)

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ No major issues detected

[github-actions]

Persistent review updated to latest commit c0db53d

[github-actions]

Persistent review updated to latest commit 7e57892

[srikanthpadakanti]

Thanks @Davidding4718 for the contribution and the clear write-up! JsonFactories.MAX_JSON_STRING_LENGTH from #7167 already holds this 64 MB value, and the opensearch plugin depends on data-prepper-api. Could we use it here instead of a second constant?

[Davidding4718]

Thanks! I will remove the local constant and reuse.

[srikanthpadakanti]

These tests check the helper methods, but not that the clients use them. I switched the REST, AWS and Elasticsearch transports back to the default mapper one by one, and every test still passed. Could you add a test per transport that checks the client's mapper limit?

[Davidding4718]

Thanks for verifying that. I will add assertions.

[srikanthpadakanti]

Optional, @Davidding4718: I tested this against a real OpenSearch cluster and it works. A page of 5 documents with 25M characters each still fails on the REST client with ContentTooLongException (the 100 MB buffer limit), so batch_size must be small for huge documents. Should we note that in the docs?

[Davidding4718]

Optional, @Davidding4718: I tested this against a real OpenSearch cluster and it works. A page of 5 documents with 25M characters each still fails on the REST client with ContentTooLongException (the 100 MB buffer limit), so batch_size must be small for huge documents. Should we note that in the docs?

Thanks for testing this end to end! I saw the same result locally against OpenSearch 2.19.1 with PIT: a single document with a 20,054,016-character field fails on main with StreamConstraintsException and blocks the whole index, and it goes through fully with this change.

Agreed that the response size limit is worth documenting. It's separate from the string limit: for very large documents, search_options.batch_size has to keep the whole page under the client's response buffer. I'd prefer to keep this PR focused on the string limit and add a note to the opensearch source batch_size docs in a follow-up documentation-website PR.

[github-actions]

Persistent review updated to latest commit 7a41c78

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Configure JsonFactory before constructing mapper JacksonJsonpMapper caches a JsonProvider (built from the ObjectMapper's JsonFactory) during construction. Mutating the factory's StreamReadConstraints after the mapper is instantiated may not propagate to the already-created provider used for parsing responses, leaving the raised limit ineffective. Build the ObjectMapper (and configure its JsonFactory) first, then pass it to the JacksonJsonpMapper constructor. data-prepper-plugins/opensearch/src/main/java/org/opensearch/dataprepper/plugins/source/opensearch/worker/client/OpenSearchClientFactory.java [113-117] static JacksonJsonpMapper createJacksonJsonpMapper() { - final JacksonJsonpMapper jacksonJsonpMapper = new JacksonJsonpMapper(); - raiseMaxStringLength(jacksonJsonpMapper.objectMapper()); - return jacksonJsonpMapper; + final ObjectMapper objectMapper = new ObjectMapper(); + objectMapper.getFactory().setStreamReadConstraints(StreamReadConstraints.builder() + .maxStringLength(JsonFactories.MAX_JSON_STRING_LENGTH) + .build()); + return new JacksonJsonpMapper(objectMapper); } Suggestion importance[1-10]: 3 __ Why: The PR already includes tests (OpenSearchClientFactoryJsonpMapperTest) that verify the raised limit actually takes effect after mutating the factory, so the concern about the limit being ineffective appears unfounded. The suggestion may be a stylistic improvement but does not fix a real bug.	Low

[srikanthpadakanti]

Thanks for the fix and for addressing both comments, @Davidding4718. The shared constant is reused and every client path is now covered by a test. Looks good to me.

[dlvenable]

@Davidding4718 , Thank you for this contribution! It looks good for the opensearch source. But, presumably this also writes to the opensearch sink. I see this JacksonJsonpMapper still uses defaults. So it may end up failing when writing to OpenSearch if a processor doesn't already change the value.

data-prepper/data-prepper-plugins/opensearch/src/main/java/org/opensearch/dataprepper/plugins/sink/opensearch/bulk/PreSerializedJsonpMapper.java

Line 31 in 48d8bc3

innerMapper = new JacksonJsonpMapper();