Xushaohong commented on PR #3691: URL: https://github.com/apache/ozone/pull/3691#issuecomment-1231078771
> Thanks for the proto update @Xushaohong. I probably won't have time to review the rest of this PR but that part looks good. Just a question: I see the new proto is in hdds.proto. Would it make more sense to be in SCMAdminProtocol.proto since it is used as the response to an SCM admin command? Hi @errose28, hdds.proto is a common client proto, my motivation for putting the definition here has two reasons: 1. The `DeletedBlocksTransactionInfo` could be used for the future client request, it is a prospective placement. Refer to the proto of `ContainerInfoProto` and `Pipeline`. 2. I think the placement in ScmServerDatanodeHeartbeatProtocol.proto is a historical problem. Originally it is only considered to be used in HB, not in client queries. Such common proto shall be unified and could help reduce confusion for the user and reader. So I wonder if someday we change the reference of this TXN proto from the ScmServerDatanodeHeartbeatProtocol.proto to the hdds.proto. And we could deprecate the old reference. Pls feel free to correct me if i am wrong~ -- 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]
