Skip to content

HDDS-8208. [SNAPSHOT] Allow om snapshot bootstrap tarballs to be incremental. - #4770

Merged
smengcl merged 21 commits into
apache:masterfrom
GeorgeJahad:HDDS-8208
Jun 16, 2023
Merged

HDDS-8208. [SNAPSHOT] Allow om snapshot bootstrap tarballs to be incremental.#4770
smengcl merged 21 commits into
apache:masterfrom
GeorgeJahad:HDDS-8208

Conversation

@GeorgeJahad

@GeorgeJahadGeorgeJahad commented May 24, 2023•
edited
Loading

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

The incremental bootstrapping feature, currently in master, only treats the active fs sst files incrementally. This PR extends that functionality to the om snapshot related sst files.

The feature is currently implemented with an exclude list. The follower generates a list of the active sst files it has currently received from the leader, and passes that list to the leader as part of the next http ratis snapshot request. The leader then excludes those sst files from next tarball it sends to the follower.

In this PR, the follower adds the existing om snapshot sst files to the exclude list. The leader than uses that list to determine which snapshot sst files to exclude as well as which can be used as targets for hard links.

What is the link to the Apache JIRA

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

How was this patch tested?

Unit/Integration tests added

@GeorgeJahadGeorgeJahad changed the title HDDS_8208. Handle large tarballs in bootstrapping.HDDS_8208. [SNAPSHOT] Handle large tarballs in bootstrapping.May 24, 2023
@GeorgeJahadGeorgeJahad changed the title HDDS_8208. [SNAPSHOT] Handle large tarballs in bootstrapping.HDDS_8208. [SNAPSHOT] Allow om snapshot bootstrap tarballs to be incrementalMay 24, 2023
@GeorgeJahadGeorgeJahad changed the title HDDS_8208. [SNAPSHOT] Allow om snapshot bootstrap tarballs to be incrementalHDDS_8208. [SNAPSHOT] Allow om snapshot bootstrap tarballs to be incremental.May 24, 2023
@adoroszlaiadoroszlai added the snapshot https://issues.apache.org/jira/browse/HDDS-6517 label May 25, 2023
@GeorgeJahadGeorgeJahad changed the title HDDS_8208. [SNAPSHOT] Allow om snapshot bootstrap tarballs to be incremental.HDDS-8208. [SNAPSHOT] Allow om snapshot bootstrap tarballs to be incremental.May 25, 2023
@smengclsmengcl added the gr label May 25, 2023
smengcl
smengcl reviewed Jun 15, 2023
View reviewed changes

@smengclsmengcl left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @GeorgeJahad for working on this. Overall lgtm. Some comments inline, mostly nits.

Comment threadhadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/utils/TestRDBSnapshotProvider.java Outdated
Comment threadhadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/RDBSnapshotProvider.java Outdated
Comment threadhadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/RDBSnapshotProvider.java
Comment threadhadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java
public static final TimeDuration
OZONE_OM_SNAPSHOT_PROVIDER_REQUEST_TIMEOUT_DEFAULT =
TimeDuration.valueOf(5000, TimeUnit.MILLISECONDS);
TimeDuration.valueOf(300000, TimeUnit.MILLISECONDS);

@smengclsmengclJun 15, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

5s timeout to 5m is quite a jump. If this default config value revision is intended (i.e. not just for debugging), pls change ozone-default.xml correspondingly:

https://github.com/GeorgeJahad/ozone/blob/f758ab2cd96d92d2d73d51286bdd3c87840bceec/hadoop-hdds/common/src/main/resources/ozone-default.xml#L2008-L2010

Comment thread...one/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOmSnapshotUtils.java
Comment on lines +77 to +78
assertEquals(tree1Files, tree2Files);
}

@smengclsmengclJun 15, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: clean it up by recursively deleting tempDir at the end?

Comment threadhadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java Outdated
Comment threadhadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OMDBCheckpointServlet.java Outdated
Comment threadhadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OMDBCheckpointServlet.java Outdated
GeorgeJahadand others added 2 commits June 15, 2023 13:38
@GeorgeJahad@smengcl
Apply suggestions from code review
54aed00
Co-authored-by: Siyao Meng <50227127+smengcl@users.noreply.github.com>
George Jahad
addressed review comments
dd25523
smengcl
smengcl approved these changes Jun 15, 2023
View reviewed changes

@smengclsmengcl left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm, pending CI

@GeorgeJahad

GeorgeJahad commented Jun 16, 2023

Copy link
Copy Markdown
ContributorAuthor

@smengcl the checkstyle error is actually because I haven't merged to the latest master. Fixing now.

@GeorgeJahad
GeorgeJahad requested a review from smengclJune 16, 2023 03:04
@smengcl
smengcl merged commit 9f6cb9d into apache:masterJun 16, 2023
@smengcl

smengcl commented Jun 16, 2023

Copy link
Copy Markdown
Contributor

Thanks @GeorgeJahad for the improvement.

@GeorgeJahadGeorgeJahad mentioned this pull request Jul 1, 2023
Merged
@xBis7xBis7 mentioned this pull request Jul 4, 2023
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aswinshakilaswinshakilAwaiting requested review from aswinshakil
@smengclsmengclAwaiting requested review from smengcl

Assignees

No one assigned

Labels

snapshothttps://issues.apache.org/jira/browse/HDDS-6517

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@GeorgeJahad@smengcl@adoroszlai