Skip to content

HDDS-7553. Ozone OM stopped with iIllegalStateException.#5222

Merged
szetszwo merged 6 commits intoapache:masterfrom
aryangupta1998:HDDS-7553
Aug 29, 2023
Merged

HDDS-7553. Ozone OM stopped with iIllegalStateException.#5222
szetszwo merged 6 commits intoapache:masterfrom
aryangupta1998:HDDS-7553

Conversation

@aryangupta1998
Copy link
Contributor

What changes were proposed in this pull request?

Currently, we don't support om service id change but if someone changes om service id then we throw a warning message which results in the creation of a new raft group directory with respect to the new om service id and finally leads to iIllegalStateException. This PR aims to prevent the om service id change by throwing an exception, with this PR we also prevent the existence of SCM and OM ratis storage directory at the same path.

What is the link to the Apache JIRA

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

How was this patch tested?

Tested Manually.

szetszwo
szetszwo previously approved these changes Aug 25, 2023
Copy link
Contributor

@szetszwo szetszwo left a comment

Choose a reason for hiding this comment

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

@aryangupta1998 , thanks for working on this! The idea looks good. However, there are many test failures. When the storage directory is not set in both SCM and OM, they will use the same directory. Then OM will fail when it restarts after this change.

@szetszwo szetszwo self-requested a review August 25, 2023 17:52
@szetszwo szetszwo dismissed their stale review August 25, 2023 17:52

Need to fix test failures

Copy link
Contributor

@sumitagrawl sumitagrawl left a comment

Choose a reason for hiding this comment

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

@aryangupta1998 LGTM +1

Copy link
Contributor

@sadanand48 sadanand48 left a comment

Choose a reason for hiding this comment

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

Thanks @aryangupta1998 for addressing the comments, LGTM

Copy link
Contributor

@szetszwo szetszwo left a comment

Choose a reason for hiding this comment

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

+1 the change looks good.

@szetszwo szetszwo merged commit 827ee4e into apache:master Aug 29, 2023
@szetszwo
Copy link
Contributor

@sadanand48 , @sumitagrawl thanks a lot for reviewing this!

jojochuang pushed a commit to jojochuang/ozone that referenced this pull request Feb 1, 2024
…pache#5222)

(cherry picked from commit 827ee4e)
Change-Id: I62c3d3de6f2ba528dd9a0283f53662a66e3a6e3c
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.

4 participants