Add operator state cache to IndexedChainState #983
Conversation
| @@ -365,3 +392,13 @@ func convertIndexedOperatorInfoGqlToIndexedOperatorInfo(operator *IndexedOperato | |||
| Socket: string(operator.SocketUpdates[0].Socket), | |||
| }, nil | |||
| } | |||
|
|
|||
| // Computes a cache key for the operator state cache. The cache key is a | |||
| // combination of the block number and the quorum IDs. Note: the order of the | |||
There was a problem hiding this comment.
I don't know if this is a problem but noticed that the cache key is dependent on the order you input the quorum ids.
There was a problem hiding this comment.
I think it's a unordered set, so you may eliminate the ordering effect by sorting the them
There was a problem hiding this comment.
Sounds good I'll add sorting.
| cli.IntFlag{ | ||
| Name: OperatorStateCacheSize, | ||
| Usage: "The size of the operator state cache in elements (0 to disable)", | ||
| Value: 0, |
There was a problem hiding this comment.
Should default be 0 or 32?
There was a problem hiding this comment.
What's the estimated size bytes for 32 entries and how many instances of this struct is expected to be created? If it's small enough it should be fine to enable by default
There was a problem hiding this comment.
Depends on the amount of operators but let's assume 200. The order of magnitude is ~1-5mb.
There was a problem hiding this comment.
Actually, it seems this cache is not really useful:
- At different components, it different instances been created so they don't share the cache
- Within same components, like controller, it may not need to access same blockNumber many times, only when it creates batch (https://github.com/Layr-Labs/eigenda/blob/master/disperser/controller/dispatcher.go#L294), and then the blockNumber will advance
Where does the cache help?
| @@ -124,6 +138,14 @@ func (ics *indexedChainState) Start(ctx context.Context) error { | |||
| } | |||
|
|
|||
| func (ics *indexedChainState) GetIndexedOperatorState(ctx context.Context, blockNumber uint, quorums []core.QuorumID) (*core.IndexedOperatorState, error) { | |||
| // Check if the indexed operator state has been cached | |||
| cacheKey := computeCacheKey(blockNumber, quorums) | |||
There was a problem hiding this comment.
If we are using cache then we need to assume that the blockNumber has been finalized already. I believe all users of this function would satisfy that assumption since they're passing in a reference block number.
Why are these changes needed?
Checks