-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
refactor: use epochs to gc eth tx hashes from chain indexer #12516
Changes from 5 commits
b04162b
6587808
29c1566
efe4582
da05e17
da1e02b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,7 +2,6 @@ package index | |
|
||
import ( | ||
"context" | ||
"strconv" | ||
"time" | ||
|
||
logging "github.com/ipfs/go-log/v2" | ||
|
@@ -15,6 +14,8 @@ var ( | |
cleanupInterval = time.Duration(4) * time.Hour | ||
) | ||
|
||
const graceEpochs = 10 | ||
|
||
func (si *SqliteIndexer) gcLoop() { | ||
defer si.wg.Done() | ||
|
||
|
@@ -50,7 +51,7 @@ func (si *SqliteIndexer) gc(ctx context.Context) { | |
|
||
head := si.cs.GetHeaviestTipSet() | ||
|
||
removalEpoch := int64(head.Height()) - si.gcRetentionEpochs - 10 // 10 is for some grace period | ||
removalEpoch := int64(head.Height()) - si.gcRetentionEpochs - graceEpochs | ||
if removalEpoch <= 0 { | ||
log.Info("no tipsets to gc") | ||
return | ||
|
@@ -75,17 +76,24 @@ func (si *SqliteIndexer) gc(ctx context.Context) { | |
// ------------------------------------------------------------------------------------------------- | ||
// Also GC eth hashes | ||
|
||
// Convert gcRetentionEpochs to number of days | ||
gcRetentionDays := si.gcRetentionEpochs / (builtin.EpochsInDay) | ||
if gcRetentionDays < 1 { | ||
log.Infof("skipping gc of eth hashes as retention days is less than 1") | ||
return | ||
} | ||
// Calculate the retention duration based on the number of epochs to retain. | ||
// retentionDuration represents the total duration (in nano seconds) for which data should be retained before considering it for garbage collection. | ||
// graceDuration represents the additional duration (in nano seconds) to retain data after the retention duration. | ||
// Since time.Duration expects a nanosecond value, we multiply the total seconds by time.Second to convert it to nanoseconds. | ||
retentionDuration := time.Duration(si.gcRetentionEpochs*builtin.EpochDurationSeconds) * time.Second | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you add a comment explaining this calculation here ? Why do we need time.Duration(si.gcRetentionEpochs*builtin.EpochDurationSeconds) * time.Second ? |
||
graceDuration := time.Duration(graceEpochs*builtin.EpochDurationSeconds) * time.Second | ||
|
||
// Calculate the total duration to retain data. | ||
totalRetentionDuration := retentionDuration + graceDuration | ||
currHeadTime := time.Unix(int64(head.MinTimestamp()), 0) | ||
// gcTime is the time that is (gcRetentionEpochs + graceEpochs) in nano seconds before currHeadTime | ||
gcTime := currHeadTime.Add(-totalRetentionDuration) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if this is less than or equal to 0, return without doing anything. Also please can we had a test for when |
||
|
||
log.Infof("gc'ing eth hashes before time %s", gcTime.UTC().String()) | ||
|
||
log.Infof("gc'ing eth hashes older than %d days", gcRetentionDays) | ||
res, err = si.stmts.removeEthHashesOlderThanStmt.ExecContext(ctx, "-"+strconv.Itoa(int(gcRetentionDays))+" day") | ||
res, err = si.stmts.removeEthHashesBeforeTimeStmt.ExecContext(ctx, gcTime.Unix()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why do we need to do There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I just wanted to provide as accurate time as possible, so I choose unix. |
||
if err != nil { | ||
log.Errorf("failed to gc eth hashes older than %d days: %w", gcRetentionDays, err) | ||
log.Errorf("failed to gc eth hashes before time %s: %w", gcTime.String(), err) | ||
return | ||
} | ||
|
||
|
@@ -95,5 +103,5 @@ func (si *SqliteIndexer) gc(ctx context.Context) { | |
return | ||
} | ||
|
||
log.Infof("gc'd %d eth hashes older than %d days", rows, gcRetentionDays) | ||
log.Infof("gc'd %d eth hashes before time %s", rows, gcTime.String()) | ||
} |
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.
how is it in nanoseconds ?
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.
It will be seconds 👍🏾 (I will update it)
so what is happening here is we are converting EpochDurationSeconds(30) to its time duration by multiplying with time.seconds.
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.
@aarshkshah1992 So
time.Duration
takes data innanoseconds
(that's why I added nano seconds)so if I just multiply
time.Duration(rententionEpoch * EpochsDurationSeconds)
, it will give 600 (rententionEpoch = 20, EpochsDurationSeconds=30) but that will be in nanoseconds, to get proper time in seconds we have to multiply withtime.Seconds