Indexer Refactor feature branch

[frrist]

See me run here: https://protocollabs.grafana.net/goto/dfhCT9Pnk?orgId=1

Related Issues

• Re-architect Lily for horizontal: diff extraction, diff storage, diff processing. #867, while not fully addressed, this PR modifies lily so it can address this issue with follow on work
• Lily's watch should tolerate re-orgs #864, This PR together with completion of Re-architect Lily for horizontal: diff extraction, diff storage, diff processing. #867 will fully address this. The key piece of work done here was making the indexer stateless. State-full indexing caused errors when reorgs occur, now that indexing is stateless reorgs can be processed without error. A single instance of lily is not performant enough to process all tasks during a reog, but if ran with a lighter task load, this implementation of lily addresses this.
• Split Lily tasks further to allow more paralelism #861 the work done in Full indexer refactor #871 closes this.

Description

This PR is a feature branch as was compiled from the below PR:

#849

This PR implements the taskAPI which is passed to and used by the indexer and its tasks.

• The taskAPI presents a smaller interface for easier testing of tasks, it's simpler to mock than than the entire LilyAPI.
• The StateChangedActors method has been removed from the indexer and now lives in the taskAPI. I think this is a lot cleaner.
• The GetExecutedAndBlockMessagesForTipset and GetMessageExecutionsForTipSet method have been memoized, this allows them to be called within the Processors instead of called in the indexer with results passed to the processors. The goal here is to allow Processors to call these methods separatly to increase readability and extensabiliy of the code without sacrificing performance.

#850

This PR aims to keep indexer behavior consistent with the current while landing some refactor work to improve code extensibility.

• embed GetExecutedAndBlockMessagesForTipset in processors, preventing unnecessary (and expensive!) calls to the method when its output isn't used. And example of this would be running all actor tasks excluding the miner actor. Prior changes here ensure multiple calls to this method are only executed once, returning a cached value for subsequent calls. This change also makes the indexer's logic more concise.
• embed GetMessageExecutions in processors, taking advantage of the memoized method pattern used in previous commit. This has the added bonus of removing the ProcessMessageExecution interface and fitting the messege execution task under the ProcessMessages interface.

#871

This PR performs a (large) refactor of the indexer used by lily to index and process that chain. I have done my best to make each commit reviewable on its own and encourage reviewers to go through them sequentially. Here are some high-level details of what's changed:

Miner diffing

The DiffSectors and DiffPreCommits methods moved to the DataSource API. The API memoizes calls to these methods, allowing separate miner extractors to call them as needed.

Actor task breakup

All actor extractors have are separated such that each extractor produces a single model. For example, there is no longer a single miner state extractor rather there are several: DeadlineInfoExtractor, InfoExtractor, FeeDebtExtractor, LockedFundsExtractor, PoStExtractor, PreCommitInfoExtractor, SectorInfoExtractor, V7SectorInfoExtractor, etc. Each extract a single model from a miner state, utilizing the memoized methods (mentioned above) when needed. Similarly true for the market, init, power, reward, verifiedreg, etc. actors.

Message task breakup

The Messages processors are separate in a similar way - there exists a processor for GasOutputs, DerivedGasOutputs, Messages, ParsedMessages, InternalMessages, InternalParsedMessages, Receipts, BlockMessages, etc. These processors, too, use the memoized methods available on the DataSource API when appropriate.

The Indexer package

Here is where the new chain indexing logic lives. Its made up of 3 parts (all of which have code comments that should be referred to for more detail):

• StateProcessor: accepts a set of processor interfaces and executes them all in parallel, returning their execution results via a channel.
• TipSetIndexer: accepts a list of tasks, instantiates the appropriate processors, and passes them to the StateProcessor. The TipSetIndexer consumes results from the StateProcessor and produces results containing task names, extracted models, and reports, returning them on a channel. If the TipSetIndexer is instructed to halt before receiving all expected results from the StateProcessors it pushes the appropriate SKIP reports into its output channel as well.
• IndexManager: consumes results from the TipSetIndexer and persists them to storage. The IndexManager will signal if any reports it receives from the TipSetIndexer have the status SKIP or ERROR. The IndexManager may also be given a worker pool to run multiple TipSetIndexer in parallel.

Simiplifcation of Processors

There are now only 3 processors used to extract state from the chain:

• TipSetProcessor: processes a singel tipset
• TipSetsProcessor: processes a current and an executed tipset
• ActorProcessor: processes a current and executed tipset and actors that changed during the executed tipset appliction.
  Said processors are consumed and executed by the StateProcessor

ReportProcessor

This processor only returns a visormodel.ProcessingReport and writes null rounds to the visormodel.ProcessingReport table. It replaces logic that once lived in the consensus task.

Models as tasks

Lily now accepts a list of model names to extract rather than a list of tasks corresponding to sets of models. For example, the task actorstatesminer has been replaced with: miner_current_deadline_infos, miner_fee_debts, miner_infos, miner_locked_funds,miner_pre_commit_infos,miner_sector_deals, miner_sector_events, miner_sector_infos, miner_sector_posts
similar for all other actorstates* tasks. This gives more flexibility to the user for extracting data. And, together with the "Actor task breakup" change, allows for certaine extractors to be configured to run over specific actor versions (greatly simplifying the extraction of the MinerSectorInfos after the v7 actor upgrade)

TODO

• Implement --task-exclude flag mentioned here: Full indexer refactor #871 (comment)
• Address [Indexer Refactor] miner_pre_commit_infos, miner_sector_infos, miner_sector_deals, and miner_sector_events task failures #897
• document new APIs (index method) and task arguments

[codecov-commenter]

Codecov Report

❌ Patch coverage is 19.35484% with 150 lines in your changes missing coverage. Please review.
✅ Project coverage is 37.0%. Comparing base (92a3010) to head (60bf867).
⚠️ Report is 260 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff            @@
##           master    #869     +/-   ##
========================================
+ Coverage    29.5%   37.0%   +7.4%     
========================================
  Files          39      25     -14     
  Lines        3726    1802   -1924     
========================================
- Hits         1102     667    -435     
+ Misses       2481    1060   -1421     
+ Partials      143      75     -68     

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[placer14]

Some thoughts about task names. I don't mind a full set of task options but I would consider creating a set of helper aliases. As Ian mentioned, all could be good for explicitness (I would expect default of --tasks to be all anyway). I would also include alias for the all of the prior versions tasks mapped onto their appropriate tasks. Ex: actorstatesminer would enable all models which were processed under that task previously. We can remove them later in a major version release.

Ninjaedit: And, yes please... a tasks filter would be good.

[placer14]

Sticky approval. (Sorry for slow review turnaround on this.) Congrats on getting this shipped! 💪 🎉

[placer14]

This PR is merged now. Does this need to change?

[frrist]

Not yet, it will change when we upgrade to the lotus release with the change, so 1.15.2 or something. (Currently, this exits in lotus master, but we don't use master as our dep)

[placer14]

I see both Apply and Revert drop the event if the buffer is full (different behavior from before the change, too). How will this affect the work needed on those tipsets? If so, what is the intended recovery mechanism?

[placer14]

If it's helpful, consider adding this information to the newly-exposed arg.

[frrist]

We can either drop the event or block the observer module: we don't want to block by waiting to send on the event channel since it will prevent other registered observers from receiving apply and revert events (method hangs), which is all-around bad.

Blocking should never happen during normal operations since the watcher will queue work it can't currently perform: https://github.com/filecoin-project/lily/blob/indexer-refactor/chain/watcher.go#L176. But if a watch job blows up/fails/gets restarted, its observer will still be registered and receiving events https://github.com/filecoin-project/lily/blob/indexer-refactor/chain/watcher.go#L69. And may continue to receive events even after its unregistered: https://github.com/filecoin-project/lotus/blob/20bf46f3098b3c433231480dda98cbf66940a201/chain/events/observer.go#L259. So this change prevents "zombie observers" from blocking on Apply/Revert until they can be unregistered.

[placer14]

If I'm hearing correctly that this will prevent that work from being completed, it would probably be helpful to indicate a brief warning in that error that that param could be adjusted. And a good idea to include in the help text as well.