Optimise Contract Sizes #773
Conversation
peterjan
force-pushed
the
pj/contract-sizes
branch
from
004978c to
a080cf4
Compare
peterjan
force-pushed
the
pj/contract-sizes
branch
from
a080cf4 to
39f3f7d
Compare
stores/metadata.go
Outdated
| var nullContracts []size | ||
| var dataContracts []size | ||
| if err := s.db.Transaction(func(tx *gorm.DB) error { | ||
| // use WHERE NOT EXISTS to catch contracts that would otherwise have been caught by a LEFT JOIN |
There was a problem hiding this comment.
it would be bit more helpful to explain what this query does rather than what alternative we had for it. Someone who looks at this in a week probably won't remember what the left join was about.
so e.g. "fetch all contracts that don't have any useful sectors in them anymore"
| `, rhpv2.SectorSize, rhpv2.SectorSize). | ||
| Scan(&rows). | ||
| Error; err != nil { | ||
| var nullContracts []size |
There was a problem hiding this comment.
null requires some insider knowledge on the query we had before. I'd recommend something like unusedContracts and usedContracts to indicate that it's just contracts without sectors.
There was a problem hiding this comment.
I updated the comments on the query to better explain what's happening. I'd like to keep nullContract because for me it's the only name that really makes sense. I don't like unusedContracts or emptyContracts because if they're unused or empty why would you be interested in their size field for pruning...
| LEFT JOIN contract_sectors cs ON cs.db_contract_id = c.id | ||
| WHERE c.fcid = ? | ||
| `, rhpv2.SectorSize, rhpv2.SectorSize, fileContractID(id)). | ||
| SELECT contract_size as size, CASE WHEN contract_size > sector_size THEN contract_size - sector_size ELSE 0 END as prunable FROM ( |
There was a problem hiding this comment.
Is this new query faster? Or did you just refactor it?
There was a problem hiding this comment.
It's marginally faster. It avoids doing MAX and COUNT multiple times. I never really cared b/c I figured the database engine would optimise that for me.
This prunable data query is rather slow because we have to
LEFT JOIN contract_sectors. We need that because we want to include contracts without sectors too. I figured in this case it makes sense to split one query into two: one where we useWHERE NOT EXISTSto catch everything we would miss out on if we were toINNER JOIN.In the contract size query (so fetching size details for a single contract) I kept the
LEFT JOINsince it's bound by thefcidanyway. I considered getting rid of it all together because we only use it in the worker to escape early. We could also return a custom error and return a 400 when the user supplies a contract to prune but there's no sectors to prune. This would remove abusroute though (/contract/:id/size) and I wasn't sure whether that was a good or a bad thing.. might be usable at one point but otoh less routes less trouble... cc @ChrisSchinnerl wdyt.NOTE: I didn't have to alter the tests because they broke when switching to anINNER JOIN