Skip to content
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

Clean up bitasset_data during maintenance #518

Closed
pmconrad opened this issue Dec 8, 2017 · 10 comments
Closed

Clean up bitasset_data during maintenance #518

pmconrad opened this issue Dec 8, 2017 · 10 comments

Comments

@pmconrad
Copy link
Contributor

pmconrad commented Dec 8, 2017

I just noticed that HKD (and others) contain lots of long-expired pricefeed entries. These clutter up the database and cause unnecessary burden on undo_db.
IMO we should remove expired feed entries from bitasset_data during maintenance.

@abitmore abitmore added this to the Future Non-Consensus-Changing Release 201802 milestone Dec 8, 2017
@oxarbitrage oxarbitrage self-assigned this Jan 1, 2018
@abitmore
Copy link
Member

Please be careful, the cleanup may affect the logic here.

@abitmore abitmore modified the milestones: 201802 - Non-Consensus-Changing Release, Future Consensus-Changing Release Jan 25, 2018
@abitmore
Copy link
Member

This is actually a consensus change. Theoretically, since feed expiration period of an asset can change via asset_update_bitasset_operation, an expired feed can become valid later if hasn't been removed.

@oxarbitrage
Copy link
Member

#598

@abitmore
Copy link
Member

This is a behavior change, thus need a BSIP to clearly define the specifications, E.G. "how old is too old".

@xeroc
Copy link
Member

xeroc commented Feb 21, 2018

@abitmore bitassets have a feed life time. If that timeframe is exceeded, the feed is useless and IMHO can be removed from the object

@oxarbitrage
Copy link
Member

currently as commented by @abitmore feed_lifetime_sec and minimum_feeds can be changed by asset_update_bitasset_operation so we need a hardfork, expired feeds can became valid if they are still available.

we need to wrap #598 inside hardfork and do test cases IMHO.

not sure why a bsip will be needed here.

@pmconrad
Copy link
Contributor Author

It's a change of behaviour / protocol change, therefore a BSIP is required.

@oxarbitrage
Copy link
Member

ok, i'll create it tomorrow.

@oxarbitrage
Copy link
Member

bitshares/bsips#61

@oxarbitrage oxarbitrage modified the milestones: Future Consensus-Changing Release, 201805 - Consensus Changing Release Apr 4, 2018
@abitmore
Copy link
Member

abitmore commented May 8, 2018

Done with #889. Closing.

@abitmore abitmore closed this as completed May 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants