-
Notifications
You must be signed in to change notification settings - Fork 614
HDDS-13912. Modularise Snapshot Delta file computer (Full Diff) #9283
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
Changes from 195 commits
6e241f6
a869500
252d338
4099bc6
e02670c
5a66cfc
79580e9
2a331ef
c4f69e2
afbc592
70ac2c7
b554cc7
49eccfa
51eda04
25f766c
96689fa
0674299
5d9fc49
2d88176
a3c4c69
686d0c7
491a54b
5e69ee9
d36622a
ee213d1
81871b2
a95604e
5a90fcf
20d7d6a
ae655cb
25fa6ae
d419283
8a44308
cb94c36
e26052c
4d272d1
2a38f59
ca098cf
67d4b3d
9838cda
6a19dbb
665f411
2894e40
ea0ab16
915562b
1c0d0ac
24da3eb
8f3774a
503cd4e
6865fad
903ecd1
06d1e99
60a7728
4711517
af8754c
655a724
6386c1b
2bc6134
0de7c62
8e8c534
f148f24
da030c0
6af6498
b281569
1ad24b4
d629911
2aecde4
8eeb44b
d9301b3
06e7d37
efd6c51
fab85ea
c73a355
76b99e2
1d39bee
54f1508
b1a3834
1986bbe
52be3dd
5f50a04
908c47d
278605a
40265e1
ac4719b
cf19dce
34097de
c46ddc2
99afc02
fcc630e
6f144e2
4600c96
48ec0bb
f524cad
cb31b7c
02dd061
ff90af8
8b014dd
57662c6
bcc0fc8
79a46f4
cd24a81
5f0bb91
3de4346
5b55a59
aa6facf
cc35056
9c1689c
3f59895
95341dd
616bef3
b0023d1
36b6fb3
4596386
7af6521
e19dae2
8c1373a
25ee4e2
c6e3914
fd4bfdb
49f4424
613d106
8a29736
cca2dbf
a810cc1
78c1036
a2bbea5
bf4746f
09d955c
cde567d
49c662a
2cf1bce
7afc8f5
5849dac
e58ff09
83b887e
519495a
c125250
408e213
715b2f0
ec59b89
b0b6d6a
a759807
808b174
c829a8b
8e91e47
bfd341c
4ccd3fc
4fd3b0e
41b7cfb
3a8c8f6
d0422ae
4ff8cea
649468d
bb5139f
c46e1ae
298ae36
8e43b90
55c68bd
d1d04cf
974b5c2
018571c
6cd54dd
261a669
8955d80
ac88692
91c143a
151e887
3771ff4
0736d82
894a047
5823347
e3c9a94
6fbf8c7
ed007a4
43923cb
ea64602
0b16ef6
1507659
4c2a175
e2d5e83
9d4dd26
d696ffa
8523dea
9ee0298
1d771e0
121977e
9736247
2167b03
08156e4
84a0352
8890252
1067935
b086376
90c28de
ed8ee79
7c27532
025411d
34fa1b4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -20,6 +20,7 @@ | |||||
| import static org.apache.hadoop.hdds.utils.Archiver.includeFile; | ||||||
| import static org.apache.hadoop.hdds.utils.Archiver.tar; | ||||||
| import static org.apache.hadoop.hdds.utils.HddsServerUtil.includeRatisSnapshotCompleteFlag; | ||||||
| import static org.apache.hadoop.hdds.utils.IOUtils.getINode; | ||||||
| import static org.apache.hadoop.ozone.OzoneConsts.OM_CHECKPOINT_DIR; | ||||||
| import static org.apache.hadoop.ozone.OzoneConsts.OM_SNAPSHOT_CHECKPOINT_DIR; | ||||||
| import static org.apache.hadoop.ozone.OzoneConsts.OM_SNAPSHOT_DIR; | ||||||
|
|
@@ -535,8 +536,7 @@ private static Path findLinkPath(Map<String, Map<Path, Path>> files, Path file) | |||||
| // Check if the files are hard linked to each other. | ||||||
| // Note comparison must be done against srcPath, because | ||||||
| // destPath may only exist on Follower. | ||||||
| if (OmSnapshotUtils.getINode(srcPath).equals( | ||||||
| OmSnapshotUtils.getINode(file))) { | ||||||
| if (getINode(srcPath).equals(getINode(file))) { | ||||||
|
||||||
| if (getINode(srcPath).equals(getINode(file))) { | |
| if (Files.isSameFile(srcPath, file)) { |
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.
Removed the visibleForTesting not relevant anymore
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,50 @@ | ||||||||||||
| /* | ||||||||||||
| * Licensed to the Apache Software Foundation (ASF) under one or more | ||||||||||||
| * contributor license agreements. See the NOTICE file distributed with | ||||||||||||
| * this work for additional information regarding copyright ownership. | ||||||||||||
| * The ASF licenses this file to You under the Apache License, Version 2.0 | ||||||||||||
| * (the "License"); you may not use this file except in compliance with | ||||||||||||
| * the License. You may obtain a copy of the License at | ||||||||||||
| * | ||||||||||||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||||||||||||
| * | ||||||||||||
| * Unless required by applicable law or agreed to in writing, software | ||||||||||||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||||||||||||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||||||||||||
| * See the License for the specific language governing permissions and | ||||||||||||
| * limitations under the License. | ||||||||||||
| */ | ||||||||||||
|
|
||||||||||||
| package org.apache.hadoop.ozone.om.snapshot.diff.delta; | ||||||||||||
|
|
||||||||||||
| import java.io.Closeable; | ||||||||||||
| import java.io.IOException; | ||||||||||||
| import java.nio.file.Path; | ||||||||||||
| import java.util.Collection; | ||||||||||||
| import java.util.Set; | ||||||||||||
| import org.apache.commons.lang3.tuple.Pair; | ||||||||||||
| import org.apache.hadoop.ozone.om.helpers.SnapshotInfo; | ||||||||||||
| import org.apache.ozone.rocksdb.util.SstFileInfo; | ||||||||||||
|
|
||||||||||||
| /** | ||||||||||||
| * The DeltaFileComputer interface defines a contract for computing delta files | ||||||||||||
| * that represent changes between two snapshots. Implementations of this | ||||||||||||
| * interface are responsible for determining the modifications made from a | ||||||||||||
| * baseline snapshot to a target snapshot in the form of delta files. | ||||||||||||
| */ | ||||||||||||
| public interface DeltaFileComputer extends Closeable { | ||||||||||||
|
|
||||||||||||
| /** | ||||||||||||
| * Retrieves the delta files representing changes between two snapshots for specified tables. | ||||||||||||
| * | ||||||||||||
| * @param fromSnapshot the baseline snapshot from which changes are computed | ||||||||||||
| * @param toSnapshot the target snapshot to which changes are compared | ||||||||||||
| * @param tablesToLookup the set of table names to consider when determining changes | ||||||||||||
| * @return an {@code Optional} containing a collection of pairs, where each pair consists of a | ||||||||||||
| * {@code Path} representing the delta file and an associated {@code SstFileInfo}, or | ||||||||||||
| * an empty {@code Optional} if no changes are found | ||||||||||||
|
||||||||||||
| * @return an {@code Optional} containing a collection of pairs, where each pair consists of a | |
| * {@code Path} representing the delta file and an associated {@code SstFileInfo}, or | |
| * an empty {@code Optional} if no changes are found | |
| * @return a collection of pairs, where each pair consists of a {@code Path} representing the delta file | |
| * and an associated {@code SstFileInfo} |
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.
done
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,155 @@ | ||||||
| /* | ||||||
| * Licensed to the Apache Software Foundation (ASF) under one or more | ||||||
| * contributor license agreements. See the NOTICE file distributed with | ||||||
| * this work for additional information regarding copyright ownership. | ||||||
| * The ASF licenses this file to You under the Apache License, Version 2.0 | ||||||
| * (the "License"); you may not use this file except in compliance with | ||||||
| * the License. You may obtain a copy of the License at | ||||||
| * | ||||||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||||||
| * | ||||||
| * Unless required by applicable law or agreed to in writing, software | ||||||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||||||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||||||
| * See the License for the specific language governing permissions and | ||||||
| * limitations under the License. | ||||||
| */ | ||||||
|
|
||||||
| package org.apache.hadoop.ozone.om.snapshot.diff.delta; | ||||||
|
|
||||||
| import static java.nio.file.Files.createDirectories; | ||||||
| import static org.apache.commons.io.FilenameUtils.getExtension; | ||||||
| import static org.apache.commons.io.file.PathUtils.deleteDirectory; | ||||||
|
|
||||||
| import java.io.IOException; | ||||||
| import java.nio.file.FileAlreadyExistsException; | ||||||
| import java.nio.file.Files; | ||||||
| import java.nio.file.Path; | ||||||
| import java.util.Collection; | ||||||
| import java.util.Map; | ||||||
| import java.util.Optional; | ||||||
| import java.util.Set; | ||||||
| import java.util.UUID; | ||||||
| import java.util.concurrent.atomic.AtomicInteger; | ||||||
| import java.util.function.Consumer; | ||||||
| import org.apache.commons.lang3.StringUtils; | ||||||
| import org.apache.commons.lang3.tuple.Pair; | ||||||
| import org.apache.hadoop.hdds.utils.db.TablePrefixInfo; | ||||||
| import org.apache.hadoop.ozone.om.OMMetadataManager; | ||||||
| import org.apache.hadoop.ozone.om.OmSnapshot; | ||||||
| import org.apache.hadoop.ozone.om.OmSnapshotManager; | ||||||
| import org.apache.hadoop.ozone.om.helpers.SnapshotInfo; | ||||||
| import org.apache.hadoop.ozone.om.snapshot.OmSnapshotLocalDataManager.ReadableOmSnapshotLocalDataProvider; | ||||||
| import org.apache.hadoop.ozone.snapshot.SnapshotDiffResponse.SubStatus; | ||||||
| import org.apache.ozone.rocksdb.util.SstFileInfo; | ||||||
| import org.apache.ratis.util.function.UncheckedAutoCloseableSupplier; | ||||||
| import org.slf4j.Logger; | ||||||
| import org.slf4j.LoggerFactory; | ||||||
|
|
||||||
| /** | ||||||
| * The {@code FileLinkDeltaFileComputer} is an abstract class that provides a | ||||||
| * base implementation for the {@code DeltaFileComputer} interface. It is | ||||||
| * responsible for computing delta files by creating hard links to the | ||||||
| * relevant source files in a specified delta directory, enabling a compact | ||||||
| * representation of changes between snapshots. | ||||||
| * | ||||||
| * This class encapsulates the logic for managing snapshots and metadata, | ||||||
| * creating hard links for delta representation, and reporting activity | ||||||
| * during the computation process. | ||||||
| */ | ||||||
| public abstract class FileLinkDeltaFileComputer implements DeltaFileComputer { | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. apart from test code, where does it get used?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This would be used in the PR #9312 |
||||||
|
|
||||||
| private static final Logger LOG = LoggerFactory.getLogger(FileLinkDeltaFileComputer.class); | ||||||
| private final OmSnapshotManager omSnapshotManager; | ||||||
| private final OMMetadataManager activeMetadataManager; | ||||||
| private final Consumer<SubStatus> activityReporter; | ||||||
| private Path deltaDir; | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. deltaDir is a path to a temporary directory whose life cycle is managed by an instance of this class. |
||||||
| private AtomicInteger linkFileCounter = new AtomicInteger(0); | ||||||
|
||||||
| private AtomicInteger linkFileCounter = new AtomicInteger(0); | |
| private final AtomicInteger linkFileCounter = new AtomicInteger(0); |
Copilot
AI
Nov 18, 2025
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.
The error message contains inconsistent formatting with a missing space before "tablesToLookup". Should be:
"Failed to compute delta files for snapshots %s and %s. tablesToLookup: %s"or
"Failed to compute delta files for snapshots %s and %s, tablesToLookup: %s"| "Failed to compute delta files for snapshots %s and %s tablesToLookup : %s", fromSnapshot, toSnapshot, | |
| "Failed to compute delta files for snapshots %s and %s. tablesToLookup: %s", fromSnapshot, toSnapshot, |
Copilot
AI
Nov 18, 2025
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.
Extra space before catch keyword. Should be:
} catch (FileAlreadyExistsException ignored) {| } catch (FileAlreadyExistsException ignored) { | |
| } catch (FileAlreadyExistsException ignored) { |
Copilot
AI
Nov 18, 2025
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.
The infinite loop with do-while (!createdLink) lacks a safety mechanism to prevent infinite loops in case of persistent failures. Consider adding a maximum retry limit to prevent resource exhaustion:
int maxRetries = 1000;
int attempts = 0;
do {
if (++attempts > maxRetries) {
throw new IOException("Failed to create link after " + maxRetries + " attempts for path: " + source);
}
link = deltaDir.resolve(linkFileCounter.incrementAndGet() + extension);
try {
Files.createLink(link, source);
createdLink = true;
} catch (FileAlreadyExistsException ignored) {
LOG.info("File for source {} already exists: at {}. Will attempt to create link with a different path", source, link);
}
} while (!createdLink);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.
this shouldn't happen ideally. Not required
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.
Access of element annotated with VisibleForTesting found in production code.
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.
Removed the visibleForTesting not relevant anymore