Add ChannelManager for local channel management and constants to Spout.java

[FelixEngl]

Add constants for Spout & ChannelManager. Modify pom.xml.

(see #980)

Signed-off-by: Felix Engl felix.engl@uni-bamberg.de

Thanks for contributing to StormCrawler, your efforts are appreciated!

Developer Certificate of Origin

By contributing to StormCrawler, you accept and agree to the following terms and conditions (the Developer Certificate of Origin) for your present and future contributions submitted to StormCrawler.
Please refer to the Developer Certificate of Origin section in CONTRIBUTING.md for details.

Developer Certificate of Origin
Version 1.1

Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129

Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.


Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.

Before opening a PR, please check that:

• You've squashed your commits into a single one
• You've described what the PR does or at least point to a related issue
• You've signed-ff your commits with 'git commit -s'
• The code is properly formatted with 'mvn git-code-format:format-code -Dgcf.globPattern=**/*'

Thanks!

[jnioche]

thanks! apart from the use of constants, which is great, the main addition by this PR is the use of the ChannelManager. Can you please explain what it does and why it is needed?

[FelixEngl]

thanks! apart from the use of constants, which is great, the main addition by this PR is the use of the ChannelManager. Can you please explain what it does and why it is needed?

A managed channel provides livecycle management of a connection to a server. If we have the Spout and Status on the same JVM we should use the same ManagedChannel in both nodes.

https://grpc.github.io/grpc-java/javadoc/io/grpc/ManagedChannel.html

(Unless I misunderstood the documentation. But as far as I understood it, best practice would be one instance per Frontier-Host, per JVM)

[jnioche]

thanks! apart from the use of constants, which is great, the main addition by this PR is the use of the ChannelManager. Can you please explain what it does and why it is needed?

A managed channel provides livecycle management of a connection to a server. If we have the Spout and Status on the same JVM we should use the same ManagedChannel in both nodes.

https://grpc.github.io/grpc-java/javadoc/io/grpc/ManagedChannel.html

(Unless I misunderstood the documentation. But as far as I understood it, best practice would be one instance per Frontier-Host, per JVM)

Thanks for the explanation. You probably don't want it per Frontier host as multiple instance could be running on that host with different ports.

There is an opposite argument for having separate channels, in fact, this is why I recently allowed the StatusUpdaterBolt instances to be a multiple of the number of target frontiers. See https://grpc.io/docs/guides/performance/. Since both the spout and the statusupdater operations are streaming ones, the server uses a single thread to deal with the sequence of events. If you have multiple channels, the Frontier runs the operations in parallel. If you run a large crawl hitting a single Frontier and look at the CPU usage, you will notice that it doesn't get very high - that's because often it won't be using all the threads it can use.

See crawler-commons/url-frontier#41 for a related issue. A similar one could be made for the putting of the URLs, so that when dealing with a stream, it would use a pool of threads to deal with each update one by one.

[FelixEngl]

Thanks for the explanation. You probably don't want it per Frontier host as multiple instance could be running on that host with different ports.

There is an opposite argument for having separate channels, in fact, this is why I recently allowed the StatusUpdaterBolt instances to be a multiple of the number of target frontiers. See https://grpc.io/docs/guides/performance/. Since both the spout and the statusupdater operations are streaming ones, the server uses a single thread to deal with the sequence of events. If you have multiple channels, the Frontier runs the operations in parallel. If you run a large crawl hitting a single Frontier and look at the CPU usage, you will notice that it doesn't get very high - that's because often it won't be using all the threads it can use.

See crawler-commons/url-frontier#41 for a related issue. A similar one could be made for the putting of the URLs, so that when dealing with a stream, it would use a pool of threads to deal with each update one by one.

Thank you, I didn't know that page. (I'll read into this a little bit more, the Grpc protocol seems quite convenient. They even got Kotlin support.)
When I understood the paragraph (Special topic) correctly, Grpc has (at the moment) a problem in high load/long long-lived areas (like Spouts/Status) and that's why you decided to use multiple channel instances in the bolts like they said. That makes sense. For the time being I'll remove the ChannelManager from this PR and the associated ones.

What do you think about writing a comment or long term issue relating to that point? Because at some point this Grpc-problem will be fixed and then we should start reusing channels as described in best practices.

[jnioche]

What do you think about writing a comment or long term issue relating to that point? Because at some point this Grpc-problem will be fixed and then we should start reusing channels as described in best practices.

yes, a comment to that effect would be good

[jnioche]

A few minor points, main one being the name of ManagedChannelToolkit but this can be changed later on. Thanks

[jnioche]

Merged. thanks @FelixEngl