runtime: remove ttl #5461

alindima · 2024-08-26T08:37:13Z

Resolves #4776

This will enable proper core-sharing between paras, even if one of them is not producing blocks.

TODO:

duplicate first entry in the claim queue if the queue used to be empty
don't back anything if at the end of the block there'll be a session change
write migration for removing the availability core storage
update and write unit tests
prdoc
add zombienet test for synchronous backing
add zombienet test for core-sharing paras where one of them is not producing any blocks

Important note:
The ttl and max_availability_timeouts fields of the HostConfiguration are not removed in this PR, due to #64.
Adding the workaround with the storage version check for every use of the active HostConfiguration in all runtime APIs would be insane, as it's used in almost all runtime APIs.

So even though the ttl and max_availability_timeouts fields will now be unused, they will remain part of the host configuration.

These will be removed in a separate PR once #64 is fixed. Tracked by #6067

Signed-off-by: Andrei Sandu <[email protected]>

command-bot · 2024-10-17T08:53:57Z

@alindima https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7591514 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=pallet --runtime=westend --target_dir=polkadot --pallet=polkadot_runtime_parachains::paras_inherent. Check out https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 14-25b27c8e-2ab2-4725-b285-d343fd881212 to cancel this command or bot cancel to cancel all commands in this pull request.

command-bot · 2024-10-17T14:05:09Z

@alindima Command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=pallet --runtime=westend --target_dir=polkadot --pallet=polkadot_runtime_parachains::paras_inherent has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7591514 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7591514/artifacts/download.

alindima · 2024-10-17T14:17:30Z

bot bench polkadot-pallet --pallet=polkadot_runtime_parachains::paras_inherent --runtime=westend -v UPSTREAM_MERGE=n

command-bot · 2024-10-17T14:17:35Z

@alindima https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7595155 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=pallet --runtime=westend --target_dir=polkadot --pallet=polkadot_runtime_parachains::paras_inherent. Check out https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 19-557204d2-9955-4c63-ad77-ae8044403d48 to cancel this command or bot cancel to cancel all commands in this pull request.

…=westend --target_dir=polkadot --pallet=polkadot_runtime_parachains::paras_inherent

command-bot · 2024-10-17T19:26:37Z

@alindima Command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=pallet --runtime=westend --target_dir=polkadot --pallet=polkadot_runtime_parachains::paras_inherent has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7595155 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7595155/artifacts/download.

…ma/remove-ttl

alindima · 2024-10-21T09:30:28Z

bot bench polkadot-pallet --pallet=polkadot_runtime_parachains::paras_inherent --runtime=rococo -v UPSTREAM_MERGE=n

command-bot · 2024-10-21T09:30:34Z

@alindima https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7607482 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=pallet --runtime=rococo --target_dir=polkadot --pallet=polkadot_runtime_parachains::paras_inherent. Check out https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 1-19553a01-d9e9-4e77-8c4c-b745786ff00a to cancel this command or bot cancel to cancel all commands in this pull request.

eskimor

Looks good!

polkadot/runtime/parachains/src/disputes.rs

eskimor · 2024-10-21T09:04:43Z

polkadot/runtime/parachains/src/inclusion/mod.rs

@@ -514,6 +486,14 @@ impl<T: Config> Pallet<T> {
 		T::MessageQueue::sweep_queue(AggregateMessageOrigin::Ump(UmpQueueId::Para(para)));
 	}

+	pub(crate) fn get_occupied_cores(


Nice! Who to ask for occupied cores ... inclusion! ;-)

eskimor · 2024-10-21T09:06:49Z

polkadot/runtime/parachains/src/inclusion/mod.rs

@@ -1173,7 +1151,7 @@ impl<T: Config> Pallet<T> {

 	/// Returns the first `CommittedCandidateReceipt` pending availability for the para provided, if
 	/// any.
-	pub(crate) fn candidate_pending_availability(
+	pub(crate) fn first_candidate_pending_availability(


Nit: What does "first" mean?

added an explanation in the doc block

eskimor · 2024-10-21T09:33:11Z

polkadot/runtime/parachains/src/paras_inherent/mod.rs


-			Self::process_inherent_data(data, ProcessInherentDataContext::Enter)
-				.map(|(_processed, post_info)| post_info)
+			Self::process_inherent_data(data).and_then(|(processed, post_info)| {


Thanks! :-)

Future optimization possibilities, if we ever needed to:

Hashing instead of full cloning.

Counting bytes/vec sizes and only compare total size in the end.

eskimor · 2024-10-21T09:37:57Z

polkadot/runtime/parachains/src/paras_inherent/mod.rs

+			let mut eligible: BTreeMap<ParaId, BTreeSet<CoreIndex>> = BTreeMap::new();
+			let mut total_eligible_cores = 0;
+
+			for (core_idx, para_id) in Self::eligible_paras(&occupied_cores) {


Not sure I follow the reasoning.

eskimor · 2024-10-21T09:39:53Z

polkadot/runtime/parachains/src/paras_inherent/mod.rs

+	pub(crate) fn eligible_paras<'a>(
+		occupied_cores: &'a BTreeSet<CoreIndex>,
+	) -> impl Iterator<Item = (CoreIndex, ParaId)> + 'a {
+		scheduler::ClaimQueue::<T>::get().into_iter().filter_map(|(core_idx, queue)| {


Not for this PR, but for the future: Let's aim to have a proper API for modules. Accessing another pallet's storage directly is code smell: We apparently don't have clear responsibilities nor an API that is sufficient to communicate with that component.

eskimor · 2024-10-21T09:46:13Z

polkadot/runtime/parachains/src/paras_inherent/tests.rs

@@ -133,6 +127,13 @@ mod enter {
 		}
 	}

+	// Clear the on-chain dispute data.
+	fn clear_dispute_storage<Test: disputes::Config>() {


This could be a cfg-test only clear_dispute_storage() function in the disputes module (avoiding making storage public).

eskimor · 2024-10-21T10:12:00Z

polkadot/runtime/parachains/src/paras_inherent/tests.rs

@@ -826,6 +919,8 @@ mod enter {
 			// over weight
 			assert_eq!(limit_inherent_data.backed_candidates.len(), 0);

+			clear_dispute_storage::<Test>();


Wouldn't it be cleaner to just create a new test environment before the enter call?

eskimor · 2024-10-21T10:29:58Z

polkadot/runtime/parachains/src/runtime_api_impl/v11.rs

-			CoreOccupied::Free => {
-				if let Some(para_id) = scheduled.get(&CoreIndex(i as _)).cloned() {
+			} else {
+				if let Some(assignment) = scheduler::Pallet::<T>::peek_claim_queue(&core_idx) {


Nit: Fetching the entire claim queue on each iteration is rather inefficient.

That's were the iterator idea came from. With this we could just zip the two iterators and loop through both just once.

Anyhow, this call is actually obsolete and only exists for backwards compatibility? If so, we can just not bother and also mark the runtime api as deprecated.

it's not realy obsolete, as there's no other runtime API we can use to find out the state of a core (whether it's occupied or not). And this is still needed

Nit: Fetching the entire claim queue on each iteration is rather inefficient.

True, I can change it to only query once

eskimor · 2024-10-21T10:59:38Z

polkadot/runtime/rococo/src/lib.rs

@@ -1710,7 +1710,8 @@ pub mod migrations {
        // permanent
        pallet_xcm::migration::MigrateToLatestXcmVersion<Runtime>,
        parachains_inclusion::migration::MigrateToV1<Runtime>,
-        parachains_shared::migration::MigrateToV1<Runtime>,
+		parachains_shared::migration::MigrateToV1<Runtime>,


Formatting looks off?

Looks fine to me

…=rococo --target_dir=polkadot --pallet=polkadot_runtime_parachains::paras_inherent

command-bot · 2024-10-21T14:40:05Z

@alindima Command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=pallet --runtime=rococo --target_dir=polkadot --pallet=polkadot_runtime_parachains::paras_inherent has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7607482 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7607482/artifacts/download.

Reported in #6161 (comment) Fixes a bug introduced in #5461, where the claim queue would contain entries even if the validator groups storage is empty (which happens during the first session). This PR sets the claim queue core count to be the minimum between the num_cores param and the number of validator groups TODO: - [x] prdoc - [x] unit test

sandreim added 30 commits August 12, 2024 15:07

WIP primitives

793142e

Signed-off-by: Andrei Sandu <[email protected]>

WIP

3a29fdf

Signed-off-by: Andrei Sandu <[email protected]>

Working version.

4a53577

Signed-off-by: Andrei Sandu <[email protected]>

Better version

8285ae7

Signed-off-by: Andrei Sandu <[email protected]>

Add missing primitives and fix things

2831c5e

Signed-off-by: Andrei Sandu <[email protected]>

Implement v2 receipts in polkadot-runtime-parachains

c767d60

Signed-off-by: Andrei Sandu <[email protected]>

add missing stuff

96999e3

Signed-off-by: Andrei Sandu <[email protected]>

Switch parachains runtime to use new primitives

c5f2dc3

Signed-off-by: Andrei Sandu <[email protected]>

use vstaging primitives

dbb0160

Signed-off-by: Andrei Sandu <[email protected]>

update rococo and westend

5efab68

Signed-off-by: Andrei Sandu <[email protected]>

client keeps using the old primitives

c2232e4

Signed-off-by: Andrei Sandu <[email protected]>

no unsafe pls

87b079f

Signed-off-by: Andrei Sandu <[email protected]>

move async backing primtiives to own file

00e8c13

Signed-off-by: Andrei Sandu <[email protected]>

fix

cd4d02f

Signed-off-by: Andrei Sandu <[email protected]>

fix test build

5509e33

Signed-off-by: Andrei Sandu <[email protected]>

fix test-runtime

f8b86d2

Signed-off-by: Andrei Sandu <[email protected]>

self review feedback

fe2fbfb

Signed-off-by: Andrei Sandu <[email protected]>

review feedback

975e13b

Signed-off-by: Andrei Sandu <[email protected]>

feedback

1c7ac55

Signed-off-by: Andrei Sandu <[email protected]>

feedback

653873b

Signed-off-by: Andrei Sandu <[email protected]>

clippy

dc98149

Signed-off-by: Andrei Sandu <[email protected]>

chores

0a6bce3

Signed-off-by: Andrei Sandu <[email protected]>

Filter v2 candidate descriptors

5e4dac2

Signed-off-by: Andrei Sandu <[email protected]>

fix

f12ca7a

Signed-off-by: Andrei Sandu <[email protected]>

fix prospective parachains tests

13734de

Signed-off-by: Andrei Sandu <[email protected]>

fix fix

effb1cc

Signed-off-by: Andrei Sandu <[email protected]>

fmt

3f75cba

Signed-off-by: Andrei Sandu <[email protected]>

fix comment

75a47bb

Signed-off-by: Andrei Sandu <[email protected]>

another one

12ed853

Signed-off-by: Andrei Sandu <[email protected]>

fix build

f2c0882

Signed-off-by: Andrei Sandu <[email protected]>

fix benchmarks one last time

04a541b

".git/.scripts/commands/bench/bench.sh" --subcommand=pallet --runtime…

1bfb31b

…=westend --target_dir=polkadot --pallet=polkadot_runtime_parachains::paras_inherent

alindima added 6 commits October 21, 2024 11:48

switch test runtime to coretime

10f7c72

fix prdoc

acf109d

Merge remote-tracking branch 'origin/master' into alindima/remove-ttl

ec5c45f

Merge remote-tracking branch 'origin/alindima/remove-ttl' into alindi…

527a80c

…ma/remove-ttl

try fixing prdoc

e93a05f

fix prdoc

14c1641

eskimor reviewed Oct 21, 2024

View reviewed changes

eskimor approved these changes Oct 21, 2024

View reviewed changes

alindima and others added 3 commits October 21, 2024 16:28

review feedback

0680a2c

fix clippy

dea739c

".git/.scripts/commands/bench/bench.sh" --subcommand=pallet --runtime…

2c7b6ca

…=rococo --target_dir=polkadot --pallet=polkadot_runtime_parachains::paras_inherent

alindima enabled auto-merge October 21, 2024 18:36

alindima added this pull request to the merge queue Oct 21, 2024

Merged via the queue into master with commit ee803b7 Oct 21, 2024
192 of 195 checks passed

alindima deleted the alindima/remove-ttl branch October 21, 2024 19:55

This was referenced Oct 23, 2024

zombienet-bridges-* test failures #6161

Closed

Fix zombienet-bridges-0001-asset-transfer-works #6175

Merged

alindima mentioned this pull request Oct 28, 2024

fix claim queue size #6257

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

runtime: remove ttl #5461

runtime: remove ttl #5461

alindima commented Aug 26, 2024 •

edited

Loading

command-bot bot commented Oct 17, 2024 •

edited

Loading

command-bot bot commented Oct 17, 2024

alindima commented Oct 17, 2024

command-bot bot commented Oct 17, 2024 •

edited

Loading

command-bot bot commented Oct 17, 2024

alindima commented Oct 21, 2024

command-bot bot commented Oct 21, 2024 •

edited

Loading

eskimor left a comment

eskimor Oct 21, 2024

eskimor Oct 21, 2024

alindima Oct 21, 2024

eskimor Oct 21, 2024

eskimor Oct 21, 2024

eskimor Oct 21, 2024

eskimor Oct 21, 2024

alindima Oct 21, 2024

eskimor Oct 21, 2024

eskimor Oct 21, 2024

eskimor Oct 21, 2024

alindima Oct 21, 2024

alindima Oct 21, 2024

alindima Oct 21, 2024

eskimor Oct 21, 2024

alindima Oct 21, 2024

command-bot bot commented Oct 21, 2024

runtime: remove ttl #5461

runtime: remove ttl #5461

Conversation

alindima commented Aug 26, 2024 • edited Loading

command-bot bot commented Oct 17, 2024 • edited Loading

command-bot bot commented Oct 17, 2024

alindima commented Oct 17, 2024

command-bot bot commented Oct 17, 2024 • edited Loading

command-bot bot commented Oct 17, 2024

alindima commented Oct 21, 2024

command-bot bot commented Oct 21, 2024 • edited Loading

eskimor left a 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

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

command-bot bot commented Oct 21, 2024

alindima commented Aug 26, 2024 •

edited

Loading

command-bot bot commented Oct 17, 2024 •

edited

Loading

command-bot bot commented Oct 17, 2024 •

edited

Loading

command-bot bot commented Oct 21, 2024 •

edited

Loading