Plumb CLI arg to control number of TVU receive threads/sockets

[steviez]

Problem

Want to be able to tune the number of sockets / streamer receiver threads we use to listen on the TVU port.

See #105 for more context

Summary of Changes

Plumb the number of TVU sockets to create from validator CLI. No change in behavior by default, the new named constant DEFAULT_NUM_TVU_SOCKETS has the same value (8) as a constant that previously resided within Node::new_with_external_ip()

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 25.00000% with 9 lines in your changes are missing coverage. Please review.

Project coverage is 81.8%. Comparing base (4247a8a) to head (b33f88f).
Report is 1 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #550   +/-   ##
=======================================
  Coverage    81.8%    81.8%           
=======================================
  Files         846      846           
  Lines      229148   229158   +10     
=======================================
+ Hits       187552   187569   +17     
+ Misses      41596    41589    -7     

[gregcusack]

looks good for the most part. just some questions about unsafe and naming

[gregcusack]

lgtm!

[behzadnouri]

Want to be able to tune the number of sockets / streamer receiver threads we use to listen on the TVU port.

Can this tuning be done by modifying the source and building from the source?

The downside I see with exposing these as the validator flags is that a) it makes validator flags even more crowded b) it makes it possible for node operators misspecifying these flags and causing more support burden.

[steviez]

a) it makes validator flags even more crowded

Fair point. However, I'll add this flag is currently hidden, as are the other num-thread arguments (such as the IP echo server one I pushed not too long ago).

b) it makes it possible for node operators misspecifying these flags and causing more support burden.

I think the argument being hidden mitigates some of this concern; --no-port-check has caused us support headache in the past but we kept it around as a hidden argument.

Can this tuning be done by modifying the source and building from the source?

Hypothetically, yes. Even though I'm creating this as a CLI arg, a subsequent PR would be to adjust the default to a well-tuned value after sufficient testing. That tuning could be done by adjusting the value and rebuilding from source as you mentioned

That being said, I still think there are some pros to having it configurable on the CLI (even if it is hidden and we have a "tuned" value as the default):

• We tune for worst case / heaviest load (mnb), but proper tuning may vary by cluster
  • For example, we might need m threads for MNB but m/2 threads might be sufficient for testnet/devnet (ie we can use lesser hardware for smaller clusters)
• Specialized uses-cases might want to tune differently to better that special case
  • For example, RPC nodes may be willing to allocate more threads to something
• Having a CLI arg makes adjustments much easier in case of emergency
  • Again, I think there is a very high bar for testing to ensure that any changes here are proper, but having the ability to adjust without rebuilding a binary is nice just incase.

[steviez]

@behzadnouri - Did my latest response about the CLI arg being hidden address your concern ?

[behzadnouri]

@behzadnouri - Did my latest response about the CLI arg being hidden address your concern ?

No objections to merging.