devmadhuu commented on PR #10928: URL: https://github.com/apache/ozone/pull/10928#issuecomment-5165990517
> Note: While reviewing the allocation path, I noticed one related point and would like to check whether this is already tracked somewhere. > > `recordPendingAllocation(...)` is defined on `PipelineManager` / `PipelineManagerImpl`, but I could not find a caller on the `HDDS-11233` branch. From what I found, container allocation calls `hasEnoughSpace(...)` from `ContainerManagerImpl`, but I do not see a matching `recordPendingAllocation(...)` call after successful allocation. > > I may be missing some context here. If this is not already covered by another task, would it make sense to track it as a follow-up? I’m happy to help with it as well. Your observation seems correct and needs to handle now. I think this will be more acute for storageType aware tracking, though before this PR, it was fine because pending count gets removed automatically after rolling window of max 10 mins and without storageType, this conservative approach seems fine as keeping pending count in tracker will rather avoid over-allocating than wait for ageing timeout based removal. But now with storageType aware tracker, the pending count is now precise per type. That means the double-counting problem is more acute — a single confirmed SSD container can block all SSD allocation on that datanode for up to 10 minutes, even though the SSD space is correctly reflected in the heartbeat. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
