sodonnel commented on code in PR #4655:
URL: https://github.com/apache/ozone/pull/4655#discussion_r1190014508
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/AbstractContainerReportHandler.java:
##########
@@ -361,6 +361,7 @@ private void updateContainerReplica(final DatanodeDetails
datanodeDetails,
.setKeyCount(replicaProto.getKeyCount())
.setReplicaIndex(replicaProto.getReplicaIndex())
.setBytesUsed(replicaProto.getUsed())
+ .setEmpty(replicaProto.getIsEmpty())
Review Comment:
I wonder if there is a compatibility problem between versions here?
If the DN software is newer than SCM, then things would work as before. That
is fine.
If SCM is upgraded to have this logic, but the DN is on an old version, it
will never send "empty=true". and I think that if the container becomes empty
at this time, it will never set empty=true later. So we could get a scenario
where the container goes empty on the DN, the ICR is sent, but this logic
defaults the isEmpty to false and then the container never gets its remove
triggered by SCM.
I think we need to check if the DN is sending isEmpty(), and if it is not,
put back the old logic here, eg something like:
```
if (replicaProto.hasIsEmpty()) {
builder.setEmpty(replicaProto.getIsEmpty())
} else {
if (replicaProto.getKeyCount() == 0 && replicaProto.getUsed == 0) {
builder.setEmpty(true);
} else {
builder.setEmpty(false);
}
}
```
--
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]
