Add functionality to start and shutdown grpc client workers from iovironment

[raakella1]

No description provided.

[codecov-commenter]

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (a51b7f7) 45.55% compared to head (9ebf4df) 45.50%.

Files	Patch %	Lines
src/lib/io_environment.cpp	25.00%	3 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##           master      #78      +/-   ##
==========================================
- Coverage   45.55%   45.50%   -0.05%     
==========================================
  Files          53       53              
  Lines        3956     3958       +2     
  Branches      472      472              
==========================================
- Hits         1802     1801       -1     
- Misses       2024     2026       +2     
- Partials      130      131       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[szmyd]

How does this integrate with nuraft_mesg which doesn't use iomanager?

[raakella1]

How does this integrate with nuraft_mesg which doesn't use iomanager?

Two ways:

1. Do not create any worker threads in nuraft mesg. Let SM estimate the worker threads each of the component needs and create them during startup
2. Let it create its own worker threads and we will rely on ioenvironment to shutdown all workers

[szmyd]

1. Do not create any worker threads in nuraft mesg. Let SM estimate the worker threads each of the component needs and create them during startup
2. Let it create its own worker threads and we will rely on ioenvironment to shutdown all workers

I'm trying to think of a way that nuraft_mesg can present the fact that starting and stopping the client workers is a requirement of the consumer of nuraft_mesg. I don't think it performing half the job is a great pattern (as in option 2). So maybe we can just assert in the nuraft_mesg::init code in the event that the workers haven't been started yet? That would make it clear that the responsibility is on the user of the library.

[szmyd]

What are all these get_instance() calls? Why not just *this like normal? This is a member function, *this is the singleton...

[raakella1]

You are right. I started returning get_instance when I first wrote this and am following it for continuity

[szmyd]

lg in general 👍

[raakella1]

1. Do not create any worker threads in nuraft mesg. Let SM estimate the worker threads each of the component needs and create them during startup
2. Let it create its own worker threads and we will rely on ioenvironment to shutdown all workers

I'm trying to think of a way that nuraft_mesg can present the fact that starting and stopping the client workers is a requirement of the consumer of nuraft_mesg. I don't think it performing half the job is a great pattern (as in option 2). So maybe we can just assert in the nuraft_mesg::init code in the event that the workers haven't been started yet? That would make it clear that the responsibility is on the user of the library.

I like the idea of nuraft_mesg asserting and not doing the half work.