New Feature: Indexer Application Box Ingestion #1067
Conversation
* integrate block processor
* add simple local ledger migration
* add fast catchup
* return empty lists from fetchApplications and fetchAppLocalStates (#1010) * Update model to converge with algod (#1005) * New Feature: Adds Data Directory Support (#1012) - Updates the go-algorand submodule hash to point to rel/beta - Moves the cpu profiling file, pid file and indexer configuration file to be options of only the daemon sub-command - Changes os.Exit() to be a panic with a special handler. This is so that defer's are handled instead of being ignored. - Detects auto-loading configuration files in the data directory and issues errors if equivalent command line arguments are supplied. - Updates the README with instructions on how to use the auto-loading configuration files and the data directory. * Update mockery version Co-authored-by: erer1243 <[email protected]> Co-authored-by: AlgoStephenAkiki <[email protected]>
* handle ledger recovery scenario
* refactor create genesis block
* Adds Local Ledger Readme Resolves #4109 Starts Readme docs * Update docs/LocalLedger.md Co-authored-by: Will Winder <[email protected]> * Update docs/LocalLedger.md Co-authored-by: Will Winder <[email protected]> * Update docs/LocalLedger.md Co-authored-by: Will Winder <[email protected]> * Removed troubleshooting section Co-authored-by: Will Winder <[email protected]>
Part 1
cleanup genesis file access.
put node catchup into a function that can be swapped out with the catchup service.
pass the indexer logger into the block processor.
move open ledger into a util function, and move the initial state util function into a new ledger util file.
add initial catchupservice implementation.
move ledger init from daemon.go to constructor.
Merge multiple read genesis functions.
Part 2
Merge local_ledger migration package into blockprocessor.
Rename Migration to Initialize
Use logger in catchup service catchup
Part 3
Update submodule and use NewWrappedLogger.
Make util.CreateInitState private
…lgorand commit ...
| @@ -7,6 +7,7 @@ services: | |||
| dockerfile: ./misc/Dockerfile | |||
| environment: | |||
| CONNECTION_STRING: "host=e2e-db port=5432 user=algorand password=algorand dbname=indexer_db sslmode=disable" | |||
| CI_E2E_FILENAME: "tzaffi-box-e2e-improvements" | |||
There was a problem hiding this comment.
^^^^ TODO: revert CI_E2E_FILENAME to "rel-nightly" before merging to develop ^^^^
| -- For looking up app box storage | ||
| CREATE TABLE IF NOT EXISTS app_box ( | ||
| app bigint NOT NULL, | ||
| name bytea NOT NULL, |
There was a problem hiding this comment.
Are these names expected to be human readable?
There was a problem hiding this comment.
No. They can be any bytestring whatsoever. We're not sure what the usage pattern will be, but I personally expect many of the names to be human readable, if not the values.
| return delta, kvUpdated, boxTotals | ||
| } | ||
|
|
||
| // CompareAppBoxesAgainstLedger uses LedgerForEvaluator to assert that provided app boxes can be retrieved as expected |
There was a problem hiding this comment.
used in ledger_for_evaluator_test.go
There was a problem hiding this comment.
In #1117 it's moved into that file
| } | ||
|
|
||
| // BuildAccountDeltasFromKvsAndMods simulates keeping track of the evolution of the box statistics | ||
| func BuildAccountDeltasFromKvsAndMods(t *testing.T, kvOriginals, kvMods map[string]*string) ( |
There was a problem hiding this comment.
I slightly prefer putting this in writer_test.go, or a utils_test.go file in the writer package. It's possible for files that don't end in _test.go to slip into a production binary, so unless they're really used across packages it is best to avoid using real packages.
There was a problem hiding this comment.
I appreciate your valiant attempt to fight the machine. I'll investigate refactoring along the lines suggested. The most frustration for me during this PR has been working around go's import restrictions for testing.
There was a problem hiding this comment.
BTW. There is a significant refactor of testing code in the upcoming followup #1117. It might be worthwhile to punt the discussion of where tests should live exactly till that PR.
There was a problem hiding this comment.
In particular, as part of the refactor this file has been deleted:
There was a problem hiding this comment.
@winder I've analyzed my refactoring for upcoming #1117. That one is not yet ready for review. However, one of the refactorings was doing exactly what you suggested. Moving BuildAccountDeltasFromKvsAndMods to writer_test.go. Thus I ask that we punt the final refactoring discussion to that PR.
Co-authored-by: Will Winder <[email protected]>
Co-authored-by: Will Winder <[email protected]>
Co-authored-by: Will Winder <[email protected]>
Adding Box Information to Indexer
Summary
The proposed
app_boxpostgres table with all the insertion/update/deletion logic in place.Additional test recommendations welcome!
Related PRs
Test Plan
TODO (move this to the integration branch once merged)
idb/postgres/postgres_rand_test.goapp_boxis populated as expectedTotalBoxesandTotalBoxBytesintoaccount.account_dataJSONBrel-nightlyonce that job includes boxes (cf.misc/docker-compose.yml)third_party/go-algorandsubmodule togo-algorandAn Aside about Minimum Balance Calculation and App-Boxes
CF: The previous (partially wrong) version of this aside.
It would be nice to solve the long-standing issue #808 and provide minimum balance information for accounts, including app-accounts. App-Boxes affect the min balance calculation for an app-account holding boxes, and as this PR introduces their basic information to indexer.
Here's a brief description of how to add min-balance for apps in indexer:
The New Minimum Balance Calculation
The new boxes-included min-balance calculation requires knowing:
totalBoxes)totalBoxBytes) which is comprised of bytes used for keys and valuesThe current PR allows calculating this information for a particular app.
Here's some pseudo-SQL: