diff --git a/cumulus/pallets/xcmp-queue/src/lib.rs b/cumulus/pallets/xcmp-queue/src/lib.rs index 5b900769622af..e92169be16b0b 100644 --- a/cumulus/pallets/xcmp-queue/src/lib.rs +++ b/cumulus/pallets/xcmp-queue/src/lib.rs @@ -600,7 +600,7 @@ impl Pallet { let QueueConfigData { drop_threshold, .. } = >::get(); let fp = T::XcmpQueue::footprint(sender); // Assume that it will not fit into the current page: - let new_pages = fp.pages.saturating_add(1); + let new_pages = fp.ready_pages.saturating_add(1); if new_pages > drop_threshold { // This should not happen since the channel should have been suspended in // [`on_queue_changed`]. @@ -663,12 +663,12 @@ impl OnQueueChanged for Pallet { let mut suspended_channels = >::get(); let suspended = suspended_channels.contains(¶); - if suspended && fp.pages <= resume_threshold { + if suspended && fp.ready_pages <= resume_threshold { Self::send_signal(para, ChannelSignal::Resume); suspended_channels.remove(¶); >::put(suspended_channels); - } else if !suspended && fp.pages >= suspend_threshold { + } else if !suspended && fp.ready_pages >= suspend_threshold { log::warn!("XCMP queue for sibling {:?} is full; suspending channel.", para); Self::send_signal(para, ChannelSignal::Suspend); diff --git a/cumulus/pallets/xcmp-queue/src/mock.rs b/cumulus/pallets/xcmp-queue/src/mock.rs index 08ab58ce81604..1fb88cafd93c4 100644 --- a/cumulus/pallets/xcmp-queue/src/mock.rs +++ b/cumulus/pallets/xcmp-queue/src/mock.rs @@ -247,6 +247,7 @@ impl> EnqueueMessage for EnqueueToLocalStorage } } footprint.pages = footprint.storage.size as u32 / 16; // Number does not matter + footprint.ready_pages = footprint.pages; footprint } } diff --git a/prdoc/pr_2393.prdoc b/prdoc/pr_2393.prdoc new file mode 100644 index 0000000000000..708d017fafd50 --- /dev/null +++ b/prdoc/pr_2393.prdoc @@ -0,0 +1,16 @@ +title: "[XCMP] Use the number of 'ready' pages in XCMP suspend logic" + +doc: + - audience: Runtime Dev + description: | + Semantics of the suspension logic in the XCMP queue pallet change from using the number of + total pages to the number of 'ready' pages. The number of ready pages is now also exposed by + the `MessageQueue` pallet to downstream via the queue `footprint`. + +crates: + - name: cumulus-pallet-xcmp-queue + bump: patch + - name: pallet-message-queue + bump: patch + - name: frame-support + bump: major diff --git a/substrate/frame/message-queue/src/integration_test.rs b/substrate/frame/message-queue/src/integration_test.rs index cc3da6ebdc669..ce8eb80805ab0 100644 --- a/substrate/frame/message-queue/src/integration_test.rs +++ b/substrate/frame/message-queue/src/integration_test.rs @@ -330,6 +330,11 @@ fn process_some_messages(num_msgs: u32) { ServiceWeight::set(Some(weight)); let consumed = next_block(); + for origin in BookStateFor::::iter_keys() { + let fp = MessageQueue::footprint(origin); + assert_eq!(fp.pages, fp.ready_pages); + } + assert_eq!(consumed, weight, "\n{}", MessageQueue::debug_info()); assert_eq!(NumMessagesProcessed::take(), num_msgs as usize); } diff --git a/substrate/frame/message-queue/src/lib.rs b/substrate/frame/message-queue/src/lib.rs index 07eb004198534..61028057394f6 100644 --- a/substrate/frame/message-queue/src/lib.rs +++ b/substrate/frame/message-queue/src/lib.rs @@ -208,8 +208,9 @@ use frame_support::{ defensive, pallet_prelude::*, traits::{ - Defensive, DefensiveTruncateFrom, EnqueueMessage, ExecuteOverweightError, Footprint, - ProcessMessage, ProcessMessageError, QueueFootprint, QueuePausedQuery, ServiceQueues, + Defensive, DefensiveSaturating, DefensiveTruncateFrom, EnqueueMessage, + ExecuteOverweightError, Footprint, ProcessMessage, ProcessMessageError, QueueFootprint, + QueuePausedQuery, ServiceQueues, }, BoundedSlice, CloneNoBound, DefaultNoBound, }; @@ -442,6 +443,7 @@ impl From> for QueueFootprint { fn from(book: BookState) -> Self { QueueFootprint { pages: book.count, + ready_pages: book.end.defensive_saturating_sub(book.begin), storage: Footprint { count: book.message_count, size: book.size }, } } @@ -1281,6 +1283,9 @@ impl Pallet { ensure!(book.message_count < 1 << 30, "Likely overflow or corruption"); ensure!(book.size < 1 << 30, "Likely overflow or corruption"); ensure!(book.count < 1 << 30, "Likely overflow or corruption"); + + let fp: QueueFootprint = book.into(); + ensure!(fp.ready_pages <= fp.pages, "There cannot be more ready than total pages"); } //loop around this origin diff --git a/substrate/frame/message-queue/src/mock.rs b/substrate/frame/message-queue/src/mock.rs index a46fa31df3e20..d176a981ca8ee 100644 --- a/substrate/frame/message-queue/src/mock.rs +++ b/substrate/frame/message-queue/src/mock.rs @@ -355,8 +355,8 @@ pub fn num_overweight_enqueued_events() -> u32 { .count() as u32 } -pub fn fp(pages: u32, count: u64, size: u64) -> QueueFootprint { - QueueFootprint { storage: Footprint { count, size }, pages } +pub fn fp(pages: u32, ready_pages: u32, count: u64, size: u64) -> QueueFootprint { + QueueFootprint { storage: Footprint { count, size }, pages, ready_pages } } /// A random seed that can be overwritten with `MQ_SEED`. diff --git a/substrate/frame/message-queue/src/tests.rs b/substrate/frame/message-queue/src/tests.rs index 86a8b79fe8bd0..dbef94b3b1035 100644 --- a/substrate/frame/message-queue/src/tests.rs +++ b/substrate/frame/message-queue/src/tests.rs @@ -1064,13 +1064,13 @@ fn footprint_num_pages_works() { MessageQueue::enqueue_message(msg("weight=2"), Here); MessageQueue::enqueue_message(msg("weight=3"), Here); - assert_eq!(MessageQueue::footprint(Here), fp(2, 2, 16)); + assert_eq!(MessageQueue::footprint(Here), fp(2, 2, 2, 16)); // Mark the messages as overweight. assert_eq!(MessageQueue::service_queues(1.into_weight()), 0.into_weight()); assert_eq!(System::events().len(), 2); - // Overweight does not change the footprint. - assert_eq!(MessageQueue::footprint(Here), fp(2, 2, 16)); + // `ready_pages` decreases but `page` count does not. + assert_eq!(MessageQueue::footprint(Here), fp(2, 0, 2, 16)); // Now execute the second message. assert_eq!( @@ -1078,7 +1078,7 @@ fn footprint_num_pages_works() { .unwrap(), 3.into_weight() ); - assert_eq!(MessageQueue::footprint(Here), fp(1, 1, 8)); + assert_eq!(MessageQueue::footprint(Here), fp(1, 0, 1, 8)); // And the first one: assert_eq!( ::execute_overweight(2.into_weight(), (Here, 0, 0)) @@ -1086,6 +1086,11 @@ fn footprint_num_pages_works() { 2.into_weight() ); assert_eq!(MessageQueue::footprint(Here), Default::default()); + assert_eq!(MessageQueue::footprint(Here), fp(0, 0, 0, 0)); + + // `ready_pages` and normal `pages` increases again: + MessageQueue::enqueue_message(msg("weight=3"), Here); + assert_eq!(MessageQueue::footprint(Here), fp(1, 1, 1, 8)); }) } diff --git a/substrate/frame/support/src/traits/messages.rs b/substrate/frame/support/src/traits/messages.rs index 995ac4f717911..f3d893bcc1d89 100644 --- a/substrate/frame/support/src/traits/messages.rs +++ b/substrate/frame/support/src/traits/messages.rs @@ -123,6 +123,8 @@ impl ServiceQueues for NoopServiceQueues { pub struct QueueFootprint { /// The number of pages in the queue (including overweight pages). pub pages: u32, + /// The number of pages that are ready (not yet processed and also not overweight). + pub ready_pages: u32, /// The storage footprint of the queue (including overweight messages). pub storage: Footprint, }