[indexer-alt] pruning tests for pipelines that depend on `CpSequenceNumbers` for tx intervals #20744

wlmyng · 2024-12-27T16:50:08Z

Description

pruning tests for pipelines that depend on CpSequenceNumbers for tx intervals

Test plan

All tests!

Release notes

Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required.

For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates.

vercel · 2024-12-27T16:50:12Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
sui-docs	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jan 17, 2025 11:24pm

3 Skipped Deployments

Name	Status	Preview	Updated (UTC)
multisig-toolkit	⬜️ Ignored (Inspect)	Visit Preview	Jan 17, 2025 11:24pm
sui-kiosk	⬜️ Ignored (Inspect)	Visit Preview	Jan 17, 2025 11:24pm
sui-typescript-docs	⬜️ Ignored (Inspect)		Jan 17, 2025 11:24pm

crates/sui-indexer-alt/tests/pruning/pruner_tests.rs

crates/sui-indexer-alt/tests/pruning/configs/base_config.toml

lxfind · 2025-01-08T03:59:24Z

crates/sui-indexer-alt/tests/pruning/pruner_tests.rs

+    (temp_db, temp_dir)
+}
+
+async fn setup_test_env(


We should extend the current Indexer::new_for_testing() into a simple test indexer builder.

admittedly, I was a little lazy and skipped this ... but can circle back

This seems to be doing something that's specific to sui-indexer-alt (because it uses an IndexerConfig) whereas new_for_testing is part of the framework. Could we split up start_indexer so that it accepts an instance of Indexer to populate that would help make best use of both things?

amnn

Some high level comments/themes:

The derive(Default)s should be unnecessary on all the pipelines.
Can we standardise the name on cp_sequence_numbers instead of cp_mapping? (This may no longer be relevant after applying the comment about derive(Default)).
In hindsight (and for next time) it would have been nice to put this out a pipeline at a time (break up the PR) -- that way we could identify common patterns like the above more quickly and avoid lots of fix ups.
It would be really great if we could find a way to augment TestCheckpointDataBuilder so that we could use it for the epoch pipeline tests as well.

crates/sui-indexer-alt-framework/src/handlers/cp_sequence_numbers.rs

crates/sui-indexer-alt/src/handlers/ev_emit_mod.rs

crates/sui-indexer-alt/src/handlers/kv_checkpoints.rs

crates/sui-indexer-alt/src/handlers/kv_epoch_ends.rs

crates/sui-indexer-alt/src/handlers/obj_info.rs

amnn · 2025-01-15T09:54:01Z

crates/sui-indexer-alt/src/lib.rs

@@ -44,6 +44,7 @@ pub async fn start_indexer(
    // TODO: There is probably a better way to handle this.
    // For instance, we could also pass in dummy genesis data in the benchmark mode.
    with_genesis: bool,
+    cancel: Option<CancellationToken>,


I really, really dislike the pattern of making the CancellationToken optional and creating a new one if one is not supplied because it makes it difficult to track where a given token originates from -- this was a source of pain for us multiple times in the previous indexer implementation, where we would forget to replace a None with a Some(...) and we had cancellation token "split-brain".

If the indexer needs to share space with another service, pass the responsibility of creating the cancellation token unequivocally up to the parent.

Suggested change

cancel: Option<CancellationToken>,

cancel: CancellationToken,

amnn · 2025-01-15T10:02:55Z

crates/sui-indexer-alt/tests/pruning/pruner_tests.rs

+    (temp_db, temp_dir)
+}
+
+async fn setup_test_env(


This seems to be doing something that's specific to sui-indexer-alt (because it uses an IndexerConfig) whereas new_for_testing is part of the framework. Could we split up start_indexer so that it accepts an instance of Indexer to populate that would help make best use of both things?

wlmyng · 2025-01-18T00:51:05Z

It would be really great if we could find a way to augment TestCheckpointDataBuilder so that we could use it for the epoch pipeline tests as well.

Looks like this would require an advance epoch transaction, which has the system state as an output object, added to the checkpoint. I've got most of this down, except the system state part ... worst case scenario, I think we can spin up simulacrum, take the VerifiedCheckpoint and convert it into CheckpointData to feed to the pipeline? But I think I can work something out. Will do so in a follow-up PR

wlmyng · 2025-01-19T00:43:13Z

It would be really great if we could find a way to augment TestCheckpointDataBuilder so that we could use it for the epoch pipeline tests as well.

Looks like this would require an advance epoch transaction, which has the system state as an output object, added to the checkpoint. I've got most of this down, except the system state part ... worst case scenario, I think we can spin up simulacrum, take the VerifiedCheckpoint and convert it into CheckpointData to feed to the pipeline? But I think I can work something out. Will do so in a follow-up PR

it's a bit easier for kv_epoch_ends, since we can get away with the advance epoch transaction and the presence of a SystemEpochInfoEvent if not in safe mode. I think I can also take the learnings there and apply it for kv_epoch_starts

wlmyng temporarily deployed to sui-typescript-aws-kms-test-env December 27, 2024 16:50 — with GitHub Actions Inactive

vercel bot deployed to Preview – sui-docs December 27, 2024 16:51 View deployment

wlmyng force-pushed the indexer-alt-prune-impls branch from e8252bf to 040df88 Compare January 8, 2025 00:54

wlmyng requested a review from ronny-mysten as a code owner January 8, 2025 00:54

lxfind reviewed Jan 8, 2025

View reviewed changes

wlmyng force-pushed the indexer-alt-prune-impls branch from 9fd872e to 5ef7c04 Compare January 9, 2025 16:56

Base automatically changed from indexer-alt-prune-impls to main January 9, 2025 19:17

wlmyng force-pushed the indexer-alt-prune-tests branch from 72e140e to 794ec20 Compare January 13, 2025 18:54

wlmyng temporarily deployed to sui-typescript-aws-kms-test-env January 13, 2025 18:54 — with GitHub Actions Inactive

vercel bot deployed to Preview – sui-docs January 13, 2025 18:59 View deployment

wlmyng temporarily deployed to sui-typescript-aws-kms-test-env January 15, 2025 02:45 — with GitHub Actions Inactive

wlmyng requested a review from lxfind January 15, 2025 02:45

wlmyng force-pushed the indexer-alt-prune-tests branch from da186e3 to 491e105 Compare January 15, 2025 02:45

wlmyng requested a review from amnn January 15, 2025 02:45

wlmyng temporarily deployed to sui-typescript-aws-kms-test-env January 15, 2025 02:46 — with GitHub Actions Inactive

vercel bot deployed to Preview – sui-docs January 15, 2025 02:49 View deployment

amnn reviewed Jan 15, 2025

View reviewed changes

wlmyng force-pushed the indexer-alt-prune-tests branch from 491e105 to 44f663f Compare January 17, 2025 23:08

wlmyng temporarily deployed to sui-typescript-aws-kms-test-env January 17, 2025 23:08 — with GitHub Actions Inactive

wlmyng changed the title ~~[indexer-alt] pruning tests~~ [indexer-alt] pruning tests for pipelines that depend on CpSequenceNumbers for tx intervals Jan 17, 2025

vercel bot deployed to Preview – sui-docs January 17, 2025 23:10 View deployment

wlmyng force-pushed the indexer-alt-prune-tests branch from 44f663f to 814dec7 Compare January 17, 2025 23:10

wlmyng temporarily deployed to sui-typescript-aws-kms-test-env January 17, 2025 23:11 — with GitHub Actions Inactive

vercel bot deployed to Preview – sui-docs January 17, 2025 23:12 View deployment

tests involving CpSequenceNumber for tx interval

2a558e4

wlmyng force-pushed the indexer-alt-prune-tests branch from 814dec7 to 2a558e4 Compare January 17, 2025 23:23

wlmyng temporarily deployed to sui-typescript-aws-kms-test-env January 17, 2025 23:23 — with GitHub Actions Inactive

vercel bot deployed to Preview – sui-docs January 17, 2025 23:24 View deployment

lxfind approved these changes Jan 17, 2025

View reviewed changes

wlmyng merged commit be9a0ee into main Jan 21, 2025
51 checks passed

wlmyng deleted the indexer-alt-prune-tests branch January 21, 2025 18:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[indexer-alt] pruning tests for pipelines that depend on `CpSequenceNumbers` for tx intervals #20744

[indexer-alt] pruning tests for pipelines that depend on `CpSequenceNumbers` for tx intervals #20744

wlmyng commented Dec 27, 2024 •

edited

Loading

vercel bot commented Dec 27, 2024 •

edited

Loading

lxfind Jan 8, 2025

wlmyng Jan 15, 2025

amnn Jan 15, 2025

amnn left a comment

amnn Jan 15, 2025

amnn Jan 15, 2025

wlmyng commented Jan 18, 2025

wlmyng commented Jan 19, 2025

	cancel: Option<CancellationToken>,
	cancel: CancellationToken,

[indexer-alt] pruning tests for pipelines that depend on CpSequenceNumbers for tx intervals #20744

[indexer-alt] pruning tests for pipelines that depend on CpSequenceNumbers for tx intervals #20744

Conversation

wlmyng commented Dec 27, 2024 • edited Loading

Description

Test plan

Release notes

vercel bot commented Dec 27, 2024 • edited Loading

lxfind Jan 8, 2025

Choose a reason for hiding this comment

wlmyng Jan 15, 2025

Choose a reason for hiding this comment

amnn Jan 15, 2025

Choose a reason for hiding this comment

amnn left a comment

Choose a reason for hiding this comment

amnn Jan 15, 2025

Choose a reason for hiding this comment

amnn Jan 15, 2025

Choose a reason for hiding this comment

wlmyng commented Jan 18, 2025

wlmyng commented Jan 19, 2025

[indexer-alt] pruning tests for pipelines that depend on `CpSequenceNumbers` for tx intervals #20744

[indexer-alt] pruning tests for pipelines that depend on `CpSequenceNumbers` for tx intervals #20744

wlmyng commented Dec 27, 2024 •

edited

Loading

vercel bot commented Dec 27, 2024 •

edited

Loading