neils-dev commented on code in PR #3382:
URL: https://github.com/apache/ozone/pull/3382#discussion_r877482887
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OMInterServiceProtocolServerSideImpl.java:
##########
@@ -60,7 +64,7 @@ public BootstrapOMResponse bootstrap(RpcController controller,
.build();
}
- checkLeaderStatus();
+ OzoneManagerRatisUtils.checkLeaderStatus(ozoneManager);
Review Comment:
Hi @ChenSammi, thanks for your updated comment on handling the leader
exceptions.
> All protocol service API need throws ServiceException, while
ozoneManager.checkLeaderStatus throws OMNotLeaderException and
OMLeaderNotReadyException. So we need OzoneManagerRatisUtils.checkLeaderStatus
wrapper to covert exceptions.
Having that, the
`OzoneManagerProtocolServerServerSideTranslatorPB.processRequest` needs to
throw a `ServiceException` should `isRatisEnabled == true` and it is _not_ a
leader. There looks to be two cases where this happens in `processRequest`:
1. For the case that `isRatisEnabled == true` and `s3Auth == false`, that's
what happens with your leader check `OzoneManagerRatisUtils.checkLeaderStatus`.
It is handled.
2. For the case that `isRatisEnabled == true` and
`request.hasS3Authentication == true` it needs to be handled so that the
`ServiceException` is thrown in the event of a leadership check exception.
This looks to be currently wrapping the leadership exception in an IOException
(`InvalidToken`) and returned to the caller through the error `OmResponse`.
Does that look to be the case? If so, instead it needs to throw a leader
exception wrapped in a `ServiceException` for the caller. If we are not
throwing the `ServiceException`, would you like to insert the
`OzoneManagerRatisUtils.checkLeaderStatus` to do so? Or have the `IOException`
handler for `processRequest` (catch block) wrap the extracted` Leader
exception` into a `ServiceException` and throw?
--
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]
