Skip to content

HDDS-7578. Wrong directory config used for SCM HA Ratis Snapshot#4030

Merged
adoroszlai merged 1 commit intoapache:masterfrom
symious:HDDS-7578
Feb 5, 2023
Merged

HDDS-7578. Wrong directory config used for SCM HA Ratis Snapshot#4030
adoroszlai merged 1 commit intoapache:masterfrom
symious:HDDS-7578

Conversation

@symious
Copy link
Contributor

@symious symious commented Dec 2, 2022

What changes were proposed in this pull request?

The configuration of "OZONE_SCM_HA_RATIS_SNAPSHOT_DIR" is not correctly used as the directory of ratis snapshot.

What is the link to the Apache JIRA

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

How was this patch tested?

unit test.

@symious symious requested a review from bshashikant December 2, 2022 14:39
@kerneltime
Copy link
Contributor

@xBis7 @sumitagrawl can you please take a look ?

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.

@symious Thanks working on this, there is a minor question, please check

public static String getSCMRatisSnapshotDirectory(ConfigurationSource conf) {
String snapshotDir =
conf.get(ScmConfigKeys.OZONE_SCM_HA_RATIS_STORAGE_DIR);
conf.get(ScmConfigKeys.OZONE_SCM_HA_RATIS_SNAPSHOT_DIR);
Copy link
Contributor

Choose a reason for hiding this comment

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

Do this have any impact over upgrade? since snapshot directory path changed for user configured case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sumitagrawl Thank you for the review.

I think this is a typo issue. A different place to store snapshot shouldn't cause difference.

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean,
Before upgrade, snapshot is stored at OZONE_SCM_HA_RATIS_STORAGE_DIR and snapshots are created in this path.
After upgrade, its changed to OZONE_SCM_HA_RATIS_SNAPSHOT_DIR.
In this case on startup, if ozone try to recover snapshots, will this cause any failure or impact to the system ?

Copy link
Contributor

@ChenSammi ChenSammi Jan 12, 2023

Choose a reason for hiding this comment

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

I think this snapshot directory acts like a tmp directory. Every file downloaded from leader SCM to this directory, will be used to replace current SCM's rocksdb. So basically change it from OZONE_SCM_HA_RATIS_STORAGE_DIR to OZONE_SCM_HA_RATIS_SNAPSHOT_DIR will not cause any compatibility issue, since the files left in this snapshot directory are not useful anymore after a service upgrade restart.

Hi @nandakumar131, please correct me if I'm wrong.

@adoroszlai adoroszlai changed the title HDDS-7578. Correct usage of SCM snapshot directory HDDS-7578. Wrong directory config used for SCM HA Ratis Snapshot Feb 5, 2023
@adoroszlai adoroszlai merged commit 940d678 into apache:master Feb 5, 2023
@adoroszlai
Copy link
Contributor

Thanks @symious for the fix, @ChenSammi, @ferhui, @sumitagrawl, @xBis7 for the review.

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.

7 participants