Refactor indexer version config #332

keyliaran · 2024-03-25T11:31:18Z

Resolve issue #219.
Added starting_version_if_nothing_in_db to the worker config. implementation works as follows:

If starting_version exists in the config, it will override all existing values (both starting_version_if_nothing_in_db and the database value).
If starting_version is not configured, we'll take a look to the database value. If the database has no value and starting_version_if_nothing_in_db is configured, we'll use that value; otherwise, we'll default to 0.

Test plan:

Drop database, so there is no starting version in it, create config with starting_version_if_nothing_in_db, check that indexer will start from this version
Restart indexer, indexer should start from the db version
Add starting_version to config, indexer should start from this version

larry-aptos · 2024-03-25T19:33:57Z

nice work!

one thing to point out is we're trying to avoid breaking changes introduced to our indexer stack.

community needs to change their processor config, otherwise crash due todeny_unknown_fields

is it possible to add a new optional field like overriding_starting_version(default to true) and if it's present and true or not present, we'll keep the existing behavior and if false, do the new logic.

keyliaran · 2024-03-25T19:59:24Z

@larry-aptos I've implemented support for backward compatibility in the configuration. Please take a look.

larry-aptos · 2024-03-25T20:04:55Z

rust/processor/README.md

@@ -25,8 +25,10 @@ Indexer GRPC parser is to indexer data processor that leverages the indexer grpc
        indexer_grpc_http2_ping_timeout_in_secs: 10
        number_concurrent_processing_tasks: 10
        auth_token: AUTH_TOKEN
-        starting_version: 0 # optional


nit: update the document

sorry for being late on this.

a question about this: if starting_version is specific, does it also mean that processing start from this version regardless of db version?

I've added a new field called "override_starting_version." The logic is straightforward:

If "override_starting_version" exists and "starting_version" is configured, use it regardless of the database version.

If "override_starting_version" doesn't exist, use the database version.

rtso

@keyliaran the current implementation is not backwards compatible with how the ecosystem uses starting_version. The current assumption is starting_version will override whatever is it DB. We should keep that assumption so we don't break ecosystem indexers. I propose to add a starting_version_if_nothing_in_db instead of the override.

keyliaran · 2024-04-17T22:08:58Z

@keyliaran the current implementation is not backwards compatible with how the ecosystem uses starting_version. The current assumption is starting_version will override whatever is it DB. We should keep that assumption so we don't break ecosystem indexers. I propose to add a starting_version_if_nothing_in_db instead of the override.

If the indexer is launched with starting_version set in the configuration, it will always resume from that version after each restart due to node crashes or other issues like panics. This behavior isn't ideal because, i guess, nobody wants to keep reindexing the entire blockchain from starting_version setted in config. To address this, I would need to start the indexer from the specified starting_version and then modify the configuration.

To improve this, you're suggesting adding a new field called starting_version_if_nothing_in_db. Here's how it would work: starting_version will override both the database value and the config value of starting_version_if_nothing_in_db. This means that if the database doesn't have a starting version stored, it will use starting_version from the config. So there are no usefull usecases of new field starting_version_if_nothing_in_db, it will be used only if there is nothing in database

keyliaran · 2024-04-17T22:17:27Z

If override_starting_version has a default value of true, the current implementation will not be backwards compatible.

keyliaran · 2024-04-17T22:28:55Z

@rtso Pls take a look to the updated implementation. It shoudln't break backward compatible.

rtso · 2024-04-17T22:51:56Z

@keyliaran I think the issue is originally suggesting this approach:

If there's no latest version set in the db, it will use starting_version_if_nothing_in_db if set
Otherwise it will use starting_version if set
Otherwise, it will use the latest version in db

This makes it easier to develop the indexer. When you wipe the db, the config remembers the version the contract was deployed (starting_version_if_nothing_in_db ) and start indexing from that version. If you need to start indexing from specific version, you can set an arbitrary version in starting_version. You won't have to set/unset the flag and starting_version every time.

rtso · 2024-04-17T23:08:54Z

starting_version_if_nothing_in_db, it will be used only if there is nothing in database

Yep you're absolutely correct! And if you want it to start from latest version in processor_status table, leave starting_version blank so that it starts from latest version in db.

…rsion

keyliaran · 2024-04-17T23:49:17Z

@rtso Made an update, pls take a look

rtso · 2024-04-18T00:50:27Z

rust/processor/src/worker.rs

-                0
-            });
-
-        let starting_version = self.starting_version.unwrap_or(starting_version_from_db);


Thanks for making the change! This line is still necessary to override with starting_version

Sure! Fixed

rtso · 2024-04-18T00:51:01Z

Could you also share your test plan in the description? TY

grao1991 · 2024-04-18T01:08:28Z

rust/processor/src/config.rs

@@ -27,6 +27,8 @@ pub struct IndexerGrpcProcessorConfig {
    pub auth_token: String,
    // Version to start indexing from
    pub starting_version: Option<u64>,
+    // Version to start indexing from if nothing in db
+    pub starting_version_if_nothing_in_db: Option<u64>,


we probably want a better name..

also, for these two parameters, it's not clear which one will take precedence. we probably should either use an enum here, or ensure they are mutually exclusive.

I believe we should avoid using enums to maintain backward compatibility. The current implementation works as follows:

If starting_version exists in the config, it will override all existing values (both starting_version_if_nothing_in_db and the database value).

If starting_version is not configured, we'll take a look to the database value. If the database has no value and starting_version_if_nothing_in_db is configured, we'll use that value; otherwise, we'll default to 0.

larry-aptos requested review from larry-aptos, banool, CapCap, bowenyang007 and just-in-chang and removed request for banool March 25, 2024 19:28

Add override_starting_version cfg option

7de84f4

keyliaran force-pushed the indexer_version_cfg_refactor branch from 2ad40b6 to 7de84f4 Compare March 25, 2024 19:57

larry-aptos reviewed Mar 25, 2024

View reviewed changes

typo fix

c7931c5

larry-aptos approved these changes Mar 25, 2024

View reviewed changes

rtso reviewed Apr 17, 2024

View reviewed changes

keyliaran added 2 commits April 18, 2024 01:20

override_starting_version defaults to true

9fedd3e

fix autoindent

f3d2e7f

Add starting_version_if_nothing_in_db instead of override_starting_ve…

41e489b

…rsion

rtso reviewed Apr 18, 2024

View reviewed changes

grao1991 reviewed Apr 18, 2024

View reviewed changes

Force using of start version if exists

831fa71

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor indexer version config #332

Refactor indexer version config #332

keyliaran commented Mar 25, 2024 •

edited

Loading

larry-aptos commented Mar 25, 2024

keyliaran commented Mar 25, 2024

larry-aptos Mar 25, 2024

keyliaran Mar 25, 2024

larry-aptos Apr 17, 2024

keyliaran Apr 17, 2024

rtso left a comment

keyliaran commented Apr 17, 2024

keyliaran commented Apr 17, 2024

keyliaran commented Apr 17, 2024

rtso commented Apr 17, 2024 •

edited

Loading

rtso commented Apr 17, 2024 •

edited

Loading

keyliaran commented Apr 17, 2024

rtso Apr 18, 2024

keyliaran Apr 18, 2024

rtso commented Apr 18, 2024

grao1991 Apr 18, 2024

keyliaran Apr 18, 2024

Refactor indexer version config #332

Are you sure you want to change the base?

Refactor indexer version config #332

Conversation

keyliaran commented Mar 25, 2024 • edited Loading

larry-aptos commented Mar 25, 2024

keyliaran commented Mar 25, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rtso left a comment

Choose a reason for hiding this comment

keyliaran commented Apr 17, 2024

keyliaran commented Apr 17, 2024

keyliaran commented Apr 17, 2024

rtso commented Apr 17, 2024 • edited Loading

rtso commented Apr 17, 2024 • edited Loading

keyliaran commented Apr 17, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rtso commented Apr 18, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

keyliaran commented Mar 25, 2024 •

edited

Loading

rtso commented Apr 17, 2024 •

edited

Loading

rtso commented Apr 17, 2024 •

edited

Loading