WIP: Fix host counter #2884
WIP: Fix host counter #2884
Conversation
…en this function is called multiple times from negotiate.go line 252, this prevents adding values multiple times.
…tract and not a renewal
modules/host/storageobligations.go
Outdated
| // Increase contract count by one only if this is a new filecontract. | ||
| if len(so.SectorRoots) == 0 { | ||
| h.financialMetrics.ContractCount++ | ||
| } |
There was a problem hiding this comment.
I'm not sure about this part. I guess it depends on the precise meaning of ContractCount. Even if you renew a contract, the host still needs to track the old contract until it ends, so it might make sense to increment ContractCount even for renewed contracts.
There was a problem hiding this comment.
You are right! I reverted the commits. Was confused about the 'renewal' word, where it actually is a new storage obligation we need to track.
|
This seems basically correct to me. Although, it makes me suspect that the host state isn't being properly rolled back in the event of an error. For example, there's a |
… new contract and not a renewal" This reverts commit 9d08da8. Contract counter should be increased as it is forming a new obligation for new contracts as well for renewals.
…tion. When this function is called multiple times from negotiate.go line 252, this prevents adding values multiple times." This reverts commit e15c6d8. When there is an error with the queuing of the action items, the storage obligation will be removed with status 'obligationRejected' and the function will return. Placing the financial metrics update at the end of the function will in this case never be reached.
there is nothing to revert.
…ror. Financial metrics are only updated when there are no errors.
AcceptTransactionSet call returns with an error. This should reset the host to the original state. However, the rejected storage obligation will stay in the database as rejected without SectorRoots.
|
@lukechampine I reverted the old and added some new commits. Please take a look if I am not missing something. Whenever The only thing that still bothers me is the way a storage obligation with
Before the financial metrics are updated it checks: If not, is it possible to define a new storage obligation status, |
@DavidVorick This is a first attempt to fix the host financial metrics that are off most of the time for all the hosts.
I noticed that whenever my host financial metrics increased a lot in a small period of time, there were at the same time consensus errors in the
host.log. These errors always repeated themselves 6 times with 15s interval. See host.log.Therefore, I looked at
negotiate.goand found out that there is a loop that callserr = h.managedAddStorageObligation(so).Worst case this function is called 6 times if there are issues with the acceptance of the transaction set (leading to the logged errors). Every time this function is called on the same storage obligation, the host financial metrics are updated.
This PR tries to fix the double counting issue. This PR will not prevent additional insertions into the host database. These additional insertions could lead to several other bugs where a host can't provide a storage proof when a storage obligation ends.
I know the focus is on upgrading the renter. But this could potentially fix a lot of errors for the hosts. Any feedback is welcome...
cc @lukechampine