feat: Auto migrate indexers to Control Plane #527

morgsmccauley · 2024-01-25T02:54:25Z

This PR builds on the Redis allowlist to auto migrate indexers from the existing infrastructure to the Control Plane. An account migration requires coordination between both the V1 and V2 architecture - the indexer must be removed/ignored from V1, and then correctly configured within V2.

Allowlist shape

Each allowlist entry now contain the following properties:

pub struct AllowlistEntry {
    account_id: AccountId, // The account which should be migrated
    v1_ack: bool,          // True if Coordinator V1 has acknowledged the entry
    migrated: bool,        // True if the migration was successful
    failed: bool,          // True if the migration failed
}

Coordinator V1

For Coordinator V1, the allowlist is really a Denylist, the code/types have therefore been named as such. Accounts within the "denylist" should be ignored completely by V1. Because we cannot guarantee the timing of when this "ignore" actually happens, a flag (v1_ack) will be set from V1. V2 will only take over once this flag has been set.

Accounts within the "denylist" will be filtered out of the in-memory registry. Any new indexer "registrations" will also be ignored. In-progress historical backfills haven't been considered as we'll disable this functionality anyway.

Coordinator V2

Once acknowledged by V1, Coordinator V2 will attempt to migrate all functions under the relevant account. The steps for migration are:

Remove the streams from the Redis streams set - preventing Runner from starting these indexers implicitly
Stop the existing V1 executors which have already been started via the streams set
Merge the existing historical (if it exists) and real-time streams

Once migrated, accounts which have v1_ack && migrated && !failed will be exposed to the control loop, prompting V2 to act on these indexers.

`migrated` flag

For now, the migrated flag will not be set on success, preventing V2 from running the indexer on the new architecture. There are some issues around V2 continuing from the right block correctly, so it's best to not run them for now. This allows us to test the migration in isolation, not worrying about what V2 does after that. I'll add this logic back in once #536 is complete.

`failed` flag

If any part of the migration fails, the failed flag will be set for that account. It would take a significant amount of time to cover all the edge cases in code so it would be faster to just set this flag to ignore the account, fix the migration manually and then reset the failed flag.

morgsmccauley · 2024-02-02T03:28:48Z

indexer/storage/src/lib.rs

+
+// `redis::transaction`s currently don't work with async connections, so we have to create a _new_
+// blocking connection to atmoically update a value.
+pub fn atomic_update<K, O, N, F>(redis_url: &str, key: K, update_fn: F) -> anyhow::Result<()>


This isn't ideal - blocking code blocks all async code, but given this happens infrequently, and is only temporary it should be ok.

morgsmccauley · 2024-02-02T03:30:19Z

coordinator/src/migration.rs

+    use crate::registry::IndexerConfig;
+
+    #[tokio::test]
+    async fn ignores_migrated_indexers() {


Could definitely expand on these cases but as Redis is mocked it wouldn't provide much benefit, the time would be better spent testing in Dev.

morgsmccauley · 2024-02-02T03:31:56Z

indexer/queryapi_coordinator/src/main.rs

+#[derive(serde::Deserialize, serde::Serialize, Debug)]
+struct DenylistEntry {
+    account_id: AccountId,
+    v1_ack: bool,


Haven't included migrated/failed because they're not needed - serde will just ignore them.

darunrs

LGTM! Really cool work on this!

darunrs · 2024-02-05T18:57:27Z

coordinator/src/migration.rs

+) -> anyhow::Result<()> {
+    tracing::info!("Migrating account {}", account_id);
+
+    for (_, indexer_config) in indexers.iter() {


Really cool workflow in this! I can see how it's friendly to both a re-run (picking up after manual fix of failed migration) and running for the first time.

darunrs · 2024-02-05T19:25:08Z

indexer/queryapi_coordinator/src/main.rs

+
+        match account_in_deny_list {
+            Some(account_in_deny_list) => {
+                tracing::info!(


This will be printed on every ignored indexer for every block coordinator publishes right? This would probably clog up the logs. Maybe its better to collect either ignored or not-ignored indexers in a list and print it all together at the end, instead.

No - because the registry is mutated as we go, only "new" removals will be logged

morgsmccauley linked an issue Jan 25, 2024 that may be closed by this pull request

Migrate Indexers to Control Plane #520

Closed

morgsmccauley force-pushed the feat/v2-migration branch 3 times, most recently from 17cf449 to 9626c7c Compare January 31, 2024 19:37

morgsmccauley force-pushed the feat/v2-migration branch from 555df8a to 92e9d54 Compare February 1, 2024 22:35

morgsmccauley changed the title ~~feat/v2 migration~~ feat: Auto migrate indexers to Control Plane Feb 2, 2024

morgsmccauley added 24 commits February 2, 2024 16:26

feat: Ignore accounts from allowlist in Coordinator V1 on load

5d19bdf

feat: Ignore indexed accounts from deny list

3f419eb

refactor: Rename DenyList -> Denylist

db53c90

refactor: Extract denylist filtering to function

65f06a4

feat: Remove accounts in denylist after startup

664a093

feat: Acknowledge denying accounts from Coordinator V1

ec53e83

feat: Filter accounts which have been acknowledged only

a2ff213

feat(migration): Prevent indexers from being started by Runner

e97d682

feat(migration): Stop existing V1 executors

d39e468

feat(migration): Move historical/real-time to combined stream

5682738

refactor(migration): Move migrate logic to own file

fc97ca0

fix(migration): Stop stream merge when no messages left

b4b7096

test(migration): Test migration to v2

bf0e989

feat(migration): Delete real time stream messages after move

68d99a1

refactor(migration): Extract migration steps to own functions

0d65508

feat(migration): Set migrated flag on complete

76c554b

feat(migration): Add logging

22a5e95

fix(migration): Only migrate indexers which have been ack by v1

fa3ade2

refactor: Move allowlist logic to migrate mod

69aa739

refactor: Rename migrate.rs -> migartion.rs

ef445f2

refactor: Rename function

d1940fa

refactor: Extract account migration logic

d8f468c

feat(migration): Add more verbose logging

2b9bfd2

feat: Only migrate streams which exist

571f7d8

morgsmccauley added 3 commits February 2, 2024 16:26

fix: Only act on account which have been successfully migrated

9f74d0e

feat: Prevent V2 from running migrated indexers

384f20c

chore: Add comments

84dd294

morgsmccauley force-pushed the feat/v2-migration branch from 6d8a4b6 to 84dd294 Compare February 2, 2024 03:26

morgsmccauley marked this pull request as ready for review February 2, 2024 03:26

morgsmccauley requested a review from a team as a code owner February 2, 2024 03:26

morgsmccauley commented Feb 2, 2024

View reviewed changes

darunrs approved these changes Feb 5, 2024

View reviewed changes

morgsmccauley merged commit 9ef1f29 into main Feb 6, 2024
8 checks passed

morgsmccauley deleted the feat/v2-migration branch February 6, 2024 19:07

morgsmccauley mentioned this pull request Apr 22, 2024

test stable branch git fix up #687

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Auto migrate indexers to Control Plane #527

feat: Auto migrate indexers to Control Plane #527

morgsmccauley commented Jan 25, 2024 •

edited

Loading

morgsmccauley Feb 2, 2024

morgsmccauley Feb 2, 2024

morgsmccauley Feb 2, 2024

darunrs left a comment

darunrs Feb 5, 2024 •

edited

Loading

darunrs Feb 5, 2024

morgsmccauley Feb 6, 2024

feat: Auto migrate indexers to Control Plane #527

feat: Auto migrate indexers to Control Plane #527

Conversation

morgsmccauley commented Jan 25, 2024 • edited Loading

Allowlist shape

Coordinator V1

Coordinator V2

migrated flag

failed flag

morgsmccauley Feb 2, 2024

Choose a reason for hiding this comment

morgsmccauley Feb 2, 2024

Choose a reason for hiding this comment

morgsmccauley Feb 2, 2024

Choose a reason for hiding this comment

darunrs left a comment

Choose a reason for hiding this comment

darunrs Feb 5, 2024 • edited Loading

Choose a reason for hiding this comment

darunrs Feb 5, 2024

Choose a reason for hiding this comment

morgsmccauley Feb 6, 2024

Choose a reason for hiding this comment

morgsmccauley commented Jan 25, 2024 •

edited

Loading

`migrated` flag

`failed` flag

darunrs Feb 5, 2024 •

edited

Loading