Fixed a few small issues with recruitment logic on the cluster controller #2241
Conversation
…this will overwrite the region configure if both are present
…e tlogs when they become available in a better satellite location
…es or resolvers from being recruited with good fitness
mpilman
left a comment
mpilman
left a comment
There was a problem hiding this comment.
Looks good to me except for my two minor comments. Please decide whether you want to address or ignore them ;)
| if( now() - startTime < SERVER_KNOBS->WAIT_FOR_GOOD_RECRUITMENT_DELAY && | ||
| ( RoleFitness(SERVER_KNOBS->EXPECTED_TLOG_FITNESS, req.configuration.getDesiredLogs(), ProcessClass::TLog).betterCount(RoleFitness(tlogs, ProcessClass::TLog)) || | ||
| RoleFitness(std::min(SERVER_KNOBS->EXPECTED_PROXY_FITNESS, SERVER_KNOBS->EXPECTED_RESOLVER_FITNESS), std::max(SERVER_KNOBS->EXPECTED_PROXY_FITNESS, SERVER_KNOBS->EXPECTED_RESOLVER_FITNESS), req.configuration.getDesiredProxies()+req.configuration.getDesiredResolvers(), ProcessClass::NoRole).betterCount(bestFitness) ) ) { | ||
| RoleFitness(SERVER_KNOBS->EXPECTED_PROXY_FITNESS, req.configuration.getDesiredProxies(), ProcessClass::Proxy).betterCount(bestFitness.first) || |
There was a problem hiding this comment.
I know this might be a bit out of scope for this PR, but... before this PR the betterMasterExists method was already 190 lines long - now with this change it might well be over 200 lines (didn't check). Would this maybe the right point in time to refactor this a bit and split it into several functions? I have a hard time reviewing this change just because of the size of this method.
There was a problem hiding this comment.
When I merge master recovery logic into the cluster controller, better master exists will hopefully become much simpler. It is probably not worth any refactoring before it is completely overhauled
There was a problem hiding this comment.
that's a fair point, doing this in CC instead of master should indeed make this simpler.
Co-Authored-By: Markus Pilman <[email protected]>
Co-Authored-By: Markus Pilman <[email protected]>
2 participants
No description provided.