sodonnel commented on code in PR #3644:
URL: https://github.com/apache/ozone/pull/3644#discussion_r936000237
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/statemachine/background/BlockDeletingService.java:
##########
@@ -198,8 +198,15 @@ private boolean isDeletionAllowed(ContainerData
containerData,
if (ozoneContainer.getWriteChannel() instanceof XceiverServerRatis) {
XceiverServerRatis ratisServer =
(XceiverServerRatis) ozoneContainer.getWriteChannel();
- PipelineID pipelineID = PipelineID
- .valueOf(UUID.fromString(containerData.getOriginPipelineId()));
+ PipelineID pipelineID;
Review Comment:
After some debugging, the issue is that
`containerData.getOriginPipelineId()` is an empty string for EC Containers. I
have not been able to track down how the pipelineID gets added to the
`writeChunk` call for Ratis requests - I tried to find it in the debugger, but
I suspect it happens inside the Xceiver Ratis code. Either way, I did see that
the writeChunk command, which creates the container on the first write has a
pipelineID set for Ratis writes, but not for EC writes. This kind of makes
sense, as EC writes don't really care about the pipeline on a DN - they are all
standalone writes.
The code here at line 198, will always end up returning true, even if its
not a Ratis write. I ran into this before trying to do something else on the DN
where I was trying to detect an EC or Ratis container.
All the logic inside this IF branch is related Ratis stuff - whether the
pipeline still exists or not.
Would we be better adding a new IF branch, something like:
```
else if (containerData.getOriginPipelineId == emptyString or Null) {
// This is not a ratis container so allow the delete
return true;
} else {
// existing code as it is
}
```
This would avoid the ugly exception handling, and I've been told catching
exceptions like this is Java is relatively slow (ie avoid using exception
handling for common cases), so if there are a lot of block deletes for EC, it
may cause a performance problem as this exception will always get thrown.
--
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]
