smengcl commented on a change in pull request #806: HDDS-3374.
OMVolumeSetOwnerRequest doesn't check if user is already the owner
URL: https://github.com/apache/hadoop-ozone/pull/806#discussion_r408251049
##########
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/volume/OMVolumeSetOwnerResponse.java
##########
@@ -55,11 +56,27 @@ public OMVolumeSetOwnerResponse(@Nonnull OMResponse
omResponse,
/**
* For when the request is not successful or it is a replay transaction.
Review comment:
I believe the comment is still accurate. There are four places that use this
constructor at the moment:
- INVALID_REQUEST
```java
// In production this will never happen, this request will be called only
// when we have ownerName in setVolumePropertyRequest.
if (!setVolumePropertyRequest.hasOwnerName()) {
omResponse.setStatus(OzoneManagerProtocolProtos.Status.INVALID_REQUEST)
.setSuccess(false);
return new OMVolumeSetOwnerResponse(omResponse.build());
}
```
- Replay
```java
// Check if this transaction is a replay of ratis logs.
// If this is a replay, then the response has already been returned to
// the client. So take no further action and return a dummy
// OMClientResponse.
if (isReplay(ozoneManager, omVolumeArgs, transactionLogIndex)) {
LOG.debug("Replayed Transaction {} ignored. Request: {}",
transactionLogIndex, setVolumePropertyRequest);
return new
OMVolumeSetOwnerResponse(createReplayOMResponse(omResponse));
}
```
- newOwner is the same as oldOwner (added in this PR)
```java
// Return OK immediately if newOwner is the same as oldOwner.
if (oldOwner.equals(newOwner)) {
LOG.warn("Volume '{}' owner is already user '{}'.", volume,
oldOwner);
omResponse.setStatus(OzoneManagerProtocolProtos.Status.OK)
.setMessage(
"Volume '" + volume + "' owner is already '" + newOwner + "'.")
.setSuccess(false);
return new OMVolumeSetOwnerResponse(omResponse.build());
}
```
- IOException
```java
} catch (IOException ex) {
exception = ex;
omClientResponse = new OMVolumeSetOwnerResponse(
createErrorOMResponse(omResponse, exception));
} finally {
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
