Skip to content

Conversation

@tanvipenumudy
Copy link
Contributor

@tanvipenumudy tanvipenumudy commented Mar 1, 2024

What changes were proposed in this pull request?

OM has a background thread running that periodically refetches the updated network topology layout information from SCM for the refresh duration frequency of every one hour (default).

This change introduces an OM admin CLI for force fetching the network topology tree information from SCM (on demand) without having to rely on the configuration: ozone.om.network.topology.refresh.duration.

CLI usage:

ozone admin om fetch-topology-tree -id=omServiceId

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-10445

How was this patch tested?

@tanvipenumudy
Copy link
Contributor Author

@kerneltime, @duongkame could you please take a look? Thanks!

@ArafatKhan2198
Copy link
Contributor

Thanks for working on this, @tanvipenumudy. I don't see any tests written for it yet. Would you mind adding an integration test to verify your results, please?

@tanvipenumudy tanvipenumudy marked this pull request as ready for review April 3, 2024 08:13
@tanvipenumudy
Copy link
Contributor Author

@adoroszlai could you please review the changes as well? Thanks!

Copy link
Contributor

@adoroszlai adoroszlai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @tanvipenumudy for working on this.

The new command seems to work only with OM HA. I don't think this functionality should be limited to such setup.

scm = cluster.getStorageContainerManager();
om = cluster.getOzoneManager();
nodeManager = (SCMNodeManager) scm.getScmNodeManager();
client = cluster.newClient();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

client needs to be closed in shutdown().

ListStatusLight = 129;
GetSnapshotInfo = 130;
RenameSnapshot = 131;
RefetchNetworkTopologyTree = 132;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's omit Tree to make it a bit shorter. (Also in request/response, method and command names.)

Comment on lines +54 to +61
System.err.println("Force fetching network topology tree information " +
"has failed: " + e.getMessage());
}
if (status) {
System.out.println(
"Force fetching network topology tree information " +
"is complete.");
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find this output a bit too verbose.


@Override
public boolean refetchNetworkTopologyTree() {
return scmTopologyClient.refetchClusterTree(configuration);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think admin privilege should be required to make the request.

…/om/FetchNetworkTopologyTreeSubCommand.java

Co-authored-by: Doroszlai, Attila <[email protected]>
@tanvipenumudy tanvipenumudy marked this pull request as draft April 22, 2024 05:33
@adoroszlai
Copy link
Contributor

@tanvipenumudy can we close this?

@tanvipenumudy
Copy link
Contributor Author

Sure, marking the PR as closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants