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

beacon: Cull beacon db memory (passivation) #2069

Merged

Conversation

jamescowens
Copy link
Member

@jamescowens jamescowens commented Mar 21, 2021

This pull request implements passivation of beacon elements in the beacon db while leaving the leveldb backing store untouched. This was already anticipated in the version of the beacon db that was in 5.2.x, but required some further adjustments to implement.

Some comments on the theory behind this and the implementation...

In general, we have the problem in Gridcoin of certain contracts being linked by protocol logic. Beacon contracts are an example. There is a relatively complex process for beacons that involve several different paths through possible beacon states. The most typical example is

Advertisement (pending) -> Activation (verification) -> Renewal -> Renewal -> ....-> etc.

Each CPID's series of beacons can be thought of as a "chainlet". This was already implemented in the BeaconRegistry/BeaconDB for 5.2.

Without getting into specifics, the beacon registry for renewals and reversions needs to be able to pull up the "prior" beacon. All beacons that are not the original beacon in a beacon chain" have a previous beacon hash entry, which is a form of a one-way linked list by hash (as opposed to CBlockIndex, which is a two-way linked list by pointer). We don't need the forward direction directly in the beacon object, only the reverse linkage, and unlike the the complexity done in CBlockIndex to turn the hash links in CDiskBlockIndex into pointers for absolute speed, we can just use the hashes and map finds, which are slower but ok, because this is not used as much.

The beacon contract fundamentals are recorded in the beacon db IN THE ORDER they were actually encountered originally in the blockchain and applied. This is the only forward state we need and is used in initialization of the beacon state from leveldb at wallet startup to avoid having to replay all of the contracts themselves, which is very heavyweight.

For normal wallet running conditions though, we do not need all of those beacon records to remain in memory. The m_historical map nominally would contain an entry for every beacon state encountered for every beacon since 180 days from the head OR from the Fern hard fork point of 2053000, whichever is earlier. (This is true up to and including 5.3.0.0.) It is obvious to see that this grows without bound over time as the back of the window is effectively pinned at the Fern hard fork point, although the size of the beacon maps is very small compared to the block map and block index.

The passivation in this PR essentially removes from memory historical records that are not currently being referred to by either m_beacons (the active map) or m_pending (the pending map). It is a primitive "minimum working set" algorithm, and quite frankly, all we need to do the job here. The find function for the beacon db are extended from the normal find function to first look in memory for the provided hash, and if it is not found, transparently load it from leveldb into the map and then provide the iterator or value. This allows functions, such as renewals, which need to pull up the prior beacon record to match the key/verify to work properly with a prior record that was passivated. As a renewal is processed, it will implicitly load back into memory the prior record. The find and insert will set a flag m_needs_passivation to true if they had to load an element into memory that was already passivated before. This will allow a scheduled passivation function to repassivate the element later if it is no longer needed.

During initialization of the BeaconRegistry/BeaconDB during wallet startup, passivation is called on each historical record pointed to by a newly loaded record's previous beacon hash to minimize the memory footprint on load. (Unfortunately, a major downside in the beacon db implementation from a memory use perspective is the need as discussed earlier to preserve the exact order of the contract application as originally encoded in the blockchain. This requires ordering the records by a record sequence number key rather than the native hash as in memory. This necessitates the use of a temporary map ordered by record sequence number populated by an leveldb iterator, which is then used to populate the m_historical map. This temporarily uses more memory but is unavoidable to preserve leveldb performance, especially using regular disks - a leveldb iterator in natural key order is MUCH faster than an iterative find in leveldb.) The m_needs_passivation flag is set after initialization to run passivation across the entire database on the next scheduled run to catch records that could not be passivated on initial load because they were originally pinned by the beacon workflow.

There is a scheduled job using the global scheduling instance that now calls the database wide passivation every 5 minutes. If the m_needs_passivation is false, it will exit without doing anything. If true, it will run passivate_db(), which will iterate through the beacon_db and call passivate on each record, then set the flag to false.

@jamescowens jamescowens added this to the Ingrid milestone Mar 21, 2021
@jamescowens jamescowens self-assigned this Mar 21, 2021
@jamescowens jamescowens changed the title beacon: Cull beacon db memory beacon: Cull beacon db memory (passivation) Mar 21, 2021
@jamescowens jamescowens marked this pull request as ready for review March 22, 2021 00:37
@jamescowens
Copy link
Member Author

jamescowens commented Mar 22, 2021

So far this runs well in both mainnet and testnet, but fails the deep reorg test. See the attached log. The auditsnapshotaccruals at 2196000 (before the new newbie fix height) is supposed to have mismatches and it does. When reorganized back to the original height these mismatches should disappear. They don't.

See the included log:
reorg_run_1-20210322-failure.log

@jamescowens
Copy link
Member Author

Ah... the failure is a false positive. It fails on master this deep too. I just realized this reorg doesn't rebuild the accruals after it comes back, which is actually done by a 5.3 client ON FIRST RUN if the accrual version doesn't match. This will not be done on a deep reorg and back. A reorg this deep is unnatural anyway. I will retest it with a shallower one.

@jamescowens
Copy link
Member Author

Ok. I did a shallower reorg from 2204007 to 2199000 and back and it looks good. I am calling it a possible success, because the auditsnapshotaccruals run after the reorg but before the reorg back shows all matches but one CPID not present. The auditsnapshotaccruals run after the reorg back matches the original auditsnapshotaccruals perfectly.

reorg_run_2-20210322-possible_success.log

@jamescowens jamescowens force-pushed the cull_beacon_db_memory branch 2 times, most recently from fad3bb2 to b700ba5 Compare March 23, 2021 21:05
src/gridcoin/beacon.h Outdated Show resolved Hide resolved
src/gridcoin/beacon.h Outdated Show resolved Hide resolved
src/gridcoin/beacon.h Outdated Show resolved Hide resolved
This includes adjustments to implement passivation and modification
of the shared pointer implementation.
This refactors the BeaconRegistry tests in beacon_tests.cpp to
eliminate the duplicated code between the testnet and mainnet tests.
Copy link
Member

@div72 div72 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK d036b82.

@jamescowens jamescowens merged commit f98753c into gridcoin-community:development Mar 27, 2021
jamescowens added a commit to jamescowens/Gridcoin-Research that referenced this pull request Apr 4, 2021
Added
 - doc: Invite users to donate to Gridcoin Foundation gridcoin-community#1997 (@smoe)
 - rpc: Add "getburnreport" RPC function gridcoin-community#2049 (@cyrossignol)
 - doc: Add undocumented "-showorphans" GUI option to help text gridcoin-community#2058 (@cyrossignol)
 - beacon: Cull beacon db memory (passivation) gridcoin-community#2069 (@jamescowens)
 - gui: Avoid refreshing GUI researcher status while out-of-sync gridcoin-community#2068 (@cyrossignol)
 - consensus: Reimplement checkpoint-based spam protection gridcoin-community#2084 (@cyrossignol)
 - consensus: Verify hardened checkpoints on start up gridcoin-community#2087 (@cyrossignol)

Changed
 - gui: Clarify overview page "stake" field gridcoin-community#2056 (@cyrossignol)
 - doc: Update Copyright headers gridcoin-community#2059 (@barton2526)
 - gui: Update Qt Linguist localization files gridcoin-community#2063 (@cyrossignol)
 - build: update dependencies gridcoin-community#2064 (@barton2526)
 - net: Reduce default connection limit back to 125 gridcoin-community#2066 (@cyrossignol)
 - build: openssl patch gridcoin-community#2074 (@barton2526)
 - translation: Translate /src/qt/locale/bitcoin_en.ts in pt_PT gridcoin-community#2083 (@DjMVeiga)
 - log: Adjust logging gridcoin-community#2076 (@jamescowens)
 - gui: Change scraper tab to Inconsolata monospace font gridcoin-community#2085 (@jamescowens)
 - researcher: Change beacon deferment fix to reference nActiveBeforeSB gridcoin-community#2092 (@jamescowens)

Removed
 - net: Clean up mandatory protocol version transition gridcoin-community#2080 (@cyrossignol)
 - refactor: Remove LessVerbose() function gridcoin-community#2089 (@cyrossignol)

Fixed
 - beacon: Fix a subtle error in renewal chain walker gridcoin-community#2054 (@jamescowens)
 - researcher: Fix "malformed CPID" status for some pool projects gridcoin-community#2052 (@cyrossignol)
 - lint: Misc Typos gridcoin-community#2060 (@barton2526)
 - lint: remove identified duplicate includes gridcoin-community#2061 (@barton2526)
 - gui: Fix splash screen block height progress gridcoin-community#2057 (@cyrossignol)
 - gui: Fix garbage placeholders in some tx notification localizations gridcoin-community#2070 (@cyrossignol)
 - build: Patch libzip to fix mingw compile regression for mingw 9.2+ gridcoin-community#2082 (@jamescowens)
 - gui: Fix shutdown response for failed core init gridcoin-community#2088 (@cyrossignol)
 - researcher: Fix deferment of beacon renewal in superblock window gridcoin-community#2090 (@cyrossignol)
 - gui: Fix typo in beacon status refresh gridcoin-community#2091 (@div72)
jamescowens added a commit to jamescowens/Gridcoin-Research that referenced this pull request Apr 4, 2021
Added
 - doc: Invite users to donate to Gridcoin Foundation gridcoin-community#1997 (@smoe)
 - rpc: Add "getburnreport" RPC function gridcoin-community#2049 (@cyrossignol)
 - gui: Add stats export reminder to beacon wizard auth page gridcoin-community#2050 (@cyrossignol)
 - doc: Add undocumented "-showorphans" GUI option to help text gridcoin-community#2058 (@cyrossignol)
 - beacon: Cull beacon db memory (passivation) gridcoin-community#2069 (@jamescowens)
 - gui: Avoid refreshing GUI researcher status while out-of-sync gridcoin-community#2068 (@cyrossignol)
 - consensus: Reimplement checkpoint-based spam protection gridcoin-community#2084 (@cyrossignol)
 - consensus: Verify hardened checkpoints on start up gridcoin-community#2087 (@cyrossignol)

Changed
 - test: autogenerate data headers gridcoin-community#2030 (@div72)
 - doc: Change copyright years to 2021 gridcoin-community#2042 (@caraka)
 - gui: Clarify overview page "stake" field gridcoin-community#2056 (@cyrossignol)
 - doc: Update Copyright headers gridcoin-community#2059 (@barton2526)
 - gui: Update Qt Linguist localization files gridcoin-community#2063 (@cyrossignol)
 - build: update dependencies gridcoin-community#2064 (@barton2526)
 - net: Reduce default connection limit back to 125 gridcoin-community#2066 (@cyrossignol)
 - build: openssl patch gridcoin-community#2074 (@barton2526)
 - translation: Translate /src/qt/locale/bitcoin_en.ts in pt_PT gridcoin-community#2083 (@DjMVeiga)
 - log: Adjust logging gridcoin-community#2076 (@jamescowens)
 - gui: Change scraper tab to Inconsolata monospace font gridcoin-community#2085 (@jamescowens)
 - researcher: Change beacon deferment fix to reference nActiveBeforeSB gridcoin-community#2092 (@jamescowens)

Removed
 - net: Clean up mandatory protocol version transition gridcoin-community#2080 (@cyrossignol)
 - refactor: Remove LessVerbose() function gridcoin-community#2089 (@cyrossignol)

Fixed
 - build: Fix depends cross-compilation for macOS gridcoin-community#2038 (@cyrossignol)
 - build: Deal with Qt depends .qmake.stash file gridcoin-community#2048 (@cyrossignol)
 - beacon: Fix a subtle error in renewal chain walker gridcoin-community#2054 (@jamescowens)
 - researcher: Fix "malformed CPID" status for some pool projects gridcoin-community#2052 (@cyrossignol)
 - lint: Misc Typos gridcoin-community#2060 (@barton2526)
 - lint: remove identified duplicate includes gridcoin-community#2061 (@barton2526)
 - gui: Fix splash screen block height progress gridcoin-community#2057 (@cyrossignol)
 - gui: Fix garbage placeholders in some tx notification localizations gridcoin-community#2070 (@cyrossignol)
 - build: Patch libzip to fix mingw compile regression for mingw 9.2+ gridcoin-community#2082 (@jamescowens)
 - gui: Fix shutdown response for failed core init gridcoin-community#2088 (@cyrossignol)
 - researcher: Fix deferment of beacon renewal in superblock window gridcoin-community#2090 (@cyrossignol)
 - gui: Fix typo in beacon status refresh gridcoin-community#2091 (@div72)
@jamescowens jamescowens deleted the cull_beacon_db_memory branch April 5, 2021 01:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants