wuchong commented on PR #1477: URL: https://github.com/apache/fluss/pull/1477#issuecomment-3265773525
Sorry, I do have some concerns about this pull request. I left the comments below: 1. Why moving it to background can improve the ZK performance? Doesn't the background execution also require recursively deleting all the children for the partition assignment? Could you share the benchmark result before and after using the background? 2. I think this changes the semantic of `deletePartitionAssignment` which means all the assignment has been deleted after the method returns. This may introduce some unexpected behavior when using this method. For example: `TableManager#completeDeletePartition` removes the partition from `coordinatorContext` only when the assignement has been deleted in zk. But this method becomes async, and it will remove the partition from `coordinatorContext` before all the assignment is deleted from zk. 3. We have a lot of `deletingChildrenIfNeeded` in `ZooKeeperClient`, should we also update other methods as well? Otherwise, it introduces in-consistent in the code. This makes future development confusing that which way is suggested to use. -- 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]
