Only log if we actually persisted LiquidityManager data (0.2 backport)#4662
Only log if we actually persisted LiquidityManager data (0.2 backport)#4662tnull wants to merge 1 commit into
LiquidityManager data (0.2 backport)#4662Conversation
Previously, we logged "Persisting LiquidityManager..." on each background processor wakeup, which can be very spammy, even on TRACE level. Here, we opt to only log if something actually needed to be repersisted and we did so (in case of failure we're logging that anyways, too).
|
I've assigned @valentinewallace as a reviewer! |
tnull
changed the title
LiquidityManager dataLiquidityManager data (0.2 backport)
| /// Persists the state of the service handlers towards the given [`KVStore`] implementation if | ||
| /// needed. | ||
| /// | ||
| /// Returns `true` if it persisted sevice handler data. |
There was a problem hiding this comment.
Typo: "sevice" → "service"
Review SummaryIssues found:
Issues 1 and 2 are low severity since they only affect logging accuracy, but they make the |
valentinewallace
left a comment
valentinewallace
left a comment
There was a problem hiding this comment.
LGTM aside from Claude's feedback
|
If you're gonna backport something to 0.2, please just go ahead and backport all the pending things with the backport 0.2 tag? Also please include the |
|
Superseded by #4683 |
|
Oh oops forgot about this sorry. |
No worries, question is if we want the claude comments above addressed or not. But seem relatively minor? |
|
If we do we should fix them upstream and backport, not fix them in a backport. |
5 participants
Backport of #4246.
Previously, we logged "Persisting LiquidityManager..." on each background processor wakeup, which can be very spammy, even on TRACE level.
Here, we opt to only log if something actually needed to be repersisted and we did so (in case of failure we're logging that anyways, too).