#621 add batching for cloud updates, fix cloud requests

[mvolikas]

#621 Batching for Solr Cloud updates and other Cloud improvements

• I have tested this in Solr Cloud with multiple shards for the status collection, and 2 replicas.
• I tried to make the minimum set of changes to have "async" support for the Cloud client while keeping most of the advantages offered by solrj out of the box. Of course, this approach is not as robust as the solrj CloudSolrClient, but I think it is enough in the context of crawling.

[mvolikas]

We could add a retry policy to avoid this, but it will complicate the code even more. Additionally, missed Solr updates should not cause long-term issues in the crawl, since pages get refetched after some time, or am I missing something here?

[rzo1]

They only get re-fetched, if the user has enabled re-fetch on error or didn't disable re-fetching at all :)

[mvolikas]

Hmm, then I will add a simple retry policy to be on the safe side.

[mvolikas]

I have added logic that re-queues update batches that failed (e.g. the shard leader we sent the update to went down before handling the request). The batch will be dealt with the next time a flush is triggered.

[rzo1]

I am wondering, if it would be possible, to add some (simple) Testcontainer based unit test? I guess, it would need zookeeper for cloud mode? Maybe there is a way to do it, because it would be really good to have some coverage :)

[mvolikas]

I am wondering, if it would be possible, to add some (simple) Testcontainer based unit test? I guess, it would need zookeeper for cloud mode? Maybe there is a way to do it, because it would be really good to have some coverage :)

In principle, yes, but I had difficulties running everything in containers because the Zookeeper container advertises something like "zookeeper:2181" to the Solr containers inside the Docker network, and from the host (where StormCrawler runs), you cannot resolve "zookeeper".
That being said, I agree that we should have some tests for Solr Cloud, and I will give it another try.

[rzo1]

I am wondering, if it would be possible, to add some (simple) Testcontainer based unit test? I guess, it would need zookeeper for cloud mode? Maybe there is a way to do it, because it would be really good to have some coverage :)

In principle, yes, but I had difficulties running everything in containers because the Zookeeper container advertises something like "zookeeper:2181" to the Solr containers inside the Docker network, and from the host (where StormCrawler runs), you cannot resolve "zookeeper". That being said, I agree that we should have some tests for Solr Cloud, and I will give it another try.

The guys from Apache Camel had a similar issue: https://github.com/apache/camel-quarkus/tree/e5e768acd680b0d78122fb7eee30b0a70947f3f9/integration-tests/solr/src/test - looks like their setup worked (somehow) - maybe we can just port it for our needs?

They bascially rely on docker compose (in two variants) to spin it up. Maybe worth a look?

[mvolikas]

The guys from Apache Camel had a similar issue: https://github.com/apache/camel-quarkus/tree/e5e768acd680b0d78122fb7eee30b0a70947f3f9/integration-tests/solr/src/test - looks like their setup worked (somehow) - maybe we can just port it for our needs?

They bascially rely on docker compose (in two variants) to spin it up. Maybe worth a look?

Thanks @rzo1, this helps.
The issue that remains is that the zookeeper gives back to StormCrawler Solr endpoints like "solr1:8983" that StormCrawler cannot resolve because it's not part of the Docker network.
I should be able to find a way around this, though.

[rzo1]

We can add tests later for it, so no direct blocker from my side

[mvolikas]

We can add tests later for it, so no direct blocker from my side

@rzo1, I will create an issue for this and take care after this PR is merged. Is that ok?

[mvolikas]

This should not happen unless a shard is being split, which, in turn, should not happen in the context of StormCrawler while the crawl is running.