fapifta commented on a change in pull request #2083:
URL: https://github.com/apache/ozone/pull/2083#discussion_r616028504
##########
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:
If we skip this assertion, then we need som changes later on this method.
The reportStatus logic was designed for one client only, and the plan was to
track all the messages that should be supplied to the client, and when the
client polls the status and reports it, we clean up the messages from the
temporary in memory storage (msgs), if we allow multiple clients to poll the
status then they start to race and steal messages from each other (see 5 lines
down we do msgs.poll()). That is why if a takeover happens the original client
fails due to this assertion. On the other hand if we do not clean the cumulated
messages, then we will serve messages more than once to the original client.
Also if we just allow through a secondary client concurrently, then it will
not be able to read all the messages.
In order for this to work as it was for the original client, and in the
meantime to allow an other client to get the status messages, we will need an
other in memory temporary storage, and keep the messages that are flushed to
the original client in that, and serve the secondary client from that second in
memory storage, so that it will be able to get the full status report. In this
case it is still a question when we clean up these structures? At the end of
the finalization? Some time later? At restart? I am hesitant because of the
memory footprint, but this should not be more than a couple strings maybe a
hundred...
--
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]
