Batch index updates #332
Batch index updates #332
Conversation
|
Warning Rate limit exceeded@sideninja has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 15 minutes and 17 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe changes revolve around batch processing operations in the pebble database to ensure atomicity, meaning that if an error occurs during the batch process, no partial data is persisted. This includes modifying methods to accept batch parameters, updating signatures, and altering error handling across various files including Changes
Assessment against linked issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (24)
- api/api.go (1 hunks)
- bootstrap/bootstrap.go (5 hunks)
- models/mocks/Engine.go (5 hunks)
- services/ingestion/engine.go (6 hunks)
- services/ingestion/engine_test.go (22 hunks)
- services/ingestion/mocks/EventSubscriber.go (3 hunks)
- services/traces/engine.go (1 hunks)
- services/traces/engine_test.go (3 hunks)
- services/traces/mocks/Downloader.go (3 hunks)
- storage/index.go (6 hunks)
- storage/index_testsuite.go (20 hunks)
- storage/mocks/AccountIndexer.go (5 hunks)
- storage/mocks/BlockIndexer.go (10 hunks)
- storage/mocks/ReceiptIndexer.go (6 hunks)
- storage/mocks/TraceIndexer.go (4 hunks)
- storage/mocks/TransactionIndexer.go (4 hunks)
- storage/mocks/mocks.go (1 hunks)
- storage/pebble/accounts.go (4 hunks)
- storage/pebble/blocks.go (5 hunks)
- storage/pebble/receipts.go (3 hunks)
- storage/pebble/storage.go (4 hunks)
- storage/pebble/storage_test.go (6 hunks)
- storage/pebble/traces.go (2 hunks)
- storage/pebble/transactions.go (3 hunks)
Files skipped from review due to trivial changes (5)
- services/ingestion/mocks/EventSubscriber.go
- services/traces/mocks/Downloader.go
- storage/index_testsuite.go
- storage/mocks/TransactionIndexer.go
- storage/mocks/mocks.go
Additional comments not posted (57)
storage/pebble/transactions.go (2)
Line range hint
27-38: LGTM!The
Storefunction is well-implemented with proper error handling, mutex locking for thread safety, and batch operations.
Line range hint
40-52: LGTM!The
Getfunction is well-implemented with proper error handling and read-locking for thread safety.storage/pebble/traces.go (2)
28-35: LGTM!The
StoreTransactionfunction is well-implemented with proper error handling, mutex locking for thread safety, and batch operations.
Line range hint
37-49: LGTM!The
GetTransactionfunction is well-implemented with proper error handling and read-locking for thread safety.models/mocks/Engine.go (1)
Line range hint
72-78: LGTM!The
NewEnginefunction is well-implemented and includes a cleanup function to assert mock expectations.storage/mocks/TraceIndexer.go (2)
46-55: LGTM!The
StoreTransactionfunction is well-implemented and handles batch operations correctly.
Line range hint
65-71: LGTM!The
NewTraceIndexerfunction is well-implemented and includes a cleanup function to assert mock expectations.storage/mocks/AccountIndexer.go (3)
22-38: LGTM!The function
GetBalancehas been updated to accept acommon.Addressinstead of a pointer. This change is straightforward and does not introduce any issues.
48-62: LGTM!The function
GetNoncehas been updated to accept acommon.Addressinstead of a pointer. This change is straightforward and does not introduce any issues.
72-81: LGTM!The function
Updatehas been updated to include abatchparameter. This change is straightforward and does not introduce any issues.storage/pebble/accounts.go (3)
Line range hint
35-64: LGTM!The function
Updatehas been updated to include abatchparameter. This change ensures atomicity and does not introduce any issues.
87-95: LGTM!The function
GetNoncehas been updated to accept acommon.Addressinstead of a pointer. This change is straightforward and does not introduce any issues.
98-100: LGTM!The function
GetBalancehas been updated to accept acommon.Addressinstead of a pointer. This change is straightforward and does not introduce any issues.services/traces/engine.go (1)
115-115: LGTM!The function
indexBlockTraceshas been updated to include anilparameter in theStoreTransactionfunction call. This change is straightforward and does not introduce any issues.storage/mocks/ReceiptIndexer.go (1)
111-119: LGTM!The function
Storehas been updated to include abatchparameter. This change is straightforward and does not introduce any issues.storage/pebble/storage.go (6)
89-90: Ensure atomicity and caching logic in thesetmethod.The
setmethod now supports batch operations, but the caching logic is skipped when using a batch. This is correct as the batch may not be committed yet.
92-93: Cache the value after setting it in the database.The value is added to the cache after it is successfully set in the database. This ensures that subsequent reads can benefit from the cache.
104-104: Check the cache before accessing the database.The
getmethod checks the cache before accessing the database. This improves performance by reducing the number of database reads.
109-109: Handle cache misses by loading from the database.If the value is not found in the cache, it is loaded from the database. This ensures that the cache is used effectively while maintaining data consistency.
118-120: Add the value to the cache after loading from the database.The value is added to the cache after it is successfully loaded from the database. This ensures that subsequent reads can benefit from the cache.
131-131: Create a new batch for atomic operations.The
NewBatchmethod creates a new batch for atomic operations. This allows multiple operations to be grouped and committed together.storage/index.go (8)
18-21: Add batch parameter to theStoremethod.The
Storemethod now includes abatchparameter for atomic operations. This ensures that multiple operations can be grouped and committed together.
49-50: Add batch parameter to theSetLatestCadenceHeightmethod.The
SetLatestCadenceHeightmethod now includes abatchparameter for atomic operations. This ensures that multiple operations can be grouped and committed together.
68-71: Add batch parameter to theStoremethod.The
Storemethod now includes abatchparameter for atomic operations. This ensures that multiple operations can be grouped and committed together.
93-96: Add batch parameter to theStoremethod.The
Storemethod now includes abatchparameter for atomic operations. This ensures that multiple operations can be grouped and committed together.
106-107: Add batch parameter to theUpdatemethod.The
Updatemethod now includes abatchparameter for atomic operations. This ensures that multiple operations can be grouped and committed together.
111-111: Update theGetNoncemethod signature.The
GetNoncemethod now accepts acommon.Addressinstead of a pointer. This change simplifies the method signature and usage.
114-114: Update theGetBalancemethod signature.The
GetBalancemethod now accepts acommon.Addressinstead of a pointer. This change simplifies the method signature and usage.
119-121: Add batch parameter to theStoreTransactionmethod.The
StoreTransactionmethod now includes abatchparameter for atomic operations. This ensures that multiple operations can be grouped and committed together.storage/mocks/BlockIndexer.go (2)
195-201: Add batch parameter to theSetLatestCadenceHeightmethod.The mock implementation of the
SetLatestCadenceHeightmethod now includes abatchparameter. This change aligns with the updated interface and ensures that batch operations are correctly mocked.
209-215: Add batch parameter to theStoremethod.The mock implementation of the
Storemethod now includes abatchparameter. This change aligns with the updated interface and ensures that batch operations are correctly mocked.storage/pebble/storage_test.go (5)
37-39: Include nil batch parameter in theStoremethod calls.The
Storemethod calls now include anilbatch parameter. This change aligns with the updated method signature and ensures that the tests are correctly updated.
72-72: Include nil batch parameter in theStoremethod call.The
Storemethod call now includes anilbatch parameter. This change aligns with the updated method signature and ensures that the test is correctly updated.
86-86: Include nil batch parameter in theStoremethod call.The
Storemethod call now includes anilbatch parameter. This change aligns with the updated method signature and ensures that the test is correctly updated.
113-113: Include nil batch parameter in theStoremethod call.The
Storemethod call now includes anilbatch parameter. This change aligns with the updated method signature and ensures that the test is correctly updated.
137-165: Test batch operations for atomicity.The
TestBatchtest case verifies that batch operations are correctly implemented and committed. This ensures that multiple operations can be grouped and committed together, maintaining data consistency and atomicity.storage/pebble/blocks.go (2)
35-40: LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
Storematch the new signature.
179-183: LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
SetLatestCadenceHeightmatch the new signature.storage/pebble/receipts.go (1)
41-41: LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
Storematch the new signature.services/ingestion/engine.go (5)
234-259: LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
indexTransactionmatch the new signature.Verification successful
The function call to
indexTransactionmatches the new signature.The code changes are correct and consistent with the new function signature.
services/ingestion/engine.go: if err := e.indexTransaction(tx, receipts[i], batch); err != nil {Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `indexTransaction` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type go $'indexTransaction'Length of output: 193
Line range hint
194-227: LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
indexBlockmatch the new signature.Verification successful
Verified function usage in the codebase.
The changes to the
indexBlockfunction signature are correctly applied and isolated within theservices/ingestion/engine.gofile.
indexBlockis called with the new signature inservices/ingestion/engine.go.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `indexBlock` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type go $'indexBlock'Length of output: 439
Line range hint
94-138: LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
Runmatch the new signature.Verification successful
All function calls to
Runmatch the new signature.The code changes are verified and approved.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `Run` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type go $'Run'Length of output: 9007
Script:
#!/bin/bash # Extract and verify the context of `Run` calls in `services/ingestion/engine_test.go`. # Print the lines around the `Run` function calls to provide context. rg --type go --context 5 'Run\(context\.Background\(\)\)' services/ingestion/engine_test.goLength of output: 1175
Line range hint
37-51: LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
NewEventIngestionEnginematch the new signature.Verification successful
Function calls match the new signature.
All function calls to
NewEventIngestionEnginecorrectly include thestoreparameter as per the new signature.
services/ingestion/engine_test.gobootstrap/bootstrap.goScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `NewEventIngestionEngine` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type go $'NewEventIngestionEngine'Length of output: 526
Script:
#!/bin/bash # Description: Verify that all usages of `NewEventIngestionEngine` match the new signature. # Check the lines where `NewEventIngestionEngine` is called in the test file rg --type go --context 5 'NewEventIngestionEngine' services/ingestion/engine_test.go # Check the lines where `NewEventIngestionEngine` is called in the bootstrap file rg --type go --context 5 'NewEventIngestionEngine' bootstrap/bootstrap.goLength of output: 1601
Line range hint
140-190: LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
processEventsmatch the new signature.Verification successful
All function calls to
processEventsmatch the new signature. Verification complete.
services/ingestion/engine.go: The functionprocessEventsis only called and defined within this file.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `processEvents` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type go $'processEvents'Length of output: 304
Script:
#!/bin/bash # Description: Use ast-grep to search for all function calls to `processEvents` in the entire codebase. ast-grep --lang go --pattern $'$_ := $_.processEvents($_)'Length of output: 59
Script:
#!/bin/bash # Description: Use rg to search for all occurrences of `processEvents` in the entire codebase to ensure comprehensive coverage. rg 'processEvents'Length of output: 294
services/traces/engine_test.go (3)
176-177: LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
StoreTransactionmatch the new signature.Verification successful
All function calls to
StoreTransactionmatch the new signature.The updated signature of
StoreTransactionwith thebatchparameter is consistently used across the codebase.
storage/index.gostorage/index_testsuite.gostorage/pebble/traces.gostorage/pebble/storage_test.gostorage/mocks/TraceIndexer.goservices/traces/engine_test.goservices/traces/engine.goScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `StoreTransaction` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type go $'StoreTransaction'Length of output: 1297
77-78: LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
StoreTransactionmatch the new signature.Verification successful
Function signature updated consistently
The
StoreTransactionfunction has been updated to the new signature across the codebase. The following locations confirm the consistent usage:
storage/index.gostorage/index_testsuite.gostorage/pebble/traces.gostorage/pebble/storage_test.goservices/traces/engine_test.goservices/traces/engine.goNo instances of the old signature were found.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `StoreTransaction` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type go $'StoreTransaction'Length of output: 1297
176-177: LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
StoreTransactionmatch the new signature.bootstrap/bootstrap.go (4)
33-33: Initializepebblestorage correctly.The initialization of the
storevariable looks good. Ensure thatcfg.DatabaseDirandloggerare correctly configured.
38-42: Update function calls to usestore.The function calls are correctly updated to use the new
storevariable.
51-51: Includenilargument inSetLatestCadenceHeight.The additional
nilargument is correctly added to theSetLatestCadenceHeightfunction call.
122-122: Passstoreparameter tostartIngestion.The
storeparameter is correctly passed to thestartIngestionfunction.Also applies to: 144-144
services/ingestion/engine_test.go (5)
9-10: ImportpebbleDBpackage.The import of the
pebbleDBpackage is necessary for the batch operations in the tests.
12-12: Importpebblepackage.The import of the
pebblepackage is necessary for the storage operations in the tests.
38-40: Initializestorevariable in tests.The
storevariable is correctly initialized in the test function.
65-65: Passstoreparameter in test functions.The
storeparameter is correctly passed to theNewEventIngestionEnginefunction in various test functions.Also applies to: 145-145, 218-219, 255-255, 354-354, 449-449
91-92: Includebatchparameter in mock functions.The
batchparameter is correctly included in each mock function.Also applies to: 171-172, 229-230, 237-238, 274-275, 284-285, 292-293, 336-337, 374-375, 382-383, 390-391, 479-480, 500-501, 510-511
api/api.go (1)
604-604: UpdateGetNoncemethod to acceptaddressparameter.The
GetNoncemethod is correctly updated to accept anaddressparameter instead of a pointer.
Co-authored-by: Ardit Marku <markoupetr@gmail.com>
Closes: #116
Description
Make sure we make the index updates atomic across different storage implementations.
For contributor use:
masterbranchFiles changedin the Github PR explorerSummary by CodeRabbit
Refactor
batchparameter for improved batch processing.New Features
Bug Fixes
Tests