Call removeRetiredPools from monitorStakePools.
#2057
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jonathanknowles
added
the
IMPROVEMENT
jonathanknowles
added this to the (ADP-376) Garbage collection of retired pools from the DB milestone
Base automatically changed from
jonathanknowles/remove-retired-pools
to
master
August 24, 2020 11:19
jonathanknowles
force-pushed
the
jonathanknowles/call-remove-retired-pools
branch
from
617ce4b to
5296008
Compare
jonathanknowles
commented
Aug 24, 2020
This comment has been minimized.
This comment has been minimized.
jonathanknowles
force-pushed
the
jonathanknowles/call-remove-retired-pools
branch
4 times, most recently
from
42a8250 to
0c5e82d
Compare
jonathanknowles
commented
Aug 25, 2020
jonathanknowles
force-pushed
the
jonathanknowles/call-remove-retired-pools
branch
from
b1e4d15 to
812b0c6
Compare
|
bors try |
2 tasks
jonathanknowles
changed the title
removeRetiredPools from within monitorStakePools.removeRetiredPools from monitorStakePools.
This comment has been minimized.
This comment has been minimized.
jonathanknowles
commented
Aug 25, 2020
jonathanknowles
added a commit
that referenced
this pull request
Aug 25, 2020
In response to review feedback: #2057 (comment)
jonathanknowles
added a commit
that referenced
this pull request
Aug 25, 2020
In response to review feedback: #2057 (comment)
jonathanknowles
force-pushed
the
jonathanknowles/call-remove-retired-pools
branch
from
bfb0609 to
78e8e04
Compare
jonathanknowles
added a commit
that referenced
this pull request
Aug 25, 2020
In response to review feedback: #2057 (comment)
jonathanknowles
force-pushed
the
jonathanknowles/call-remove-retired-pools
branch
from
78e8e04 to
e0d5eb7
Compare
jonathanknowles
requested review from
Anviking and
KtorZ
and removed request for
Anviking
By only performing garbage collection when we actually have new pool certificates to add, we avoid paying the cost of garbage collection in every single slot.
When garbage collecting pools.
We don't need to see any informational messages when there are no pools to garbage collect. In response to review feedback: https://github.com/input-output-hk/cardano-wallet/pull/2057/files#r476141310
The nested function definitions did not depend on the outer function, so it is safe to move them up one level.
In response to review feedback: #2057 (comment)
In this rationale, we explain why we only garbage collect pools that retired at least two epochs before the current epoch. In response to review feedback: #2057 (comment)
Justification: Some `DBLayer` functions are defined in terms of other `DBLayer` functions, such as `readPoolLifeCycleStatus`, which is defined in terms of `readPoolRegistration` and `readPoolRetirement`. Since mutual recursion between functions defined as fields of a record is somewhat awkward, we currently define auxilliary functions for each of the functions that we need to reuse, and then refer to these auxilliary functions when building the record. However, this creates a layer of indirection and duplication. By using `RecordWildCards`, we can instead build a `DBLayer` object in terms of ordinary functions defined in a `where` clause. Since ordinary functions can refer to one another using mutual recursion, we no longer need auxilliary functons, saving a layer of indirection.
With the use of `RecordWildCards`, these auxilliary functions are no longer necessary, as functions in a where clause can refer to one another directly.
In response to review feedback: #2057 (comment)
In response to review feedback: #2057 (comment)
Nesting a call to `atomically` will result in the nested call blocking indefinitely. This fixes: #2057 (comment) #2057 (comment) This change also removes bracketing for the `MsgStakePoolGarbageCollection` constructor of `StakePoolLog`, as there are no longer any child log entries belonging to the pools engine. (Other log entries pertaining to garbage collection are emitted by the pool DB component, as a result of calling the `removeRetiredPools` DB function.)
jonathanknowles
force-pushed
the
jonathanknowles/call-remove-retired-pools
branch
from
6ae4115 to
5daa926
Compare
|
bors r+ |
Build succeeded |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue Number
#2019
Overview
PR #2057 provided the
removeRetiredPoolsfunction which, when given an epoch, removes pools from the database that retired on or before that epoch.This PR adjusts
monitorStakePoolsto actually callremoveRetiredPools.Frequency of Garbage Collection
Since garbage collection has a runtime cost associated with it, we want to avoid doing it more than necessary, even if the cost is relatively small.
Thus, we only perform garbage collection:
Logging Examples
A single garbage collection invocation (at most once per epoch)
Click to view log sample
Lifecycle of a single pool
Click to view log sample