[Sharded DB][1] migrate Event and Transaction Schemas #13240

areshand · 2024-05-09T23:43:46Z

Description

Type of Change

Which Components or Systems Does This Change Impact?

How Has This Been Tested?

added new tests
manual test with config https://gist.github.com/areshand/ec91d7b7d8acae9138de999f74773ab0
both db tailer and table info runs as expected

2024-06-13T21:38:32.118930Z [index-db-0] INFO ecosystem/indexer-grpc/indexer-grpc-utils/src/counters.rs {"duration_in_secs":"0.000201417","end_version":"973831","message":"[Indexer DB Tailer] Tailed AptosDB and write to indexer DB","num_transactions":"18","service_type":"db_tailer_service","start_version":"973813","step":"1"}
2024-06-13T21:38:32.119613Z [sync-driver-10] INFO state-sync/state-sync-driver/src/storage_synchronizer.rs:690 {"message":"Committed a new transaction chunk! Transaction total: 18, event total: 0","name":"storage_synchronizer"}
2024-06-13T21:38:32.126573Z [table-info-9] INFO ecosystem/indexer-grpc/indexer-grpc-table-info/src/table_info_service.rs:217 [Table Info] Preparing to fetch transactions {"current_version":973813,"highest_known_version":973830,"parser_batch_size":100,"parser_task_count":10}
2024-06-13T21:38:32.130307Z [table-info-9] INFO ecosystem/indexer-grpc/indexer-grpc-table-info/src/table_info_service.rs:287 [Table Info] Table info parsed successfully {"num_transactions":18,"table_info_first_version":973813,"table_info_parsing_millis_per_batch":2}

^C

Key Areas to Review

Checklist

I have read and followed the CONTRIBUTING doc
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I identified and added all stakeholders and component owners affected by this change as reviewers
I tested both happy and unhappy path of the functionality
I have made corresponding changes to the documentation

trunk-io · 2024-05-09T23:43:49Z

⏱️ 4h 4m total CI duration on this PR

Job	Cumulative Duration	Recent Runs
rust-targeted-unit-tests	1h 15m	🟥 🟩 🟩 🟥
rust-move-tests	1h 5m	🟩 🟩 🟩 🟩 🟩
execution-performance / single-node-performance	49m	🟥 🟥
rust-lints	25m	🟥 🟥 🟥 🟥 🟥
run-tests-main-branch	17m	🟩 🟩 🟩 🟩
general-lints	5m	🟩 🟩 🟩
check-dynamic-deps	4m	🟩 🟩 🟩
semgrep/ci	1m	🟩 🟩 🟩
file_change_determinator	37s	🟩 🟩 🟩
file_change_determinator	34s	🟩 🟩 🟩
permission-check	9s	🟩 🟩 🟩
permission-check	9s	🟩 🟩 🟩
permission-check	8s	🟩 🟩 🟩
permission-check	6s	🟩 🟩 🟩

_{settings ⋅ feedback ⋅ docs ⋅ learn more about trunk.io}

grao1991 · 2024-05-29T00:19:49Z

ecosystem/indexer-grpc/indexer-grpc-table-info/src/tailer_service.rs

@@ -0,0 +1,69 @@
+// Copyright © Aptos Foundation


would it be better to put this file under storage?

more consistent with the existing structure. when we want to refactor in future, it is easier to find them in one place.

Reading the code, it seems weird to me that the logic was split between this crate and storage/indexer. probably a migration was in mind. That said, I guess adding more functionality in either crate is probably okay.

@jillxuu any comments? (And please review this PR as well?)

storage/aptosdb/src/db/include/aptosdb_reader.rs

storage/indexer/src/db_ops.rs

grao1991 · 2024-05-29T00:34:35Z

storage/indexer/src/db_tailer.rs

+            .expect("Cannot create db tailer iterator");
+        let batch = SchemaBatch::new();
+        let metadata_batch = SchemaBatch::new();
+        db_iter.for_each(|res| {


nit: consider using rayon here.

don't know how to do this? this db_iter is sequential itself.

storage/indexer/src/db_tailer.rs

ecosystem/indexer-grpc/indexer-grpc-table-info/src/table_info_service.rs

msmouse · 2024-06-03T20:50:32Z

ecosystem/indexer-grpc/indexer-grpc-table-info/src/tailer_service.rs

@@ -0,0 +1,69 @@
+// Copyright © Aptos Foundation


Reading the code, it seems weird to me that the logic was split between this crate and storage/indexer. probably a migration was in mind. That said, I guess adding more functionality in either crate is probably okay.

@jillxuu any comments? (And please review this PR as well?)

msmouse · 2024-06-03T20:53:36Z

config/src/config/index_db_tailer_config.rs

+impl Default for IndexDBTailerConfig {
+    fn default() -> Self {
+        Self {
+            enable: false,


For rollout we probably should default to enabled?

we don't want to turn on tailer by default. For rollout, I assume we have some indexer nodes in mind that we can update their node config?

Can we have one node flag for each index?

added a separate flag anyway. previously thought we read them in batch and no need to separate.

ecosystem/indexer-grpc/indexer-grpc-table-info/src/runtime.rs

msmouse · 2024-06-03T21:30:37Z

storage/indexer/src/db_tailer.rs

+};
+use std::sync::Arc;
+
+pub struct DBTailer {


why not just call it InternalIndexer or something along that line?
DBTrailer::db sounds like what the thing is tailing.

I think it is just tailing the db . Internalindexer can be reserved for sth combining the table info and this tailer?

renamed to DBindexer

storage/indexer/src/db_tailer.rs

msmouse · 2024-06-03T21:38:58Z

storage/indexer/src/db_tailer.rs

+        });
+        batch.put::<TailerMetadataSchema>(&version, &())?;
+        self.db.write_schemas(batch)?;
+        Ok(version)


Shall we sleep 10ms, for example, if it didn't see any new txns? (Or did you do it somewhere else?)

and shall we make it a pipeline (building the batch and committing it) from the beginning?

It sleeps at the tailer service run function if no new data from db iterator. made it pipeline

storage/indexer/src/schema/transaction_by_account/mod.rs

storage/indexer/src/utils.rs

codecov · 2024-06-18T21:33:11Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (main@c31afdf). Learn more about missing BASE report.

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #13240   +/-   ##
=======================================
  Coverage        ?    58.9%           
=======================================
  Files           ?      817           
  Lines           ?   195823           
  Branches        ?        0           
=======================================
  Hits            ?   115374           
  Misses          ?    80449           
  Partials        ?        0

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Add test for internal indexer merge two traits address review

github-actions · 2024-06-19T03:10:00Z

✅ Forge suite `realistic_env_max_load` success on `9cf89e8570ebfdca7eb85085453c6148ed3877f8`

two traffics test: inner traffic : committed: 8602.365229563708 txn/s, latency: 4550.537874044549 ms, (p50: 4400 ms, p90: 6000 ms, p99: 9600 ms), latency samples: 3718140
two traffics test : committed: 100.06585463003883 txn/s, latency: 1924.05625 ms, (p50: 1900 ms, p90: 2200 ms, p99: 2700 ms), latency samples: 1760
Latency breakdown for phase 0: ["QsBatchToPos: max: 0.218, avg: 0.213", "QsPosToProposal: max: 0.257, avg: 0.245", "ConsensusProposalToOrdered: max: 0.302, avg: 0.289", "ConsensusOrderedToCommit: max: 0.376, avg: 0.364", "ConsensusProposalToCommit: max: 0.663, avg: 0.652"]
Max round gap was 1 [limit 4] at version 1818465. Max no progress secs was 5.014555 [limit 15] at version 1818465.
Test Ok

github-actions · 2024-06-19T03:12:30Z

✅ Forge suite `framework_upgrade` success on `a68e71c05caebf01504d4499110f3fba213fb53d` ==> `9cf89e8570ebfdca7eb85085453c6148ed3877f8`

Compatibility test results for a68e71c05caebf01504d4499110f3fba213fb53d ==> 9cf89e8570ebfdca7eb85085453c6148ed3877f8 (PR)
Upgrade the nodes to version: 9cf89e8570ebfdca7eb85085453c6148ed3877f8
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1229.0498102117558 txn/s, submitted: 1231.3128369291408 txn/s, failed submission: 2.2630267173849306 txn/s, expired: 2.2630267173849306 txn/s, latency: 2485.577306205119 ms, (p50: 2100 ms, p90: 4100 ms, p99: 7700 ms), latency samples: 108620
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1165.677968359623 txn/s, submitted: 1168.6680657528107 txn/s, failed submission: 2.9900973931876673 txn/s, expired: 2.9900973931876673 txn/s, latency: 2694.8539265982636 ms, (p50: 2100 ms, p90: 4800 ms, p99: 7000 ms), latency samples: 101360
5. check swarm health
Compatibility test for a68e71c05caebf01504d4499110f3fba213fb53d ==> 9cf89e8570ebfdca7eb85085453c6148ed3877f8 passed
Upgrade the remaining nodes to version: 9cf89e8570ebfdca7eb85085453c6148ed3877f8
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1163.4049556943844 txn/s, submitted: 1166.3117910632152 txn/s, failed submission: 2.906835368830866 txn/s, expired: 2.906835368830866 txn/s, latency: 2733.282798385547 ms, (p50: 1900 ms, p90: 4800 ms, p99: 9500 ms), latency samples: 104060
Test Ok

github-actions · 2024-06-19T14:47:04Z

✅ Forge suite `compat` success on `a68e71c05caebf01504d4499110f3fba213fb53d` ==> `9cf89e8570ebfdca7eb85085453c6148ed3877f8`

Compatibility test results for a68e71c05caebf01504d4499110f3fba213fb53d ==> 9cf89e8570ebfdca7eb85085453c6148ed3877f8 (PR)
1. Check liveness of validators at old version: a68e71c05caebf01504d4499110f3fba213fb53d
compatibility::simple-validator-upgrade::liveness-check : committed: 6201.205159279459 txn/s, latency: 4628.923355111453 ms, (p50: 3900 ms, p90: 6000 ms, p99: 25100 ms), latency samples: 260200
2. Upgrading first Validator to new version: 9cf89e8570ebfdca7eb85085453c6148ed3877f8
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 855.3177164651671 txn/s, latency: 29201.3332996769 ms, (p50: 33300 ms, p90: 46700 ms, p99: 48700 ms), latency samples: 49520
3. Upgrading rest of first batch to new version: 9cf89e8570ebfdca7eb85085453c6148ed3877f8
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 3018.275962853761 txn/s, latency: 10236.838734495212 ms, (p50: 10400 ms, p90: 14300 ms, p99: 14600 ms), latency samples: 127380
4. upgrading second batch to new version: 9cf89e8570ebfdca7eb85085453c6148ed3877f8
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 6548.3082043423765 txn/s, latency: 5053.862711494854 ms, (p50: 5100 ms, p90: 7800 ms, p99: 8700 ms), latency samples: 229320
5. check swarm health
Compatibility test for a68e71c05caebf01504d4499110f3fba213fb53d ==> 9cf89e8570ebfdca7eb85085453c6148ed3877f8 passed
Test Ok

areshand force-pushed the migrate_api_to_internal_indexer branch 6 times, most recently from 13555fa to a296a65 Compare May 17, 2024 23:10

areshand changed the title ~~[api]migrate to state_kv internal indexer~~ [api]migrate APIs to use internal indexer May 17, 2024

areshand force-pushed the migrate_api_to_internal_indexer branch 3 times, most recently from 827f6d4 to 2678686 Compare May 22, 2024 03:00

areshand changed the title ~~[api]migrate APIs to use internal indexer~~ [Sharded DB][1] migrate Event and Transaction Schemas May 22, 2024

areshand marked this pull request as ready for review May 22, 2024 03:01

areshand requested review from msmouse, lightmark, grao1991, JoshLind, gregnazario, banool and 0xmaayan as code owners May 22, 2024 03:01

areshand force-pushed the migrate_api_to_internal_indexer branch 2 times, most recently from a7ff4f9 to 7b8c2b9 Compare May 22, 2024 03:03

msmouse requested review from jillxuu, bowenyang007 and larry-aptos May 22, 2024 16:54

grao1991 reviewed May 29, 2024

View reviewed changes

areshand force-pushed the migrate_api_to_internal_indexer branch 2 times, most recently from 5d4ec3b to 7e12fcb Compare May 29, 2024 16:52

areshand requested a review from grao1991 May 29, 2024 17:14

msmouse reviewed Jun 3, 2024

View reviewed changes

This comment has been minimized.

Sign in to view

areshand force-pushed the migrate_api_to_internal_indexer branch from 84354c2 to 9a2e48f Compare June 18, 2024 03:18

This comment has been minimized.

Sign in to view

areshand force-pushed the migrate_api_to_internal_indexer branch from 9a2e48f to d072cd4 Compare June 18, 2024 21:23

This comment has been minimized.

Sign in to view

areshand force-pushed the migrate_api_to_internal_indexer branch from d072cd4 to d5dc9a6 Compare June 19, 2024 02:32

areshand added 2 commits June 18, 2024 19:38

[api] migrate event and transaction schemas

b44d92e

Add test for internal indexer merge two traits address review

add perf improvment to unblock

9cf89e8

areshand force-pushed the migrate_api_to_internal_indexer branch from d5dc9a6 to 9cf89e8 Compare June 19, 2024 02:38

This comment has been minimized.

Sign in to view

areshand merged commit 8501f33 into main Jun 19, 2024
46 of 49 checks passed

areshand deleted the migrate_api_to_internal_indexer branch June 19, 2024 14:47

[Sharded DB][1] migrate Event and Transaction Schemas #13240

[Sharded DB][1] migrate Event and Transaction Schemas #13240

Conversation

areshand commented May 9, 2024 • edited Loading

Description

Type of Change

Which Components or Systems Does This Change Impact?

How Has This Been Tested?

Key Areas to Review

Checklist

trunk-io bot commented May 9, 2024 • edited Loading

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

areshand Jun 5, 2024 • edited Loading

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

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

codecov bot commented Jun 18, 2024 • edited Loading

Codecov Report

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

github-actions bot commented Jun 19, 2024

✅ Forge suite realistic_env_max_load success on 9cf89e8570ebfdca7eb85085453c6148ed3877f8

This comment has been minimized.

github-actions bot commented Jun 19, 2024

✅ Forge suite framework_upgrade success on a68e71c05caebf01504d4499110f3fba213fb53d ==> 9cf89e8570ebfdca7eb85085453c6148ed3877f8

This comment has been minimized.

github-actions bot commented Jun 19, 2024

✅ Forge suite compat success on a68e71c05caebf01504d4499110f3fba213fb53d ==> 9cf89e8570ebfdca7eb85085453c6148ed3877f8

areshand commented May 9, 2024 •

edited

Loading

trunk-io bot commented May 9, 2024 •

edited

Loading

areshand Jun 5, 2024 •

edited

Loading

codecov bot commented Jun 18, 2024 •

edited

Loading

✅ Forge suite `realistic_env_max_load` success on `9cf89e8570ebfdca7eb85085453c6148ed3877f8`

✅ Forge suite `framework_upgrade` success on `a68e71c05caebf01504d4499110f3fba213fb53d` ==> `9cf89e8570ebfdca7eb85085453c6148ed3877f8`

✅ Forge suite `compat` success on `a68e71c05caebf01504d4499110f3fba213fb53d` ==> `9cf89e8570ebfdca7eb85085453c6148ed3877f8`