add farmerbot #509
add farmerbot #509
Conversation
…h/tfgrid-sdk-go into development_farmerbot
rawdaGastan
requested review from
muhamadazmy,
MarioBassem,
ashraffouda,
AlaaElattar,
AbdelrahmanElawady,
xmonader and
sameh-farouk
as code owners
There was a problem hiding this comment.
I feel this is still too complex and not clear as it should. Most of my comments about architecture which i still think is far from optimal. And i feel like i need to re add the same comments from last PR.
In general please consider the following:
- Shared internal state is a no no. If you feel u need to share internal state then u are doing something wrong
- again, state is private, no object need to expose his internal state, if u do so then you are doing something wrong.
- Use small and parameterised functions for example
PeriodicWakeUp(&node)againstPeriodicWakeup()the first one is better, because it checks this specific node if it can be waked up or not. - Smaller functions, leads to clearer and smaller function names.
|
It might be bit too much, but can we check the close issues (and the open ones) https://github.com/threefoldtech/farmerbot/issues (ofc we can ignore anything related to typescript or rmb-peer .. etc) to make sure things are already covered |
| possibleNodes[i].ID < possibleNodes[j].ID // for tests | ||
| }) | ||
|
|
||
| nodeFound := possibleNodes[0] |
There was a problem hiding this comment.
i think we still need the possible nodes list. a power on call for the first node may fail for any reason, and maybe it's best to check with other suitable nodes if that's the case
There was a problem hiding this comment.
Problem here is, booting another node is also subject to the same thing and can get worse for the rest of the offline nodes even. I don't think that should be the behavior, the node may take 1-30 mins too boot, while the farmerbot iteration is every 5mins :D
| continue | ||
| } | ||
|
|
||
| if !node.canClaimResources(nodeOptionsCapacity, f.config.Power.OverProvisionCPU) { |
There was a problem hiding this comment.
how do we make sure that the local state of nodes is not invalidated somehow? a user could deploy contracts on any of the monitored nodes and consume it's resources, which invalidates the state, no?
There was a problem hiding this comment.
the state is rebuilt every 5mins, we can have a 10% buffer around it for optimistic calculations
There was a problem hiding this comment.
will always get real time
Related issues