nornir-automation / gornir

https://godoc.org/github.com/nornir-automation/gornir
Apache License 2.0
156 stars 15 forks source link

Use of pointer and value semantics for Inventory, Host, etc. #11

Open nleiva opened 5 years ago

nleiva commented 5 years ago

Today, most of the concrete types use pointer semantics; *Gornir, *Inventory, *Host, etc. This is OK when sharing data that you want to modify, however this means we need to be careful to avoid data races when launching multiple goroutines. We might need to implement mutexes to protect us from this.

We can potentially change most of the types to value semantics to pass copies of the values around. This also means we can store the data in the Stack and not the Heap.

Not sure at this point what the best approach is in this case, but wanted to raise awareness of this so we can address it somewhere along the line.

dbarrosop commented 5 years ago

My concern is that if the inventory is large passing by value might bring performance issues with large datasets. We should probably benchmark this.

If we decide to stick with pointers we could add a mutex to each struct and make sure all the attributes are accessed via Getters/Setters to make sure it's properly handled.

nleiva commented 5 years ago

Right now a Gornir is very light weight.

type Gornir struct {
    Inventory *Inventory // Inventory for the object
    Logger    Logger     // Logger for the object
}

And so it's the Inventory

type Inventory struct {
    Hosts map[string]*Host // Hosts represents a collection of Hosts
}

As both store pointer values (32 or 64 bits each depending on the architecture) and not the actual Host values.

Anyways, we can revisit this later on with more data to better understand the tradeoff of introducing mutex vs data size to pass around.