fix: global queue capacity #225
Conversation
undercover-cactus
force-pushed
the
fix/global-queue-capacity
branch
from
50d9ebd
to
17fca66
Compare
| if !capacity_queue.is_empty() { | ||
| let size: u64 = capacity_queue.drain(..).sum(); | ||
|
|
||
| let new_capacity = self.calculate_capacity(size, current_capacity)?; |
There was a problem hiding this comment.
I couldn't find a good way to test this, especially for the max storage value because it would require a 4GB file and it is not practical.
A way this function could be tested is to pass the config as an argument and have a test in rust to make sure the calculation is right and that it will goes higher than the max storage authorize.
There was a problem hiding this comment.
You could pass these aditional arguments in your test and reduce it to a reasonable size.
storage-hub/test/suites/integration/bsp/onboard.test.ts
Lines 22 to 25 in 64cc252
| @@ -69,7 +69,7 @@ export const getContainerIp = async (containerName: string, verbose = false): Pr | |||
|
|
|||
| export const checkNodeAlive = async (url: string, verbose = false) => getContainerIp(url, verbose); | |||
| export const getContainerPeerId = async (url: string, verbose = false) => { | |||
| const maxRetries = 60; | |||
| const maxRetries = 120; | |||
There was a problem hiding this comment.
For my info, why the increase?
| .capacityUsed.toNumber(); | ||
| await userApi.block.skipToMinChangeTime(); | ||
| const minCapacity = userApi.consts.providers.spMinCapacity.toNumber(); | ||
| const newCapacity = Math.max(minCapacity, capacityUsed + 1); |
There was a problem hiding this comment.
Why capacityUsed + 1 and not just capacityUsed?
| )) | ||
| .watch_for_success(&self.storage_hub_handler.blockchain) | ||
| .await?; | ||
| // wwe read from the queue |
There was a problem hiding this comment.
| // wwe read from the queue | |
| // we read from the queue |
| .watch_for_success(&self.storage_hub_handler.blockchain) | ||
| .await?; | ||
|
|
||
| drop(capacity_queue); |
There was a problem hiding this comment.
Why drop the lock here and not outside?
| &mut self, | ||
| event: &NewStorageRequest, | ||
| &self, | ||
| size: StorageDataUnit, |
There was a problem hiding this comment.
What do you think of renaming this to required_additional_capacity?
| @@ -53,6 +56,7 @@ where | |||
| { | |||
| storage_hub_handler: StorageHubHandler<FL, FSH>, | |||
| file_key_cleanup: Option<H256>, | |||
| capacity_queue: Arc<Mutex<Vec<u64>>>, | |||
There was a problem hiding this comment.
I might be missing something, but as far as I understand this could be just a u64 instead of Vec<u64> - seems this is the way we are using it.
No description provided.