[GitMehdi-sys]

…ent Storm topology settings #2131

Thank you for contributing to Apache StormCrawler.

In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:

For all changes

• Is there a issue associated with this PR? Is it referenced in the commit message?

• Does your PR title start with #XXXX where XXXX is the issue number you are trying to resolve?

• Has your PR been rebased against the latest commit within the target branch (typically main)?

• Is your initial contribution a single, squashed commit?

• Is the code properly formatted with mvn git-code-format:format-code -Dgcf.globPattern="**/*" -Dskip.format.code=false?

For code changes

• Have you ensured that the full suite of tests is executed via mvn clean verify?
• Have you written or updated unit tests to verify your changes?
• If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?
• If applicable, have you updated the LICENSE file, including the main LICENSE file?
• If applicable, have you updated the NOTICE file, including the main NOTICE file?

[rzo1]

Thanks for picking this up, the documentation of the Storm settings is a good addition on its own.

Before we change the defaults in the archetypes, though, I'd like to see some numbers. Neither the issue nor the PR measures the effect so far, and the archetype config is what most new users start from. Could one of you (@GitMehdi-sys or @GGraziadei) run the archetype topology with batch size 1 and with 32 + dynamic: true, and compare:

• throughput (pages fetched per second, acks per second)
• complete latency of the spout tuples, and whether anything times out
• CPU per worker

ideally once under light load (a few hosts, politeness-bound) and once under sustained load (many hosts). That would also tell us whether 32 is a good value or whether something smaller works as well. jnioche's StatusUpdaterBenchmark (#2213) might help for the status updater part.

Small things: the THIRD-PARTY.txt change looks unrelated and can be dropped, and it would be nice to fill in the PR description with a short summary and the issue number.

[GGraziadei]

Hi @GitMehdi-sys, thanks for bringing this feature to StormCrawler!

I’d suggest narrowing the scope of this PR to improving the documentation rather than enabling the feature by default.

Let me explain my reasoning: there’s no magic number that works for every setup, as the optimal configuration depends on the cluster resources, parallelism, and topology itself. I’d prefer to let users tune the settings according to their specific environments.

One useful approach would be to run the StatusUpdaterBenchmark with different cluster configurations x parameter values, then include the results directly in the documentation. This would give users a practical reference to help them find the optimal configuration for their setup.

What do you think?

[GitMehdi-sys]

Thanks both for the quick feedback, @rzo1 and @GGraziadei.

That makes sense: without measurements I can’t justify changing the defaults that most new users start from, and I agree there’s no single value that fits every setup. I’ve narrowed this PR to documentation only:

kept the documentation of the Storm topology settings that affect a crawl
reverted the archetype crawler-conf.yaml changes, so the defaults stay as they are
dropped the unrelated THIRD-PARTY.txt change
filled in the PR description

As a follow-up, I’m happy to look at StatusUpdaterBenchmark (#2213) with different parallelism and batch-size combinations, and add the results to the docs as a tuning reference. Does that work for you?

[GGraziadei]

Sounds good to me!
If you need to experiment with storm clusters of different size, you can use the storm dev-tools to set it up and customize it as needed.

If you need cloud resources and you're still a student, you could also check out the GitHub Student Developer Pack. I'm not sure what it currently offers, but it used to provide access to various developer tools and cloud resources for students.

[jnioche]

One useful approach would be to run the StatusUpdaterBenchmark with different cluster configurations x parameter values, then include the results directly in the documentation. This would give users a practical reference to help them find the optimal configuration for their setup.

@GGraziadei the StatusUpdaterBenchmark isn’t suitable for this: it calls the status updater’s store() method directly, without a topology, so no tuples are transferred between executors and the producer batching settings have no effect on it. The impact of batching would have to be measured with a real crawl topology, e.g. the one generated by the archetype, using URLFrontier.

Given that most of a tuple’s life is spent waiting in the FetcherBolt queues for politeness reasons, I’m not sure batching would have any noticeable effect, positive or negative.

[GGraziadei]

Thanks a lot for the clarification, @jnioche! Sorry I didn’t check the benchmark myself and instead followed up on a previous message.

I agree that dynamic batching might not make much sense in this context. The idea behind it is to address backpressure, but I agree that in a crawling topology, where most of a tuple’s lifetime is spent waiting in the FetcherBolt queues due to politeness constraints, batching is unlikely to have a significant impact.

One possible scenario could be a very small cluster, although even there, I’d expect the effect to be limited. In any case, we would need to demonstrate its actual impact with some numbers.

Thanks again for the clarification!