-
Notifications
You must be signed in to change notification settings - Fork 492
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 trackers for missing block metadata retroactively #2945
Merged
Merged
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
861c564
Add trackers for missing block metadata retroactively
ganeshvanahalli f30dc3d
Merge branch 'master' into backfill-trackersinarbdb-missingblockmetadata
ganeshvanahalli 7ffcd15
address PR comments
ganeshvanahalli 61a7a23
Merge branch 'master' into backfill-trackersinarbdb-missingblockmetadata
ganeshvanahalli a9a21eb
Merge branch 'master' into backfill-trackersinarbdb-missingblockmetadata
ganeshvanahalli ddc0e72
Merge branch 'master' into backfill-trackersinarbdb-missingblockmetadata
ganeshvanahalli 41501fb
Merge branch 'master' into backfill-trackersinarbdb-missingblockmetadata
tsahee File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1207,17 +1207,16 @@ func (s *TransactionStreamer) checkResult(pos arbutil.MessageIndex, msgResult *e | |
"actual", msgResult.BlockHash, | ||
) | ||
// Try deleting the existing blockMetadata for this block in arbDB and set it as missing | ||
if msgAndBlockInfo.BlockMetadata != nil { | ||
if msgAndBlockInfo.BlockMetadata != nil && | ||
s.trackBlockMetadataFrom != 0 && pos >= s.trackBlockMetadataFrom { | ||
batch := s.db.NewBatch() | ||
if err := batch.Delete(dbKey(blockMetadataInputFeedPrefix, uint64(pos))); err != nil { | ||
log.Error("error deleting blockMetadata of block whose BlockHash from feed doesn't match locally computed hash", "msgSeqNum", pos, "err", err) | ||
return | ||
} | ||
if s.trackBlockMetadataFrom != 0 && pos >= s.trackBlockMetadataFrom { | ||
if err := batch.Put(dbKey(missingBlockMetadataInputFeedPrefix, uint64(pos)), nil); err != nil { | ||
log.Error("error marking deleted blockMetadata as missing in arbDB for a block whose BlockHash from feed doesn't match locally computed hash", "msgSeqNum", pos, "err", err) | ||
return | ||
} | ||
if err := batch.Put(dbKey(missingBlockMetadataInputFeedPrefix, uint64(pos)), nil); err != nil { | ||
log.Error("error marking deleted blockMetadata as missing in arbDB for a block whose BlockHash from feed doesn't match locally computed hash", "msgSeqNum", pos, "err", err) | ||
return | ||
} | ||
if err := batch.Write(); err != nil { | ||
log.Error("error writing batch that deletes blockMetadata of the block whose BlockHash from feed doesn't match locally computed hash", "msgSeqNum", pos, "err", err) | ||
|
@@ -1320,7 +1319,71 @@ func (s *TransactionStreamer) executeMessages(ctx context.Context, ignored struc | |
return s.config().ExecuteMessageLoopDelay | ||
} | ||
|
||
func (s *TransactionStreamer) backfillTrackersForMissingBlockMetadata(ctx context.Context) { | ||
if s.trackBlockMetadataFrom == 0 { | ||
return | ||
} | ||
msgCount, err := s.GetMessageCount() | ||
if err != nil { | ||
log.Error("Error getting message count from arbDB", "err", err) | ||
return | ||
} | ||
if s.trackBlockMetadataFrom >= msgCount { | ||
return // We dont need to back fill if trackBlockMetadataFrom is in the future | ||
} | ||
|
||
wasKeyFound := func(pos uint64, prefix []byte) bool { | ||
key := dbKey(prefix, pos) | ||
_, err := s.db.Get(key) | ||
if err == nil { | ||
return true | ||
} | ||
if !dbutil.IsErrNotFound(err) { | ||
log.Error("Error reading key in arbDB while back-filling trackers for missing blockMetadata", "key", key, "err", err) | ||
} | ||
return false | ||
} | ||
|
||
start := s.trackBlockMetadataFrom | ||
finish := msgCount - 1 | ||
for start < finish { | ||
mid := (start + finish + 1) / 2 | ||
if wasKeyFound(uint64(mid), blockMetadataInputFeedPrefix) || | ||
wasKeyFound(uint64(mid), missingBlockMetadataInputFeedPrefix) { | ||
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. easier to make wasKeyFound accept just one index and attempt both blockMetadataInputFeedPrefix and missingBlockMetadataInputFeedPrefix 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. agreed |
||
finish = mid - 1 | ||
} else { | ||
start = mid | ||
} | ||
} | ||
if wasKeyFound(uint64(start), blockMetadataInputFeedPrefix) || | ||
wasKeyFound(uint64(start), missingBlockMetadataInputFeedPrefix) { | ||
return // back-filling not required | ||
} | ||
lastNonExistent := start | ||
|
||
// We back-fill in reverse to avoid fragmentation in case of any failures | ||
batch := s.db.NewBatch() | ||
for i := lastNonExistent; i >= s.trackBlockMetadataFrom; i-- { | ||
if err := batch.Put(dbKey(missingBlockMetadataInputFeedPrefix, uint64(i)), nil); err != nil { | ||
log.Error("Error marking blockMetadata as missing while back-filling", "pos", i, "err", err) | ||
return | ||
} | ||
// If we reached the ideal batch size, commit and reset | ||
if batch.ValueSize() >= ethdb.IdealBatchSize { | ||
if err := batch.Write(); err != nil { | ||
log.Error("Error writing batch with missing trackers to db while back-filling", "err", err) | ||
return | ||
} | ||
batch.Reset() | ||
} | ||
} | ||
if err := batch.Write(); err != nil { | ||
log.Error("Error writing batch with missing trackers to db while back-filling", "err", err) | ||
} | ||
} | ||
|
||
func (s *TransactionStreamer) Start(ctxIn context.Context) error { | ||
s.StopWaiter.Start(ctxIn, s) | ||
s.LaunchThread(s.backfillTrackersForMissingBlockMetadata) | ||
return stopwaiter.CallIterativelyWith[struct{}](&s.StopWaiterSafe, s.executeMessages, s.newMessageNotifier) | ||
} |
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,67 @@ | ||
package arbnode | ||
|
||
import ( | ||
"bytes" | ||
"context" | ||
"encoding/binary" | ||
"testing" | ||
|
||
"github.com/ethereum/go-ethereum/core/rawdb" | ||
"github.com/ethereum/go-ethereum/rlp" | ||
|
||
"github.com/offchainlabs/nitro/arbutil" | ||
) | ||
|
||
func TestTimeboostBackfillingsTrackersForMissingBlockMetadata(t *testing.T) { | ||
t.Parallel() | ||
|
||
ctx, cancel := context.WithCancel(context.Background()) | ||
defer cancel() | ||
|
||
messageCount := uint64(20) | ||
|
||
// Create arbDB with fragmented blockMetadata across blocks | ||
arbDb := rawdb.NewMemoryDatabase() | ||
countBytes, err := rlp.EncodeToBytes(messageCount) | ||
Require(t, err) | ||
Require(t, arbDb.Put(messageCountKey, countBytes)) | ||
addKeys := func(start, end uint64, prefix []byte) { | ||
for i := start; i <= end; i++ { | ||
Require(t, arbDb.Put(dbKey(prefix, i), []byte{})) | ||
} | ||
} | ||
// 12, 13, 14, 18 have block metadata | ||
addKeys(12, 14, blockMetadataInputFeedPrefix) | ||
addKeys(18, 18, blockMetadataInputFeedPrefix) | ||
// 15, 16, 17, 19 are missing | ||
addKeys(15, 17, missingBlockMetadataInputFeedPrefix) | ||
addKeys(19, 19, missingBlockMetadataInputFeedPrefix) | ||
|
||
// Create tx streamer | ||
txStreamer := &TransactionStreamer{db: arbDb} | ||
txStreamer.StopWaiter.Start(ctx, txStreamer) | ||
|
||
backfillAndVerifyCorrectness := func(trackBlockMetadataFrom arbutil.MessageIndex, missingTrackers []uint64) { | ||
txStreamer.trackBlockMetadataFrom = trackBlockMetadataFrom | ||
txStreamer.backfillTrackersForMissingBlockMetadata(ctx) | ||
iter := arbDb.NewIterator([]byte("x"), nil) | ||
pos := 0 | ||
for iter.Next() { | ||
keyBytes := bytes.TrimPrefix(iter.Key(), missingBlockMetadataInputFeedPrefix) | ||
if binary.BigEndian.Uint64(keyBytes) != missingTrackers[pos] { | ||
t.Fatalf("unexpected presence of blockMetadata. msgSeqNum: %d, expectedMsgSeqNum: %d", binary.BigEndian.Uint64(keyBytes), missingTrackers[pos]) | ||
} | ||
pos++ | ||
} | ||
if pos != len(missingTrackers) { | ||
t.Fatalf("number of keys with blockMetadataInputFeedPrefix doesn't match expected value. Want: %d, Got: %d", len(missingTrackers), pos) | ||
} | ||
iter.Release() | ||
} | ||
|
||
// Backfill trackers for missing data and verify that 10, 11 get added to already existing 16, 17, 18, 19 keys | ||
backfillAndVerifyCorrectness(10, []uint64{10, 11, 15, 16, 17, 19}) | ||
|
||
// Backfill trackers for missing data and verify that 5, 6, 7, 8, 9 get added to already existing 10, 11, 16, 17, 18, 19 keys | ||
backfillAndVerifyCorrectness(5, []uint64{5, 6, 7, 8, 9, 10, 11, 15, 16, 17, 19}) | ||
} |
Oops, something went wrong.
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.
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.
start by checking if there is a value for the first tracked block. This will usually succeed then we can be done with just one database access.
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.
ahh I had this check after the binary search (since its a required check), will just move this before search