Updated network retry delay strategy to scale #3306
Conversation
|
|
aakarshg
force-pushed
the
scale_network_retry_delay
branch
3 times, most recently
from
77eb466 to
e5dbedd
Compare
src/network/linkers_socket.cpp
Outdated
| const int connect_fail_retry_cnt = 20; | ||
| const int connect_fail_retries_factor_machine = 25; | ||
| const int connect_fail_retries_scale_factor = static_cast<int>(num_machines_ / connect_fail_retries_factor_machine); | ||
| const int connect_fail_retry_cnt = std::max(20, connect_fail_retries_scale_factor); |
There was a problem hiding this comment.
so it is 20 when num_machines_ = 500, Is this too small?
There was a problem hiding this comment.
it's 20 for num_machines_ less than 500, correct. It's too small if we're spinning say > 1000 num_machines_, in which case time to spin them up and get them running itself takes a while ( especially when we launch them as containers say in kubernetes cluster )
aakarshg
force-pushed
the
scale_network_retry_delay
branch
from
e5dbedd to
00b881a
Compare
|
@StrikerRUS and @guolinke can you PTAL at the PR again when you get a chance? Thanks :) |
|
@StrikerRUS maybe @aakarshg can have better naming for these variates. |
|
@StrikerRUS I put in 2 different values 20 and 25, for following reasons:
But if we are more than 525 machines then the number of retries will be 21 and then scale up accordingly as we increase num_machines. Hope that explains things. About the number of nodes being usually power of 2, that's not true tbh. There are cases ( can't quite go into detail ) where num_machines is more tightly died to number of files needed to train data on. I was looking at around 900 or so machines :) |
|
@aakarshg |
Why can't number of machines be divided by |
I can do that, but then the current behavior will only upto 400 machines, as 400/20 will be the current number of 20 retries. If that's okay then I'll update the PR to just divide by 20 and make code more accessible |
|
I find both 400 and 500 machines in cluster very big number, IMHO. So I don't see any difference between 400 and 500. Is 500 something special threshold? |
aakarshg
force-pushed
the
scale_network_retry_delay
branch
from
00b881a to
615d265
Compare
nothing really, its more of an opinion.
agreed, updated the PR. PTAL again thanks :) |
aakarshg
force-pushed
the
scale_network_retry_delay
branch
from
615d265 to
8389be8
Compare
src/network/linkers_socket.cpp
Outdated
| const int connect_fail_retries_scale_factor = static_cast<int>(num_machines_ / 20); | ||
| const int connect_fail_retry_cnt = std::max(20, connect_fail_retries_scale_factor); |
There was a problem hiding this comment.
@aakarshg Thank you for addressing comments! I'm not sure but maybe it will be better to store 20 in a constant? If we will need to update this value, that constant will allow us to do it in only one place.
There was a problem hiding this comment.
yes that'll be absolutely okay, i'll update the pr :) and apologies for late replies.
aakarshg
force-pushed
the
scale_network_retry_delay
branch
from
8389be8 to
4963ec2
Compare
This allows for network retries, to scale well with the number of machines, and still retains the existing functionality for cases with smaller num_machines ( 500 ) Fixes microsoft#3301
aakarshg
force-pushed
the
scale_network_retry_delay
branch
from
4963ec2 to
0b002e1
Compare
|
This pull request has been automatically locked since there has not been any recent activity since it was closed. To start a new related discussion, open a new issue at https://github.com/microsoft/LightGBM/issues including a reference to this. |
This allows for network retries, to scale well with the
number of machines, and still retains the existing functionality
for cases with smaller num_machines ( 500 )
Fixes #3301