-
Notifications
You must be signed in to change notification settings - Fork 31
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
Add time_buckets to EarningsTracker data #228
Closed
ksedgwic
wants to merge
8
commits into
ZmnSCPxj:master
from
ksedgwic:2024-08-earnings-tracker-time-buckets
Closed
Add time_buckets to EarningsTracker data #228
ksedgwic
wants to merge
8
commits into
ZmnSCPxj:master
from
ksedgwic:2024-08-earnings-tracker-time-buckets
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
ksedgwic
force-pushed
the
2024-08-earnings-tracker-time-buckets
branch
4 times, most recently
from
August 8, 2024 22:16
0d3d72a
to
6a9fe04
Compare
ksedgwic
force-pushed
the
2024-08-earnings-tracker-time-buckets
branch
from
August 8, 2024 22:29
6a9fe04
to
167138c
Compare
This makes it easier to generate test cases by using literal JSON.
This makes it easier to write test cases by comparing to expected values
…tracker A time source is needed for upcoming time buckets change.
This commit modifies the schema of EarningsTracker to allow storing and accessing earning and expenditure data in specific time ranges. All existing strategies and reports still use all data from all time so this PR should not change any balancing behavior. After we've run w/ this for a while we'll have time-based data collected and can evaluate how to improve the strategies.
ksedgwic
force-pushed
the
2024-08-earnings-tracker-time-buckets
branch
from
August 12, 2024 00:33
167138c
to
13a4e8f
Compare
From the users perspective - is anything needed to be done when upgrading to a version that includes this? Or is it "just" that once the upgrade is carried out, downgrading requires some extra manual labor. |
Nothing needs to be done by the user for forward migration. Downgrading is possible (w/o loss of any data) but would need to be done manually or by future code. |
This is subsumed by #230 |
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.
Addresses #227
IMPORTANT - This PR makes a schema change and migrates data!
Abstract
This PR modifies the
EarningsTracker
schema to hold data in "buckets", one per day. This PR only modifies the collection and storage, the strategies and existing status continue to use earnings data from all time for now.Motivation
As described in #227 there are several shortcomings w/ the current storage of
EarningsTracker
data:This PR enables time-based data earnings collection, future PR's should investigate modifying the strategies and reports to take advantage of the ability to filter by time range.
Specification
This PR updates the
EarningsTracker
table to have a "time bucket" column. The timestamps of incoming fee and balance events are quantized to a time bucket, currently one day.The existing strategies and reports use the sql
SUM
to return the same answers that they would with the current schema.The time and nodeid columns are indexed for efficient operations.
Existing legacy data is migrated into the table with a time bucket value of 0. This allows it to be considered and used but eventually be aged out.
Two new CLI commands are added to allow inspecting the new data:
clboss-recent-earnings
:offchain_earnings_tracker
collection inclboss-status
, butonly includes recent earnings and expenditures.
days
(optional): Specifies the number of days to include inthe report. Defaults to a fortnight (14 days) if not provided.
clboss-earnings-history
:nodeid
(optional): Limits the history to a particular node ifprovided. Without this argument, the values are accumulated
across all peers.
and expenditures for each day.
which contains any legacy earnings and expenditures collected by
CLBOSS before daily tracking was implemented.