feat: cache sealing pipeline status #1209
Conversation
storagemarket/provider_dealfilter.go
Outdated
|
|
||
| // sealingPipelineStatus updates the SealingPipelineCache to reduce constant sealingpipeline.GetStatus calls | ||
| // to the lotus-miner. This is to speed up the deal filter processing | ||
| func (p *Provider) sealingPipelineStatus() { |
There was a problem hiding this comment.
Because this call is quite expensive, I'd suggest that we don't do in continuously.
Instead I suggest that when there's a call to get the sealing pipeline status
- check the cache
- if the value has not expired
- return the value
- in the background run a process to get a new value
- if the value has expired
- get a new value and wait for it before returning
- if the value has not expired
There was a problem hiding this comment.
I have tweaked the logic a little more. Since we are processing one deal at a time, there is no chance of clash if we don't use goroutines
Filter requests sealing status:
- Check cache
- if expired or cache error is true
- fetch new status (keep the caller waiting)
- This would also work when Boost starts
- Return the new status to caller and update cache
- if cache is not expired
- Return the cached status to caller (no waiting)
- Don't trigger a refresh in case we have multiple deals processing at the same time
- This would prevent the scenario where goroutine and sealingPipelineStatus() both try to update the cache
- Or we can use locks to prevent this (more complexity)
- if expired or cache error is true
There was a problem hiding this comment.
The problem with the code as implemented is that we're repeatedly calling sealingpipeline.GetStatus, even if there are no deal proposals being made. We have seen that this can be a very expensive call (sometimes takes ~30s). So I would prefer to change the logic to only call sealingpipeline.GetStatus when it's needed. That's what I've proposed above.
There was a problem hiding this comment.
Ahh! Sorry for the confusion in that case.
I have corrected the logic. Pushed new commit with changes.
* cache sealing pipeline status * fix test, surface config * redesign logic, implement suggestions * fix type conversion * fix config override
…ejection (#1211) * feat: cache sealing pipeline status (#1209) * cache sealing pipeline status * fix test, surface config * redesign logic, implement suggestions * fix type conversion * fix config override * fix: BasicDealFilter logic (#1210) * add more data to dealfilterparam * fix BasicDealFilter logic * use IsOffline * remove external filter params
* cache sealing pipeline status * fix test, surface config * redesign logic, implement suggestions * fix type conversion * fix config override
Fixes #1207
Suggested design by DIrk:
if there is no deal filter enabled in config, don't call the method at all
if the deal filter is enabled