Repository navigation
Conversation
bc7fe49 to
c344ce8
Compare
8732708 to
74f035e
Compare
This commit adds periodical GC to the WAL, every WALGCInterval rounds. Signed-off-by: Yacov Manevich <yacov.manevich@avalabs.org>
| } | ||
|
|
||
| // EpochAwareWAL is a GarbageCollectableWAL that ignores records with an epoch lower than its designated epoch. | ||
| type EpochAwareWAL struct { |
There was a problem hiding this comment.
i dont know why we need an entire struct just for knowing what the epoch is while reading. We already know what the epoch should be when reading the records.
There was a problem hiding this comment.
Because it's more testable this way. Otherwise to test it, we'd have to make an epoch test with every kind of message written in the WAL in the test.
There was a problem hiding this comment.
hmm lemme know what you think of this #629. i think the EpochAwareWal is a bit unnecessarily big.
| if walInterval <= 0 { | ||
| return nil | ||
| } | ||
| if round%uint64(walInterval) == 0 { |
There was a problem hiding this comment.
why do we do rounds and not sequences? We may end up skipping rounds(empty notarization) and thus not garbage collecting anything even though we should.
| wals: []wal.DeletableWAL{staleWAL}, storage: node.storage, existingNode: true, | ||
| }) | ||
|
|
||
| restarted.inst.lock.Lock() |
There was a problem hiding this comment.
using the internal lock is kinda gross here and manually setting epich
| // The WAL is the node's own: the test delays the finalization of the sealing block so that the | ||
| // validators extend the sealed epoch with Telocks, and then takes the WAL as the node wiped it | ||
| // when it moved to the new epoch, which is what a crash in that window would have left behind. | ||
| func TestInstanceRestartSkipsSealedEpochWALRecords(t *testing.T) { |
There was a problem hiding this comment.
yea i think if we dont go with the epoch aware wal, then we can just remove this test all-together plus all the helpers especially these
// hold, when set, picks the messages the network holds back instead of delivering;
// held keeps them, in send order, until releaseHeldMessages delivers them.
hold func(from common.NodeID, msg *common.Message) bool
held []heldMessage
| // A tiny WAL size makes the WAL rotate upon every entry, so garbage collection | ||
| // discards entries round by round rather than in large batches. | ||
| params := paramConfig | ||
| params.WALMaxSizeBytes = 10 |
There was a problem hiding this comment.
so this means the wal will be garbage collected every time we append? if thats the case why do we iterate over all the entries below? wouldn't it be simpler we just check the length and then take the first one?
| // collectRoundsFromWAL returns the rounds of the entries the node's WAL currently | ||
| // holds, across all the WAL files it rotated through. Garbage collected files are | ||
| // deleted, hence contribute no entries. | ||
| collectRoundsFromWAL := func() []uint64 { |
There was a problem hiding this comment.
should this just be one entry?
| // When we commit a sealing block we wipe out the WAL entirely, so ignore this corner case. | ||
| // We only care about the ordinary blocks because we're testing regular garbage collection of the WAL. | ||
| // We also ignore the first walGCInterval rounds because the WAL is not garbage collected until after that. | ||
| if block.SealingBlockInfo() != nil || round < walGCInterval { |
There was a problem hiding this comment.
why would we commit a sealing block if we aren't doing an epoch change?
2 participants
This commit adds periodical GC to the WAL, every WALGCInterval rounds.