guihecheng commented on a change in pull request #2083:
URL: https://github.com/apache/ozone/pull/2083#discussion_r616301782
##########
File path:
hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/upgrade/BasicUpgradeFinalizer.java
##########
@@ -145,12 +145,14 @@ public StatusAndMessages finalize(String upgradeClientID,
T id)
@Override
public synchronized StatusAndMessages reportStatus(
- String upgradeClientID, boolean takeover
+ String upgradeClientID, boolean takeover, boolean readonly
) throws UpgradeException {
if (takeover) {
clientID = upgradeClientID;
}
- assertClientId(upgradeClientID);
+ if (!readonly) {
+ assertClientId(upgradeClientID);
Review comment:
@fapifta thanks for your comments, there is a problem indeed.
IMHO, the 'finalizationStatus' command just returns the status like a single
word 'FINALIZATION_IN_PROGRESS' shall be enough, because we may need this
command to do some scripting/autotesting work, or just give user a simple hint
on the current status.
What about having 2 methods:
- reportStatus(void): just return the status which we can directly get from
versionManager.
- reportStatusAndMsgs(String upgradeClientID, boolean takeover): the
original 'reportStatus' method without any changes, also without the readonly
flag we added here.
Then the 'finalizationStatus' command will use reportStatus, which is just
enough.
And the 'finalizeUpgrade' command use reportStatusAndMsgs, the takeover
behaviour remains untouched.
In this way, we don't bother handling the concurrent issue for the msgs
queue.
Do you think it is workable?
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
