From 26dad11f7bdcf4482394f892659bb4c0f5e4f724 Mon Sep 17 00:00:00 2001 From: wangjj9219 <183318287@qq.com> Date: Wed, 10 Jun 2020 21:43:54 +0800 Subject: [PATCH 1/3] add extend_lock for StorageLock --- .../runtime/src/offchain/storage_lock.rs | 72 +++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/primitives/runtime/src/offchain/storage_lock.rs b/primitives/runtime/src/offchain/storage_lock.rs index f8defa422459f..67450eac17556 100644 --- a/primitives/runtime/src/offchain/storage_lock.rs +++ b/primitives/runtime/src/offchain/storage_lock.rs @@ -264,6 +264,24 @@ impl<'a, L: Lockable> StorageLock<'a, L> { } } + /// Extend ative lock's deadline + fn extend_active_lock(&mut self) -> Result<::Deadline, ()> { + let res = self.value_ref.mutate(|s: Option>| -> Result<::Deadline, ()> { + match s { + // lock is present and is still active, extend the lock. + Some(Some(deadline)) if !::has_expired(&deadline) => + Ok(self.lockable.deadline()), + // other cases + _ => Err(()), + } + }); + match res { + Ok(Ok(deadline)) => Ok(deadline), + Ok(Err(_)) => Err(()), + Err(e) => Err(e), + } + } + /// Internal lock helper to avoid lifetime conflicts. fn try_lock_inner( &mut self, @@ -337,6 +355,17 @@ impl<'a, 'b, L: Lockable> StorageLockGuard<'a, 'b, L> { pub fn forget(mut self) { let _ = self.lock.take(); } + + /// Extend the lock by guard if the lock is existed. + /// + /// Can be used to extend deadline when doing iterations + /// that it's hard to be estimated consumed time. After + /// a successful step, extend the lock if the lock is still active. + pub fn extend_lock(&mut self) { + if let Some(ref mut lock) = self.lock { + let _ = lock.extend_active_lock(); + } + } } impl<'a, 'b, L: Lockable> Drop for StorageLockGuard<'a, 'b, L> { @@ -512,4 +541,47 @@ mod tests { let opt = state.read().persistent_storage.get(b"", b"lock_3"); assert!(opt.is_some()); } + + #[test] + fn extend_active_lock() { + let (offchain, state) = testing::TestOffchainExt::new(); + let mut t = TestExternalities::default(); + t.register_extension(OffchainExt::new(offchain)); + + t.execute_with(|| { + let lock_expiration = Duration::from_millis(300); + + let mut lock = StorageLock::<'_, Time>::with_deadline(b"lock_4", lock_expiration); + let mut guard = lock.lock(); + + // sleep_until < lock_expiration + offchain::sleep_until(offchain::timestamp().add(Duration::from_millis(200))); + + // the lock is still active, extend it successfully + guard.extend_lock(); + guard.forget(); + + // sleep_until < deadline + offchain::sleep_until(offchain::timestamp().add(Duration::from_millis(200))); + + // the lock is still active, try_lock will fail + let mut lock = StorageLock::<'_, Time>::new(b"lock_4"); + let res = lock.try_lock(); + assert_eq!(res.is_ok(), false); + + // sleep_until > deadline + offchain::sleep_until(offchain::timestamp().add(Duration::from_millis(200))); + + // the lock has expired, try_lock will succeed + let mut lock = StorageLock::<'_, Time>::new(b"lock_4"); + let res = lock.try_lock(); + assert!(res.is_ok()); + let guard = res.unwrap(); + guard.forget(); + }); + + // lock must have been cleared at this point + let opt = state.read().persistent_storage.get(b"", b"lock_4"); + assert!(opt.is_some()); + } } From 0ace8fde8f3c6267a8295d2351b11c85f0655b05 Mon Sep 17 00:00:00 2001 From: wangjj9219 <183318287@qq.com> Date: Sat, 13 Jun 2020 18:26:20 +0800 Subject: [PATCH 2/3] changes --- .../runtime/src/offchain/storage_lock.rs | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/primitives/runtime/src/offchain/storage_lock.rs b/primitives/runtime/src/offchain/storage_lock.rs index 67450eac17556..987dbe38ba915 100644 --- a/primitives/runtime/src/offchain/storage_lock.rs +++ b/primitives/runtime/src/offchain/storage_lock.rs @@ -264,7 +264,7 @@ impl<'a, L: Lockable> StorageLock<'a, L> { } } - /// Extend ative lock's deadline + /// Extend active lock's deadline fn extend_active_lock(&mut self) -> Result<::Deadline, ()> { let res = self.value_ref.mutate(|s: Option>| -> Result<::Deadline, ()> { match s { @@ -356,14 +356,16 @@ impl<'a, 'b, L: Lockable> StorageLockGuard<'a, 'b, L> { let _ = self.lock.take(); } - /// Extend the lock by guard if the lock is existed. + /// Extend the lock by guard deadline if it already exists. /// - /// Can be used to extend deadline when doing iterations - /// that it's hard to be estimated consumed time. After - /// a successful step, extend the lock if the lock is still active. - pub fn extend_lock(&mut self) { + /// i.e. large sets of items for which it is hard to calculate a + /// meaning full conservative deadline which does not block for a + /// very long time on node termination. + pub fn extend_lock(&mut self) -> Result<::Deadline, ()> { if let Some(ref mut lock) = self.lock { - let _ = lock.extend_active_lock(); + lock.extend_active_lock() + } else { + Err(()) } } } @@ -558,7 +560,7 @@ mod tests { offchain::sleep_until(offchain::timestamp().add(Duration::from_millis(200))); // the lock is still active, extend it successfully - guard.extend_lock(); + assert_eq!(guard.extend_lock().is_ok(), true); guard.forget(); // sleep_until < deadline From 9c9d5d3c7833939b78fe754c11f3b548bd657220 Mon Sep 17 00:00:00 2001 From: wangjj9219 <183318287@qq.com> Date: Mon, 15 Jun 2020 11:36:16 +0800 Subject: [PATCH 3/3] changes --- .../runtime/src/offchain/storage_lock.rs | 44 ++++++++++--------- 1 file changed, 24 insertions(+), 20 deletions(-) diff --git a/primitives/runtime/src/offchain/storage_lock.rs b/primitives/runtime/src/offchain/storage_lock.rs index 987dbe38ba915..4718d2e3ddea1 100644 --- a/primitives/runtime/src/offchain/storage_lock.rs +++ b/primitives/runtime/src/offchain/storage_lock.rs @@ -560,30 +560,34 @@ mod tests { offchain::sleep_until(offchain::timestamp().add(Duration::from_millis(200))); // the lock is still active, extend it successfully - assert_eq!(guard.extend_lock().is_ok(), true); - guard.forget(); + assert_eq!(guard.extend_lock().is_ok(), true); - // sleep_until < deadline - offchain::sleep_until(offchain::timestamp().add(Duration::from_millis(200))); + // sleep_until < deadline + offchain::sleep_until(offchain::timestamp().add(Duration::from_millis(200))); - // the lock is still active, try_lock will fail - let mut lock = StorageLock::<'_, Time>::new(b"lock_4"); - let res = lock.try_lock(); - assert_eq!(res.is_ok(), false); + // the lock is still active, try_lock will fail + let mut lock = StorageLock::<'_, Time>::with_deadline(b"lock_4", lock_expiration); + let res = lock.try_lock(); + assert_eq!(res.is_ok(), false); - // sleep_until > deadline - offchain::sleep_until(offchain::timestamp().add(Duration::from_millis(200))); + // sleep again untill sleep_until > deadline + offchain::sleep_until(offchain::timestamp().add(Duration::from_millis(200))); - // the lock has expired, try_lock will succeed - let mut lock = StorageLock::<'_, Time>::new(b"lock_4"); - let res = lock.try_lock(); - assert!(res.is_ok()); - let guard = res.unwrap(); - guard.forget(); - }); + // the lock has expired, failed to extend it + assert_eq!(guard.extend_lock().is_ok(), false); + guard.forget(); - // lock must have been cleared at this point - let opt = state.read().persistent_storage.get(b"", b"lock_4"); - assert!(opt.is_some()); + // try_lock will succeed + let mut lock = StorageLock::<'_, Time>::with_deadline(b"lock_4", lock_expiration); + let res = lock.try_lock(); + assert!(res.is_ok()); + let guard = res.unwrap(); + + guard.forget(); + }); + + // lock must have been cleared at this point + let opt = state.read().persistent_storage.get(b"", b"lock_4"); + assert_eq!(opt.unwrap(), vec![132_u8, 3u8, 0, 0, 0, 0, 0, 0]); // 132 + 256 * 3 = 900 } }