DiceDB
#117 - (refactor) Multi threading Pre-req - Abstract out Store. And fix the hell fire of all test cases breaking
Abstracted out Store as struct and initialise and use in all modules, rather than accessing and mutation of the globally available store.
This is a pre-req for multi threaded store and a step closer to share nothing model.
Tests are addressed for the change.
Since we are already touching majority of the codebase in this PR, I think we should consider moving the Store object to a different package altogether, create an interface with the required methods and the actual store implementing the interface. The other code paths shouldn't be able to access the internal elements of the Store directly and should only use the publicly exposed ones. As we move towards implementing our own datastores, this will allow us to experiment quickly on other data stores, limit the usage of quirky patterns for the data store, and also protect it against any misuse.
cc @soumya-codes
I second @gauravsarma1992 . We can have a separate package to handle the store object in the internal folder. Also, There can be a separate interface that can be implemented by any store and that can have all desired method definitions mentioned. This will be kind of a factory design pattern and it will allow us to implement any kind of store quickly.
Rough idea
package somepackage
type GeneralStorage interface {
Get(ctx context.Context) *obj
Put(ctx context.Context, *obj)
}
--------------
// seperate packages for stores
package store
type Store struct {
store map[unsafe.Pointer]*Obj
expires map[*Obj]uint64 // Does not need to be thread-safe as it is only accessed by a single thread.
keypool map[string]unsafe.Pointer
storeMutex sync.RWMutex
keypoolMutex sync.RWMutex
WatchList sync.Map // Maps queries to the file descriptors of clients that are watching them.
}
func New() *Store {
return &Store{
store: make(map[unsafe.Pointer]*Obj),
expires: make(map[*Obj]uint64),
keypool: make(map[string]unsafe.Pointer),
}
}
func (s *Store) Get(ctx context.Context) *obj {}
func (s *Store) Put(ctx context.Context, obj *obj){}
-----------------------
// main.go
package main
var s somepackage.GeneralStorage
s:= store.New()
// this can be now extended to any kind of storage but underlying implementations for Get, put can be common
s.put(obj)
obj = s.Get(ctx)
Just a thought.
Interesting idea moving the store. I am keen keen to try it but i don't want to overload the refactor change. Atm the store functions are referred on several parts of code and limiting to a subset of functions could take a while.
I am happy to create a follow up task for this and unblock the multi threaded storage work. What are your thoughts?
@soumya-codes let me also know if possible when you guys are discussing. Want to hear your thoughts on this.