-
Notifications
You must be signed in to change notification settings - Fork 162
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
1843 - limit cache when close to tip #1886
base: master
Are you sure you want to change the base?
Conversation
c3e80f5
to
73a8fa6
Compare
73a8fa6
to
34bceb6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
optimiseCaches
requires fixing.
Also optomise
is sometimes used instead of optimise
. Using reduce
instead of optimise would also make it more specific, since the cache is reduced.
Finally, using patterns as suggested could reduce the cases.
-- cPools, cPrevBlock, Cstats, cEpoch | ||
pure c | ||
else pure c | ||
optimiseCaches c = pure c |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Possibly you can pattern match on a ActiveCache False cache
, to avoid this duplicate pure c
case.
atomically $ | ||
writeTVar (cStake ci) stakeCache' | ||
pure $ Right stakeAddrsId | ||
NoCache -> mapLeft (,bs) <$> resolveStakeAddress bs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NoCache -> mapLeft (,bs) <$> resolveStakeAddress bs | |
ActiveCache False ci -> do | |
... | |
_ -> mapLeft (,bs) <$> resolveStakeAddress bs |
Merge the 2 states together
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They need to be different states because when ActiveCache False ci
we still want to use cPools, cPrevBlock, Cstats, cEpoch
caches. But when No Cache
we don't use caches at all.
changed it to:
case cache of
NoCache -> mapLeft (,bs) <$> resolveStakeAddress bs
ActiveCache True _ -> mapLeft (,bs) <$> resolveStakeAddress bs
ActiveCache False ci -> do
...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When it's True, we don't need though. I think this is equivalent.
case cache of
ActiveCache False ci -> do
...
_ -> mapLeft (,bs) <$> resolveStakeAddress bs
-- Update cache. | ||
liftIO $ atomically $ modifyTVar (cTxIds cacheInternal) $ FIFO.insert txIdLedger txId | ||
-- Return ID after updating cache. | ||
ActiveCache isCacheOptomised cacheInternal -> do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ActiveCache isCacheOptomised cacheInternal -> do | |
ActiveCache False cacheInternal -> do | |
... | |
_ -> DB.queryTxId txHash |
Same, merge the 2 states together
Description
This fixes #1843
Checklist
fourmolu
on version 0.10.1.0 (which can be run withscripts/fourmolize.sh
)Migrations
If there is a breaking change, especially a big one, please add a justification here. Please elaborate
more what the migration achieves, what it cannot achieve or why a migration is not possible.