Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

elastic scaling RFC 103 end-to-end tests #6452

Open
wants to merge 20 commits into
base: master
Choose a base branch
from

Conversation

alindima
Copy link
Contributor

@alindima alindima commented Nov 12, 2024

Adds two new zombienet-sdk tests:

  • one which verifies that elastic scaling works correctly both with the MVP and the new RFC 103 implementation which sends the core selector as a UMP signal.
  • one which spawns a parachain with two collators, but only one of them is using v2 receipts. The parachain will still make progress but without full throughput

Also enables the V2 receipts node feature for testnet genesis config.

Part of #5049

@alindima alindima added R0-silent Changes should not be mentioned in any release notes T8-polkadot This PR/Issue is related to/affects the Polkadot network. T10-tests This PR/Issue is related to tests. labels Nov 12, 2024
@alindima alindima requested a review from a team as a code owner November 12, 2024 12:00
Comment on lines 15 to 18
# Wait for 20 relay chain blocks
elastic-validator-0: reports substrate_block_height{status="best"} is at least 20 within 140 seconds

elastic-validator-0: parachain 2100 block height is at least 20 within 30 seconds
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my general comment is that using zombienet-v1 for measuring throughput via timeout will always be hacky and result in flaky tests:

  • we are waiting the substrate block height to be at least 20, but our intent is to wait for the next timeslice to kick in - this check might have false positives (block number > 20, but timeslice didn't kick in)
    then we're trying to measure parachain throughput via block height and timeouts - again this is flaky, because we are not starting to wait from 0 - the risk is having "passing tests", but broken functionality

we should switch to using zombienet-sdk tests IMHO, especially for newly written tests. are there any blockers for doing this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair point. I chose not to do this in the beginning because it would have taken a longer time and I was quite busy.
I switched to using zombienet-sdk test now, please have a look

@alindima alindima changed the title elastic scaling RFC 103 end-to-end test elastic scaling RFC 103 end-to-end tests Dec 2, 2024
@paritytech-workflow-stopper
Copy link

All GitHub workflows were cancelled due to failure one of the required jobs.
Failed workflow url: https://github.com/paritytech/polkadot-sdk/actions/runs/12119271312
Failed job name: build-rustdoc

@alindima alindima added R0-silent Changes should not be mentioned in any release notes and removed R0-silent Changes should not be mentioned in any release notes labels Dec 2, 2024
@alindima alindima removed the R0-silent Changes should not be mentioned in any release notes label Dec 2, 2024
Copy link
Member

@ordian ordian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the rewrite! looks like it will be less flaky and we can assert more things

.wait_for_finalized_success()
.await?;

log::info!("2 more cores assigned to the parachain");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are they assigned instantly after the block with assign_core transaction was finalized? don't we need to wait for timeslice 1 here? i guess this is why we had to wait for 20 blocks before in the test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T8-polkadot This PR/Issue is related to/affects the Polkadot network. T10-tests This PR/Issue is related to tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants