-
Notifications
You must be signed in to change notification settings - Fork 587
HDDS-13594. Use a different endpoint for fetching the OM checkpoint tarball. #8963
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/DBCheckpointServlet.java
Outdated
Show resolved
Hide resolved
errose28
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An older client will always hit the older endpoint and newer client will hit the newer one. This way no layout change is needed.
We currently don't have rolling upgrade support, so the clients in this case would be other OMs or Recon, which would be in the same version. Is the new endpoint still necessary? As long as it still has the option to fetch the whole DB without snapshots for Recon's purposes I think the same endpoint is fine since it is for internal use.
hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java
Outdated
Show resolved
Hide resolved
Yeah it's not very relevant at the moment but might be relevant during a rolling upgrade/partial upgrade like scenarios where a follower OM/Recon has a different version than the leader OM. Also with the newer endpoint the obtained sst files in the tar won't be placed correctly in the snapshot dirs/om.db and will have inodeID based names etc. The client has additional logic to execute for the new endpoint . |
|
Looks good for me. |
jojochuang
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this looks fine. @sadanand48 do you want to update the endpoint name to /v2/dbCheckpoint ? I am okay either way.
...on/src/main/java/org/apache/hadoop/ozone/recon/spi/impl/OzoneManagerServiceProviderImpl.java
Show resolved
Hide resolved
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/DBCheckpointServlet.java
Show resolved
Hide resolved
hadoop-ozone/dist/src/main/smoketest/compatibility/checkpoint.robot
Outdated
Show resolved
Hide resolved
smengcl
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @sadanand48 . lgtm. Just one doc change requested
…robot Co-authored-by: Siyao Meng <[email protected]>
|
Thanks @errose28 @priyeshkaratha @jojochuang @smengcl for the reviews |
…kpoint tarball. (apache#8963)" This reverts commit bc731ea.
…kpoint tarball. (apache#8963)" This reverts commit bc731ea.
…kpoint tarball. (apache#8963)" This reverts commit bc731ea.
What changes were proposed in this pull request?
HDDS-12984 introduced a newer implementation for the OM tarball transfer process but replaces the older endpoint with the new impl. Instead introducing a newer endpoint maybe a better choice as it will handle upgrade related scenarios. An older client will always hit the older endpoint and newer client will hit the newer one. This way no layout change is needed.
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-13594
How was this patch tested?
compat acceptance tests covering: