-
Notifications
You must be signed in to change notification settings - Fork 18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Map counters holder component #394
Map counters holder component #394
Conversation
availabilityProvider := availabilityCommon.AvailabilityProvider{} | ||
dataAvailabilityTypes := availabilityProvider.GetAllAvailabilityTypes() | ||
|
||
countersMap := make(map[data.ObserverDataAvailabilityType]*mapCounter) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
countersMap := make(map[data.ObserverDataAvailabilityType]*mapCounter) | |
countersMap := make(map[data.ObserverDataAvailabilityType]*mapCounter, len(dataAvailabilityTypes)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
) | ||
|
||
// MapCountersHolder handles multiple counters map based on the data availability | ||
type MapCountersHolder struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can be unexported and use an interface on circularQueueNodesProvider
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, done
countersMap map[uint32]uint32 | ||
counterForAllNodes uint32 | ||
mutCounters sync.RWMutex | ||
positionsHolder *mapCounters.MapCountersHolder |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
interface instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done. but didn't pass it to the constructor since the functionality of the circularQueueNodesProvider
depends a lot on the actual implementation, so stubs in tests wouldn't help.
I just wanted to extract some code in new components so the code is easier to read :D
The base branch was changed.
return 0, errInvalidAvailability | ||
} | ||
|
||
if numNodes == 0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this can be placed on L37
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right. done
|
||
// ComputeShardPosition returns the shard position based on the availability and the shard | ||
func (mch *mapCountersHolder) ComputeShardPosition(availability data.ObserverDataAvailabilityType, shardID uint32, numNodes uint32) (uint32, error) { | ||
counterMap, exists := mch.countersMap[availability] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no protection needed because we set once and read many times
return 0, errInvalidAvailability | ||
} | ||
|
||
if numNodes == 0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can be moved on L52
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right. done
52e4e0c
bea1ef6
into
feat/snapshotless-observer-support
added new components so we can have different circular queues for each observer type (normal / snapshotless)