errose28 commented on code in PR #4655:
URL: https://github.com/apache/ozone/pull/4655#discussion_r1212427230
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/AbstractContainerReportHandler.java:
##########
@@ -370,6 +384,26 @@ private void updateContainerReplica(final DatanodeDetails
datanodeDetails,
}
}
+ /**
+ * Determines whether container replica is empty or not.
+ *
+ * @param replicaProto container replica details.
+ * @return true if container replica is empty else false
+ */
+ private boolean fillEmpty(final ContainerReplicaProto replicaProto) {
+ if (replicaProto.hasIsEmpty()) {
+ return replicaProto.getIsEmpty();
+ } else {
+ // Handled when DN version is old then there will not be isEmpty field.
+ // In this case judge container empty based on keyCount
+ // and bytesUsed field.
+ if (replicaProto.getKeyCount() == 0 && replicaProto.getUsed() == 0) {
+ return true;
+ }
Review Comment:
Compatibility guarantees have to be strictly defined and tested. For
example, we support non-rolling upgrades from all versions since 1.0.0 and
non-roling downgrades pre-finalization to all versions since 1.1.0. We have
tests for this and require changes to adhere to these rules. The compatibility
code you are referring to is to comply to these guidelines.
Rolling upgrade has not been planned yet. When it is, we will define an
earliest version that supports rolling upgrade, tests, and requirements that
developers must follow, just like we have for non-rolling upgrade. Without
this, enforcement of rolling upgrade compatibility will be arbitrary. We cannot
attempt to support the feature in this state. There are also issues that
guarantee no code getting checked in to master right now will be eligible for
rolling upgrade support:
- OM HA commit model is incompatible with rolling upgrades. There may be
other areas of the code as well that currently prevent us from doing rolling
upgrades from this version, but we have not done an invovled investigation yet.
- There are no test suites to catch any rolling upgrade incompatibilities
that are introduced, so it is impossible to guarantee the current version would
work with a rolling upgrade. If it happend to work it would be purely by
coincidence.
- There is no agreement among developers on rolling upgrade version
guarantees so requirements are not being enforced in code reviews.
Once the rolling upgrade feature is proposed and requirements are defined
then we will have explicit rules to guide changes and versions. Without this
framework in place, enforcing rolling upgrade requirements in PRs just leads to
confusing code, because it is documenting and designing for a situation that
cannot happen. For that reason I propose removing the extra check, even though
it does not affect correctness in the current version.
--
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]
