sadanand48 commented on code in PR #3411:
URL: https://github.com/apache/ozone/pull/3411#discussion_r880607063
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashOzoneFileSystem.java:
##########
@@ -577,6 +580,7 @@ boolean processKeyPath(List<String> keyPathList) {
try {
omRequest = OzoneManagerProtocolProtos.OMRequest.newBuilder()
.setClientId(CLIENT_ID.toString())
+ .setVersion(ClientVersion.CURRENT_VERSION)
Review Comment:
> Can we refactor this class to use a common request builder method like the
client side uses in OzoneManagerProtocolClientSideTranslatorPB#createOMRequest?
This way we only need to specify common fields like client ID, user info, and
client version in one place.
Thanks @errose28 for the suggestion. Will open a separate jira for this.
> Also while looking at this code, I do not think
TrashOzoneFileSystem#submitRequest should be manually invoking preExecute. The
call should go to OzoneManagerProtocolServerSideTranslatorPB#processRequest
which will invoke preExecute, so it looks like the current implementation is
calling preExecute twice.
It is not calling preExecute twice in the current implementation . It will
call `OzoneManagerProtocolServerSideTranslatorPB#submitRequest` if
`ozoneManager.isRatisEnabled()` is false. If true ,it will call preExecute and
call `OzoneManagerRatisServer#submitRequest`. We did it this way to avoid
creating an RPC since the filesystem is in the OM server itself. I recall
there was a [test failure](https://github.com/apache/ozone/pull/1818) when we
tried to call `OzoneManagerRatisServer#submitRequest`.
--
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]
