Skip to content

HDDS-10036. OzoneManagerRatisServer.getServer() should return Division.#5892

Merged
adoroszlai merged 2 commits intoapache:masterfrom
szetszwo:HDDS-10036
Jan 1, 2024
Merged

HDDS-10036. OzoneManagerRatisServer.getServer() should return Division.#5892
adoroszlai merged 2 commits intoapache:masterfrom
szetszwo:HDDS-10036

Conversation

@szetszwo
Copy link
Contributor

What changes were proposed in this pull request?

OzoneManagerRatisServer has a specific raftGroup. The getServer() method should return the division specific to that group. Otherwise, callers have to get groupId and then get the division with try-catch.

What is the link to the Apache JIRA

HDDS-10036

How was this patch tested?

By updating existing tests.

.next().getUuid().toString();
final RaftServer.Division server = om.getOmRatisServer().getServerDivision();
final String ratisDir = server.getRaftServer().getProperties().get("raft.server.storage.dir");
final String groupIdDirName = server.getGroup().getGroupId().toString();
Copy link
Contributor

Choose a reason for hiding this comment

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

TestOzoneManagerPrepare is failing due to mismatch in the directory name.

Suggested change
final String groupIdDirName = server.getGroup().getGroupId().toString();
final String groupIdDirName = server.getGroup().getGroupId().getUuid().toString();

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is a good catch! Thanks for the tip.

@szetszwo szetszwo requested a review from adoroszlai December 31, 2023 18:26
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 @szetszwo for the improvement.

@adoroszlai adoroszlai merged commit 14e7ff1 into apache:master Jan 1, 2024
@szetszwo
Copy link
Contributor Author

szetszwo commented Jan 1, 2024

@adoroszlai , thanks a lot for reviewing and merging this!

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.

2 participants