sodonnel commented on PR #10109: URL: https://github.com/apache/ozone/pull/10109#issuecomment-4336761146
This change looks good and I think it will solve the problem. However I think it reveals a problem with the code structure within the datanode. These services should not have to perform such complex locking to get a consistent view of a container and it would be very easy in the future for some other service to come along and not do things correctly, or indeed some other existing part of the DN code may also be doing things incorrectly already. I think that all the container "logic" should be hidden behind an interface and all the services like disk balancer, or block deletion should call it, eg: ``` containerManager.moveContainer(container_id, destination) containerManager.deleteBlockList(container_id, blocksToDelete) ... etc ``` Then all the locking etc happens behind the scenes in the same place where it can be controlled more closely. Fixing this is a large exercise and not something we would want to take on in this PR. There are other places in KeyValueHandler that take the container lock - are we sure they are OK or is any change needed in them too? -- 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]
