dpol1 opened a new pull request, #2242: URL: https://github.com/apache/stormcrawler/pull/2242
Second part of #2212, on top of #2241. As [@jnioche suggested](https://github.com/apache/stormcrawler/issues/2212#issuecomment-6130734067), the opensearch-java code moves into `external/opensearch`, so the artifact `stormcrawler-opensearch`, its archetype, the configuration files and the dashboards stay where they are. `external/opensearch-java` is removed. The files that exist in both modules are overwritten in place, so `git blame` still attributes the unchanged lines to their legacy commits. The files that exist only in opensearch-java (`AsyncBulkProcessor` and a few tests) are moved with `git mv`. This also holds after a squash merge: on main the shared files show as modified, not deleted and re-added. Besides the move: - In core, `Metadata.setValue(key, null)` stores a null value, and `addValue` keeps it next to the new one. `StatusUpdaterBolt` copied the values of every metadata key with `List.of`, and `IndexerBolt` the values of the keys with more than one value. `List.of` throws on a null element, so the tuple failed. Both bolts now use `Arrays.asList`, and the null is written into the JSON array, as the legacy module does. `NullMetadataValueTest` fails without the fix. - `sniff` and `responseBufferSize` are removed from the module's and the archetype's `opensearch-conf.yaml` and from the configuration reference. The WARN for these keys now points to `external/opensearch/README.md`. - opensearch-java had no test that sends a request to a local socket to check that only the configured addresses get the credentials, and no test of `WaitAckCache.shutdown()`. I ported the legacy ones. Two legacy tests are not ported: the sniffer scheme test, since there is no sniffer, and the credentials lookup without an origin, since the transport's preemptive Basic scheme looks the credentials up with the host of the request. - With `opensearch.disable.tls.validation`, the connection manager that installs the trust-all TLS strategy did not set pool limits, so the HttpClient 5 defaults applied: 5 connections per node and 25 in total, instead of the transport's 10 and 30. It now uses the transport's limits. The README section "Upgrading from versions before 4.0" lists what changes for users of `stormcrawler-opensearch` and `stormcrawler-opensearch-java`: configuration keys, behaviour and the types custom code has to use. These are the differences I propose to accept for 4.0. Should any of them match the legacy module before the merge? The ones I would look at first: the client no longer discovers nodes by sniffing, a missing mapping resource logs a WARN where the legacy module failed at startup, and `HybridSpout` sends its refill queries one at a time instead of in parallel. A few things worth a look: - "replace the legacy opensearch sources with the opensearch-java ones" does not compile on its own, because the POM changes in the next commit. - `THIRD-PARTY.txt` still lists the high-level client and the sniffer. I left it to the license-file PR that CI opens after a merge to main. The module suite passes locally with Docker. With a project generated from the archetype, the default topology (`AggregationSpout`) and the archetype's OpenSearch 2.19.4, I crawled a small local site for 3 minutes. The pages were fetched and indexed, the missing page ended as `FETCH_ERROR`, metrics were written, and the logs show no mapping or query error. For that run I added the archetype's `docker-compose.yml` to the project by hand, with a declaration for its volume, and allowed private addresses in the fetcher, since the local site runs on the Docker network. I haven't run `StatusUpdaterBenchmark` against the legacy module. -- 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]
