Keep 'opensearch-java' and remove the older 'opensearch' module?

We have both opensearch-java and opensearch; the latter uses the old rest client and is not compatible with recent versions of OpenSearch.

They do exactly the same thing, or at least they should. I would be in favour of removing opensearch then rename opensearch-java into opensearch.

We would need to do some serious testing first but now is a good time to make a substantial change like this as our next release will be a major one and it is fine to break stuff ;-)

What do you think?

Using the new StatusUpdaterBenchmark class on both modules shows unexpected results. I was expecting the legacy implementation to be slower compared to the new one but it turned out to be the other way round.

docker run -d --rm --name opensearch-bench -p 127.0.0.1:9201:9200 -e discovery.type=single-node -e DISABLE_SECURITY_PLUGIN=true -e bootstrap.memory_lock=true -e "OPENSEARCH_JAVA_OPTS=-Xms4g -Xmx4g" --ulimit memlock=-1:-1 --ulimit nofile=65536:65536 opensearchproject/opensearch:3.9.0

storm local target/benchmark-0.1-SNAPSHOT.jar org.apache.stormcrawler.persistence.StatusUpdaterBenchmark org.apache.stormcrawler.opensearch.persistence.StatusUpdaterBolt 4 opensearch-conf.yaml  outlinks.ndjson

where outlinks.ndjson is a massive file at the URLFRontier import format.

with

 opensearch.status.bulkActions: 500
 opensearch.status.flushInterval: "2s"
 opensearch.status.concurrentRequests: 8

gave

opensearch module : 109K OPS
opensearch-java module : 95K OPS

in case you were wondering

URLFrontier: 174K OPS
DigitalPebble's closed-source URLFrontier-compatible backend: 385K OPS

Claude suggested that the

the most likely cause is how the new AsyncBulkProcessor frees up a slot for the next bulk request. It makes the bolt's own thread send bulk requests itself, which the legacy processor never does.

**Main suspect**: the bolt thread ends up making the bulk HTTP call
In AsyncBulkProcessor.java:

The thread pool is ThreadPoolExecutor(1, concurrentRequests, …, new SynchronousQueue<>(), …, CallerRunsPolicy). With the default concurrentRequests=1, it has one worker thread and no queue.
client.bulk(request) is a blocking call that runs on that worker thread.
In whenComplete, the permit is released first, and only then does listener.afterBulk(...) run on the same worker thread. afterBulk is the expensive part: it calls WaitAckCache.processBulkResponse, takes the fair lock, and acks or fails up to bulkActions tuples.
Here is what happens:

Bulk N comes back. The worker releases the permit and starts running afterBulk for bulk N.
Meanwhile the bolt thread has filled the next buffer. It acquires the permit straight away and submits bulk N+1.
The only worker is still busy in afterBulk, and a SynchronousQueue has nowhere to park the task, so the pool rejects it. CallerRunsPolicy then runs client.bulk() on the bolt's executor thread.
The bolt is now blocked for the whole round trip of bulk N+1, so it can't call store() or buffer the next batch.
The legacy BulkProcessor behaves differently:

It uses bulkAsync, so the bolt thread only hands off the request and never waits for the HTTP call.
It releases the semaphore in a finally block after the listener has run, so this race can't happen.
Under load, the legacy processor fills batch N+1 while batch N is in flight. The new one often fills a batch and then waits a full round trip before it can start the next one. That roughly costs you one round trip per batch, and it gets worse when afterBulk is slow. It would also match a result where running more updater instances makes the gap look smaller.

Fix options:

Simplest: release the permit after the listener has run, or make the pool's maximum size concurrentRequests + 1. Either way the bolt never runs a bulk call itself.
Cleaner: use OpenSearchAsyncClient.bulk(), which returns a CompletableFuture, and drop the executor entirely. The behaviour would then match the legacy bulkAsync.

I will open a PR for that change.

see #2225

+1 on removing opensearch and renaming opensearch-java. Now that #2225 is in, I compared the two modules again, beyond the differences the README already lists. A few things I would sort out before the switch:

HybridSpout sends the sort values of the last hit back as FieldValue@... strings in search_after. With nextFetchDate in bucket.sort.field, as in the sample conf, every query sent when a queue empties fails with failed to parse date field [...FieldValue@c82c3cf8], so only the aggregations bring URLs (already in the 3.6.0 and 3.7.0 releases). I have a fix with a test, will open a PR.
bulk items rejected with a 429 fail their tuples straight away, whereas the legacy BulkProcessor retries them with a backoff
bulks are not flushed on size, the legacy one flushes at 5 MB (our README says otherwise, I'll fix it)
smaller ones: a null metadata value fails the tuple in StatusUpdaterBolt, and a missing mapping file only logs a warning when the index has to be created, where the legacy module fails at startup

For the 429s and the size, the client has a BulkIngester which does both. Would you rather switch to it or extend AsyncBulkProcessor?

The module's tests run against OpenSearch 3.5.0. I also ran them against 2.19.6, the version the legacy module tests with, plus the new HybridSpout test, and they all pass. I can run your benchmarks (#2213, #2226) on both modules with TLS and basic auth (your run here had the security plugin disabled), and with HybridSpout.

Do you see any of these as blockers? Happy to do the removal and the rename once these are sorted.

quick one - instead of removing and renaming, we could move the code from opensearch-java to opensearch, this way the things around it like the archetypes would stay in place, maybe with a few minor changes.

Makes sense to me. Moving the implementation into the existing module would let the users keep the archetype and surrounding resources in place. I'm happy to take that on once we've agreed how to handle the remaining differences. See #2237.

+1, as long as the code moves into the existing opensearch module and isn't renamed. Then it replaces the old module in place: same artifact, same packages and class names, same config keys, and only one OpenSearch module afterwards. Users shouldn't get confused.

Before we switch, I'd like to see:

429 retries with backoff and flushing by size. Using BulkIngester seems the easier option, since it does both already.
A short migration note in the PR description for sniff and responseBufferSize, which are no longer supported, plus the other differences dpol1 listed.

@rzo1

Thanks for ur thoughts @rzo1 I'm already working on that 👍

Navigation Menu

Repository navigation

Keep 'opensearch-java' and remove the older 'opensearch' module? #2212

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Keep 'opensearch-java' and remove the older 'opensearch' module? #2212

Description

Activity

jnioche commented on Oct 6, 2026

jnioche commented on Oct 6, 2026

dpol1 commented on Oct 12, 2026

jnioche commented on Oct 12, 2026

dpol1 commented on Oct 12, 2026

rzo1 commented on Oct 14, 2026

dpol1 commented on Oct 14, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions