RFC-0001: Rfc 0001 impl #1069

Eric-Warehime · 2022-06-28T16:15:55Z

Summary

Minimal commit which just adds the interface from #1061

codecov · 2022-06-28T16:34:53Z

Codecov Report

Merging #1069 (b601d9a) into develop (6cb6f97) will increase coverage by 0.11%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           develop    #1069      +/-   ##
===========================================
+ Coverage    59.58%   59.70%   +0.11%     
===========================================
  Files           45       48       +3     
  Lines         8353     8390      +37     
===========================================
+ Hits          4977     5009      +32     
- Misses        2918     2922       +4     
- Partials       458      459       +1

Impacted Files	Coverage Δ
exporters/exporter.go	`100.00% <100.00%> (ø)`
exporters/exporter_factory.go	`100.00% <100.00%> (ø)`
exporters/noop/noop_exporter.go	`100.00% <100.00%> (ø)`
fetcher/fetcher.go	`33.77% <0.00%> (-2.20%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6cb6f97...b601d9a. Read the comment docs.

winder · 2022-06-29T12:46:35Z

exporters/exporter.go

+
+	// Receive is called for each block to be processed by the exporter.
+	// Should return an error on failure--retries are configurable.
+	Receive(blockData *rpcs.EncodedBlockCert) error


This isn't quite right. Also, what do you think about wrapping it up in another object so that it's easier to add stuff without changing the function signature?

I'm also suggesting pass by value here, with the individual bits of data as pointers instead. We can make copies of the data object and allow plugins set each piece of data to nil if they want to (i.e. we are currently dropping the Certificate).

// ExportData is provided to the Exporter on each round. type ExportData { // Block is the block data written to the blockchain. Block *bookkeeping.Block // Delta contains a list of account changes resulting from the block. Processor plugins may have modify this data. Delta *ledgercore.StateDelta // Certificate contains voting data that certifies the block. The certificate is non deterministic, a node stops collecting votes once the voting threshold is reached. Certificate *agreement.Certificate }

Suggested change

Receive(blockData *rpcs.EncodedBlockCert) error

Receive(data ExportData) error

winder · 2022-06-29T12:47:24Z

exporters/exporter.go

+import "github.com/algorand/go-algorand/rpcs"
+
+// ExporterConfig is a generic type providing serialization/deserialization for exporter config files
+type ExporterConfig interface{}


I could be convinced otherwise, but I think this is a string.

Suggested change

type ExporterConfig interface{}

type ExporterConfig string

Ideally we would have some serde module fetching configs from disk. The objects fetched from disk may be strings, but then we're going to want to marshal them into an equivalent of api.ExtraOptions, or idb.IndexerDbOptions.

Once we've constructed those objects, we'll need to merge in/validate/override properties that were set via the command line. Then whatever is left should get passed into the Exporter.

Let me know what your thoughts are on the above process for resolving cfg values...I haven't done any serious work on the config parsing yet so maybe it is a string, but it seemed like it wouldn't be to me given the above framework.

winder

Looks good!

winder · 2022-07-06T21:31:22Z

exporters/exporter.go

+
+	// Receive is called for each block to be processed by the exporter.
+	// Should return an error on failure--retries are configurable.
+	Receive(exportData ExportData) error


The main part I'm not sure about is whether we should have this abstraction in the first iteration. It means there will be a required type check in all plugins to convert it into BlockExportData.

I think it's ok to leave it like this for now, but lets keep an eye on it.

winder · 2022-07-06T21:33:12Z

exporters/exporter.go

+	// Metadata associated with each Exporter.
+	Metadata() ExporterMetadata


I think you can do something like this, then we can use the same interface for the other plugins

type PluginMetadata interface { // Metadata associated with each Exporter. Metadata() ExporterMetadata }

Suggested change

// Metadata associated with each Exporter.

Metadata() ExporterMetadata

PluginMetadata

Yeah I agree, it seems correct to use the same Metadata construct for all of the plugin types. I'll update this in a subsequent PR to make it more generic. It should probably end up being moved to a separate module in that case as well.

Adds an Exporter interface and a noop exporter implementation with factory methods for construction

* Local Ledger (#1011) * integrate block processor * Local Ledger Deployment (#1013) * add simple local ledger migration * add deleted opts * fast catchup (#1023) * add fast catchup * Localledger merge (#1036) * 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]> * recovery scenario (#1024) * handle ledger recovery scenario * refactor create genesis block (#1026) * refactor create genesis block * Adds Local Ledger Readme (#1035) * 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]> * update ledger file path and migration (#1042) * LocalLedger Refactoring + Catchpoint Service (#1049) 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 * build: merge develop into localledger/integration (#1062) * Ledger init status (#1058) * Generate an error if the catchpoint is not valid for initialization. (#1075) * Use main logger in handler and fetcher. (#1077) * Switch from fullNode catchup to catchpoint catchup service. (#1076) * Refactor daemon, add more tests (#1039) Refactors daemon cmd into separate, testable pieces. * Merge develop into localledger/integration (#1083) * Misc Local Ledger cleanup (#1086) * Update processor/blockprocessor/initialize.go Co-authored-by: Zeph Grunschlag <[email protected]> * commit * fix function call args * RFC-0001: Rfc 0001 impl (#1069) Adds an Exporter interface and a noop exporter implementation with factory methods for construction * Fix test errors * Add/fix tests * Add postgresql_exporter tests * Update config loading * Change BlockExportData to pointers * Move and rename ExportData * Add Empty func to BlockData * Add comment Co-authored-by: shiqizng <[email protected]> Co-authored-by: [email protected] <[email protected]> Co-authored-by: erer1243 <[email protected]> Co-authored-by: AlgoStephenAkiki <[email protected]> Co-authored-by: Will Winder <[email protected]> Co-authored-by: Zeph Grunschlag <[email protected]>

Eric-Warehime added 4 commits June 15, 2022 14:12

Lock to mockery minor version

5bca3f4

Merge branch 'algorand:develop' into develop

1baa216

Add Exporter interface

aede594

Rename Shutdown method

b758ca7

Eric-Warehime requested review from winder, shiqizng and AlgoStephenAkiki June 28, 2022 16:15

winder reviewed Jun 29, 2022

View reviewed changes

update exporter interface

5210f78

Eric-Warehime changed the title ~~Rfc 0001 impl~~ RFC-0001: Rfc 0001 impl Jul 5, 2022

Eric-Warehime added the Enhancement New feature or request label Jul 5, 2022

Eric-Warehime added 4 commits July 5, 2022 15:24

add comments

a65a7b3

add noop exporter

437e691

update exporters module

95b0193

Add exporter_factory tests

b601d9a

Eric-Warehime requested a review from winder July 6, 2022 20:50

winder approved these changes Jul 6, 2022

View reviewed changes

Eric-Warehime merged commit d939d70 into algorand:develop Jul 7, 2022

Eric-Warehime added a commit to Eric-Warehime/indexer that referenced this pull request Jul 7, 2022

RFC-0001: Rfc 0001 impl (algorand#1069)

e923b9c

Adds an Exporter interface and a noop exporter implementation with factory methods for construction

Eric-Warehime added a commit to Eric-Warehime/indexer that referenced this pull request Jul 13, 2022

RFC-0001: Rfc 0001 impl (algorand#1069)

e4f6945

Adds an Exporter interface and a noop exporter implementation with factory methods for construction

algobarb mentioned this pull request Jul 25, 2022

FOR REVIEW ONLY: indexer 2.13.0 into master #1137

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RFC-0001: Rfc 0001 impl #1069

RFC-0001: Rfc 0001 impl #1069

Eric-Warehime commented Jun 28, 2022

codecov bot commented Jun 28, 2022 •

edited

Loading

winder Jun 29, 2022

winder Jun 29, 2022

Eric-Warehime Jul 5, 2022

winder left a comment

winder Jul 6, 2022

winder Jul 6, 2022

Eric-Warehime Jul 7, 2022

	Receive(blockData *rpcs.EncodedBlockCert) error
	Receive(data ExportData) error

		// Metadata associated with each Exporter.
		Metadata() ExporterMetadata

	// Metadata associated with each Exporter.
	Metadata() ExporterMetadata
	PluginMetadata

RFC-0001: Rfc 0001 impl #1069

RFC-0001: Rfc 0001 impl #1069

Conversation

Eric-Warehime commented Jun 28, 2022

Summary

codecov bot commented Jun 28, 2022 • edited Loading

Codecov Report

winder Jun 29, 2022

Choose a reason for hiding this comment

winder Jun 29, 2022

Choose a reason for hiding this comment

Eric-Warehime Jul 5, 2022

Choose a reason for hiding this comment

winder left a comment

Choose a reason for hiding this comment

winder Jul 6, 2022

Choose a reason for hiding this comment

winder Jul 6, 2022

Choose a reason for hiding this comment

Eric-Warehime Jul 7, 2022

Choose a reason for hiding this comment

codecov bot commented Jun 28, 2022 •

edited

Loading