Re: [PR] HDDS-15171. Add available space check on follower during bootstrap. [ozone]
jojochuang commented on PR #10185: URL: https://github.com/apache/ozone/pull/10185#issuecomment-4985698202 Merged. Thanks @sadanand48 @smengcl -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HDDS-15171. Add available space check on follower during bootstrap. [ozone]
jojochuang merged PR #10185: URL: https://github.com/apache/ozone/pull/10185 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HDDS-15171. Add available space check on follower during bootstrap. [ozone]
sadanand48 commented on code in PR #10185:
URL: https://github.com/apache/ozone/pull/10185#discussion_r3578584276
##
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java:
##
@@ -4113,17 +4110,9 @@ public synchronized TermIndex
installSnapshotFromLeader(String leaderId) throws
omDBCheckpoint = omRatisSnapshotProvider.
downloadDBSnapshotFromLeader(leaderId);
} catch (IOException ex) {
- if (OmRatisSnapshotProvider.isDiskFullOrQuotaIOException(ex)) {
-LOG.error(
-"Failed to download snapshot from leader {}: local disk appears
full or over quota "
-+ "on the OM ratis snapshot volume (see previous ERROR for
path/usable space). "
-+ "Free disk or adjust {}, {}, or {} before bootstrap can
succeed.",
-leaderId,
-OZONE_OM_BOOTSTRAP_MIN_SPACE_KEY,
-OZONE_OM_BOOTSTRAP_CHECKPOINT_HEADROOM_RATIO_KEY,
-OZONE_OM_CHECKPOINT_ESTIMATED_SST_BYTES_HEADER);
+ if (!OmRatisSnapshotProvider.isDiskFullOrQuotaIOException(ex)) {
+LOG.error("Failed to download snapshot from Leader {}.", leaderId, ex);
Review Comment:
I see it has been fixed by latest commit, please ignore
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] HDDS-15171. Add available space check on follower during bootstrap. [ozone]
sadanand48 commented on code in PR #10185:
URL: https://github.com/apache/ozone/pull/10185#discussion_r3578556364
##
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java:
##
@@ -4113,17 +4110,9 @@ public synchronized TermIndex
installSnapshotFromLeader(String leaderId) throws
omDBCheckpoint = omRatisSnapshotProvider.
downloadDBSnapshotFromLeader(leaderId);
} catch (IOException ex) {
- if (OmRatisSnapshotProvider.isDiskFullOrQuotaIOException(ex)) {
-LOG.error(
-"Failed to download snapshot from leader {}: local disk appears
full or over quota "
-+ "on the OM ratis snapshot volume (see previous ERROR for
path/usable space). "
-+ "Free disk or adjust {}, {}, or {} before bootstrap can
succeed.",
-leaderId,
-OZONE_OM_BOOTSTRAP_MIN_SPACE_KEY,
-OZONE_OM_BOOTSTRAP_CHECKPOINT_HEADROOM_RATIO_KEY,
-OZONE_OM_CHECKPOINT_ESTIMATED_SST_BYTES_HEADER);
+ if (!OmRatisSnapshotProvider.isDiskFullOrQuotaIOException(ex)) {
+LOG.error("Failed to download snapshot from Leader {}.", leaderId, ex);
Review Comment:
This is the common failure scenario, not just for disk out of space, will
remove the if block
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] HDDS-15171. Add available space check on follower during bootstrap. [ozone]
jojochuang commented on code in PR #10185:
URL: https://github.com/apache/ozone/pull/10185#discussion_r3573469408
##
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis_snapshot/OmRatisSnapshotProvider.java:
##
@@ -96,38 +204,64 @@ public OmRatisSnapshotProvider(File snapshotDir,
this.spnegoEnabled = spnegoEnabled;
this.connectionFactory = connectionFactory;
this.useV2CheckpointApi = OZONE_OM_DB_CHECKPOINT_USE_INODE_BASED_DEFAULT;
+this.bootstrapMinSpaceBytes = 0L;
+this.bootstrapCheckpointHeadroomRatio =
OZONE_OM_BOOTSTRAP_CHECKPOINT_HEADROOM_RATIO_DEFAULT;
}
public OmRatisSnapshotProvider(MutableConfigurationSource conf,
File omRatisSnapshotDir, Map peerNodeDetails) {
+this(conf, omRatisSnapshotDir, peerNodeDetails, null);
+ }
+
+ /**
+ * Same as {@link #OmRatisSnapshotProvider(MutableConfigurationSource, File,
Map)} but allows
+ * tests to inject a {@link URLConnectionFactory} (for example a factory
that returns a mock
+ * {@link HttpURLConnection}).
+ */
+ @VisibleForTesting
+ public OmRatisSnapshotProvider(MutableConfigurationSource conf,
+ File omRatisSnapshotDir,
+ Map peerNodeDetails,
+ URLConnectionFactory connectionFactoryOverride) {
super(omRatisSnapshotDir, OM_DB_NAME);
LOG.info("Initializing OM Snapshot Provider");
this.peerNodesMap = new ConcurrentHashMap<>();
peerNodesMap.putAll(peerNodeDetails);
this.useV2CheckpointApi =
conf.getBoolean(OZONE_OM_DB_CHECKPOINT_USE_INODE_BASED_KEY,
OZONE_OM_DB_CHECKPOINT_USE_INODE_BASED_DEFAULT);
+this.bootstrapMinSpaceBytes = (long) conf.getStorageSize(
+OZONE_OM_BOOTSTRAP_MIN_SPACE_KEY,
+OZONE_OM_BOOTSTRAP_MIN_SPACE_DEFAULT,
+StorageUnit.BYTES);
+this.bootstrapCheckpointHeadroomRatio = conf.getDouble(
+OZONE_OM_BOOTSTRAP_CHECKPOINT_HEADROOM_RATIO_KEY,
+OZONE_OM_BOOTSTRAP_CHECKPOINT_HEADROOM_RATIO_DEFAULT);
Review Comment:
nice to have, but it's unrelated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] HDDS-15171. Add available space check on follower during bootstrap. [ozone]
Copilot commented on code in PR #10185:
URL: https://github.com/apache/ozone/pull/10185#discussion_r3573297538
##
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis_snapshot/OmRatisSnapshotProvider.java:
##
@@ -86,6 +97,103 @@ public class OmRatisSnapshotProvider extends
RDBSnapshotProvider {
private final boolean spnegoEnabled;
private final URLConnectionFactory connectionFactory;
private final boolean useV2CheckpointApi;
+ /** Minimum usable bytes on snapshot volume before download; 0 = disabled. */
+ private final long bootstrapMinSpaceBytes;
+ /** Applied to leader-reported SST byte estimate to reserve tar/unpack
headroom. */
+ private final double bootstrapCheckpointHeadroomRatio;
+
+ private static final class BootstrapSpaceRequirement {
+private final long requiredBytes;
+private final boolean usedLeaderEstimateHeader;
+
+private BootstrapSpaceRequirement(long requiredBytes, boolean
usedLeaderEstimateHeader) {
+ this.requiredBytes = requiredBytes;
+ this.usedLeaderEstimateHeader = usedLeaderEstimateHeader;
+}
+ }
+
+ /**
+ * Whether this {@link IOException} (or its causes) typically means the
+ * local filesystem ran out of space or hit a quota while writing.
+ */
+ public static boolean isDiskFullOrQuotaIOException(IOException ioe) {
+for (Throwable t = ioe; t != null; t = t.getCause()) {
+ if (t instanceof DiskOutOfSpaceException) {
+return true;
+ }
+ if (matchesDiskFullOrQuotaMessage(t)) {
+return true;
+ }
+}
+return false;
+ }
+
+ /**
+ * Best-effort supplement for JDK {@link FileSystemException} ENOSPC and
+ * quota wording on Linux OM deployments (typically English from libc/JVM).
+ * {@link DiskOutOfSpaceException} in the cause chain is handled by type
+ * in {@link #isDiskFullOrQuotaIOException(IOException)} and does not depend
+ * on message text. Localized OS messages without matching substrings are not
+ * detected here.
+ */
+ private static boolean matchesDiskFullOrQuotaMessage(Throwable throwable) {
+if (throwable instanceof FileSystemException) {
+ String reason = ((FileSystemException) throwable).getReason();
+ if (reason != null && containsDiskFullOrQuotaText(reason)) {
+return true;
+ }
+}
+String msg = throwable.getMessage();
+return msg != null && containsDiskFullOrQuotaText(msg);
+ }
+
+ private static boolean containsDiskFullOrQuotaText(String text) {
+String m = text.toLowerCase(Locale.ROOT);
+return m.contains("no space left on device")
+|| m.contains("no space")
+|| m.contains("space left")
+|| m.contains("enospc")
+|| m.contains("disk quota exceeded")
+|| m.contains("quota exceeded")
+|| m.contains("quota");
+ }
Review Comment:
containsDiskFullOrQuotaText() matches very broad substrings ("no space",
"space left", and especially "quota"), which can misclassify unrelated
IOExceptions as disk-full/quota. This is now used to suppress logging in
OzoneManager, so a false positive can hide real snapshot failures from
operators.
##
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis_snapshot/OmRatisSnapshotProvider.java:
##
@@ -96,38 +204,64 @@ public OmRatisSnapshotProvider(File snapshotDir,
this.spnegoEnabled = spnegoEnabled;
this.connectionFactory = connectionFactory;
this.useV2CheckpointApi = OZONE_OM_DB_CHECKPOINT_USE_INODE_BASED_DEFAULT;
+this.bootstrapMinSpaceBytes = 0L;
+this.bootstrapCheckpointHeadroomRatio =
OZONE_OM_BOOTSTRAP_CHECKPOINT_HEADROOM_RATIO_DEFAULT;
}
public OmRatisSnapshotProvider(MutableConfigurationSource conf,
File omRatisSnapshotDir, Map peerNodeDetails) {
+this(conf, omRatisSnapshotDir, peerNodeDetails, null);
+ }
+
+ /**
+ * Same as {@link #OmRatisSnapshotProvider(MutableConfigurationSource, File,
Map)} but allows
+ * tests to inject a {@link URLConnectionFactory} (for example a factory
that returns a mock
+ * {@link HttpURLConnection}).
+ */
+ @VisibleForTesting
+ public OmRatisSnapshotProvider(MutableConfigurationSource conf,
+ File omRatisSnapshotDir,
+ Map peerNodeDetails,
+ URLConnectionFactory connectionFactoryOverride) {
super(omRatisSnapshotDir, OM_DB_NAME);
LOG.info("Initializing OM Snapshot Provider");
this.peerNodesMap = new ConcurrentHashMap<>();
peerNodesMap.putAll(peerNodeDetails);
this.useV2CheckpointApi =
conf.getBoolean(OZONE_OM_DB_CHECKPOINT_USE_INODE_BASED_KEY,
OZONE_OM_DB_CHECKPOINT_USE_INODE_BASED_DEFAULT);
+this.bootstrapMinSpaceBytes = (long) conf.getStorageSize(
+OZONE_OM_BOOTSTRAP_MIN_SPACE_KEY,
+OZONE_OM_BOOTSTRAP_MIN_SPACE_DEFAULT,
+StorageUnit.BYTES);
+this.bootstrapCheckpointHeadroomRatio = conf.getDouble(
+OZONE_OM_BOOTSTRAP_CHECKPOINT_HEADROOM_RATIO_KEY,
+
Re: [PR] HDDS-15171. Add available space check on follower during bootstrap. [ozone]
jojochuang commented on code in PR #10185:
URL: https://github.com/apache/ozone/pull/10185#discussion_r3573255466
##
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java:
##
@@ -4110,6 +4113,16 @@ public synchronized TermIndex
installSnapshotFromLeader(String leaderId) throws
omDBCheckpoint = omRatisSnapshotProvider.
downloadDBSnapshotFromLeader(leaderId);
} catch (IOException ex) {
+ if (OmRatisSnapshotProvider.isDiskFullOrQuotaIOException(ex)) {
+LOG.error(
+"Failed to download snapshot from leader {}: local disk appears
full or over quota "
++ "on the OM ratis snapshot volume (see previous ERROR for
path/usable space). "
++ "Free disk or adjust {}, {}, or {} before bootstrap can
succeed.",
+leaderId,
+OZONE_OM_BOOTSTRAP_MIN_SPACE_KEY,
+OZONE_OM_BOOTSTRAP_CHECKPOINT_HEADROOM_RATIO_KEY,
+OZONE_OM_CHECKPOINT_ESTIMATED_SST_BYTES_HEADER);
+ }
Review Comment:
Fixed: provider keeps the detailed disk-full ERROR; OzoneManager now skips
the generic "Failed to download snapshot" log when isDiskFullOrQuotaIOException
is true.
##
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis_snapshot/OmRatisSnapshotProvider.java:
##
@@ -86,6 +96,88 @@ public class OmRatisSnapshotProvider extends
RDBSnapshotProvider {
private final boolean spnegoEnabled;
private final URLConnectionFactory connectionFactory;
private final boolean useV2CheckpointApi;
+ /** Minimum usable bytes on snapshot volume before download; 0 = disabled. */
+ private final long bootstrapMinSpaceBytes;
+ /** Applied to leader-reported SST byte estimate to reserve tar/unpack
headroom. */
+ private final double bootstrapCheckpointHeadroomRatio;
+
+ private static final class BootstrapSpaceRequirement {
+private final long requiredBytes;
+private final boolean usedLeaderEstimateHeader;
+
+private BootstrapSpaceRequirement(long requiredBytes, boolean
usedLeaderEstimateHeader) {
+ this.requiredBytes = requiredBytes;
+ this.usedLeaderEstimateHeader = usedLeaderEstimateHeader;
+}
+ }
+
+ /**
+ * Whether this {@link IOException} (or its causes) typically means the
+ * local filesystem ran out of space or hit a quota while writing.
+ */
+ public static boolean isDiskFullOrQuotaIOException(IOException ioe) {
+for (Throwable t = ioe; t != null; t = t.getCause()) {
+ if (t instanceof FileSystemException) {
+FileSystemException fse = (FileSystemException) t;
+String reason = fse.getReason();
+if (reason != null) {
+ String r = reason.toLowerCase(Locale.ROOT);
+ if (r.contains("no space") || r.contains("space left")
+ || r.contains("quota") || r.contains("enospc")) {
+return true;
+ }
+}
+ }
+ String msg = t.getMessage();
+ if (msg != null) {
+String m = msg.toLowerCase(Locale.ROOT);
+if (m.contains("no space left on device")
+|| m.contains("enospc")
+|| m.contains("disk quota exceeded")
+|| m.contains("quota exceeded")) {
+ return true;
Review Comment:
Added DiskOutOfSpaceException type check in the cause chain. English
substring matching remains as a supplement for JDK FileSystemException ENOSPC
on Linux. Documented limitation in Javadoc + unit test.
##
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOMDbCheckpointServletInodeBasedXfer.java:
##
@@ -1005,6 +1015,51 @@ public static Map>
readFileToMap(String filePath) throws IO
return dataMap;
}
+ /**
+ * Follower bootstrap must abort before streaming when the leader's SST
estimate header
+ * implies more free space than is available (v2 inode-based checkpoint URL).
+ */
+ @Test
+ public void
testBootstrapSnapshotDownloadAbortsWhenDiskSpaceBelowLeaderSstEstimateV2()
Review Comment:
Covered by
testBootstrapSnapshotDownloadAbortsWhenDiskSpaceBelowLeaderSstEstimate with
@ValueSource(booleans = {true, false}) (inode-based and legacy checkpoint URL).
##
hadoop-hdds/common/src/main/resources/ozone-default.xml:
##
@@ -2378,6 +2379,16 @@
request OM snapshot from OM Leader.
+
+ozone.om.bootstrap.min.space
+5GB
Review Comment:
Leader sends X-Ozone-Om-Checkpoint-Estimated-Sst-Bytes × headroom ratio
(default 2.0). ozone.om.bootstrap.min.space (5GB) is only the fallback when the
header is absent.
##
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java:
##
@@ -291,6 +291,18 @@ public final class OMConfigKeys {
OZONE_OM_SNAPSHOT_PROVIDER_REQUEST_TIMEOUT_DEFAULT =
TimeDuration.valueOf(30, TimeUnit.MILLISECONDS);
+ public static final String OZONE_OM_BOOTSTRAP_MIN_SPACE_KEY =
+
Re: [PR] HDDS-15171. Add available space check on follower during bootstrap. [ozone]
smengcl commented on code in PR #10185:
URL: https://github.com/apache/ozone/pull/10185#discussion_r3555940919
##
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java:
##
@@ -4110,6 +4113,16 @@ public synchronized TermIndex
installSnapshotFromLeader(String leaderId) throws
omDBCheckpoint = omRatisSnapshotProvider.
downloadDBSnapshotFromLeader(leaderId);
} catch (IOException ex) {
+ if (OmRatisSnapshotProvider.isDiskFullOrQuotaIOException(ex)) {
+LOG.error(
+"Failed to download snapshot from leader {}: local disk appears
full or over quota "
++ "on the OM ratis snapshot volume (see previous ERROR for
path/usable space). "
++ "Free disk or adjust {}, {}, or {} before bootstrap can
succeed.",
+leaderId,
+OZONE_OM_BOOTSTRAP_MIN_SPACE_KEY,
+OZONE_OM_BOOTSTRAP_CHECKPOINT_HEADROOM_RATIO_KEY,
+OZONE_OM_CHECKPOINT_ESTIMATED_SST_BYTES_HEADER);
+ }
Review Comment:
Each disk-full failure would be logged 3 times:
1. this block
2. `logDiskFullOrQuotaDuringDownload` in the provider
3. `LOG.error("Failed to download snapshot from Leader", ex)` below
Ideally we should log only once for each failure. CMIIW
##
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis_snapshot/OmRatisSnapshotProvider.java:
##
@@ -86,6 +96,88 @@ public class OmRatisSnapshotProvider extends
RDBSnapshotProvider {
private final boolean spnegoEnabled;
private final URLConnectionFactory connectionFactory;
private final boolean useV2CheckpointApi;
+ /** Minimum usable bytes on snapshot volume before download; 0 = disabled. */
+ private final long bootstrapMinSpaceBytes;
+ /** Applied to leader-reported SST byte estimate to reserve tar/unpack
headroom. */
+ private final double bootstrapCheckpointHeadroomRatio;
+
+ private static final class BootstrapSpaceRequirement {
+private final long requiredBytes;
+private final boolean usedLeaderEstimateHeader;
+
+private BootstrapSpaceRequirement(long requiredBytes, boolean
usedLeaderEstimateHeader) {
+ this.requiredBytes = requiredBytes;
+ this.usedLeaderEstimateHeader = usedLeaderEstimateHeader;
+}
+ }
+
+ /**
+ * Whether this {@link IOException} (or its causes) typically means the
+ * local filesystem ran out of space or hit a quota while writing.
+ */
+ public static boolean isDiskFullOrQuotaIOException(IOException ioe) {
+for (Throwable t = ioe; t != null; t = t.getCause()) {
+ if (t instanceof FileSystemException) {
+FileSystemException fse = (FileSystemException) t;
+String reason = fse.getReason();
+if (reason != null) {
+ String r = reason.toLowerCase(Locale.ROOT);
+ if (r.contains("no space") || r.contains("space left")
+ || r.contains("quota") || r.contains("enospc")) {
+return true;
+ }
+}
+ }
+ String msg = t.getMessage();
+ if (msg != null) {
+String m = msg.toLowerCase(Locale.ROOT);
+if (m.contains("no space left on device")
+|| m.contains("enospc")
+|| m.contains("disk quota exceeded")
+|| m.contains("quota exceeded")) {
+ return true;
Review Comment:
String-matching English substrings might miss localized error messages in
other languages?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] HDDS-15171. Add available space check on follower during bootstrap. [ozone]
sadanand48 commented on code in PR #10185: URL: https://github.com/apache/ozone/pull/10185#discussion_r3485303515 ## hadoop-hdds/common/src/main/resources/ozone-default.xml: ## @@ -2394,6 +2394,29 @@ request OM snapshot from OM Leader. + +ozone.om.bootstrap.min.space +5GB +OZONE, OM, HA, MANAGEMENT + + Minimum free space required on the volume that holds ozone.om.ratis.snapshot.dir + before an OM follower downloads a ratis/bootstrap checkpoint from the leader, + when the leader does not supply the X-Ozone-Om-Checkpoint-Estimated-Sst-Bytes header + (incremental checkpoint or older OM version). + Use storage size syntax (e.g. 10GB). Set to 0 to disable this fallback check. + + + + +ozone.om.bootstrap.checkpoint.estimated.space.headroom.ratio +1.25 Review Comment: Thanks for catching, addressed. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HDDS-15171. Add available space check on follower during bootstrap. [ozone]
SaketaChalamchala commented on code in PR #10185: URL: https://github.com/apache/ozone/pull/10185#discussion_r3484074816 ## hadoop-hdds/common/src/main/resources/ozone-default.xml: ## @@ -2394,6 +2394,29 @@ request OM snapshot from OM Leader. + +ozone.om.bootstrap.min.space +5GB +OZONE, OM, HA, MANAGEMENT + + Minimum free space required on the volume that holds ozone.om.ratis.snapshot.dir + before an OM follower downloads a ratis/bootstrap checkpoint from the leader, + when the leader does not supply the X-Ozone-Om-Checkpoint-Estimated-Sst-Bytes header + (incremental checkpoint or older OM version). + Use storage size syntax (e.g. 10GB). Set to 0 to disable this fallback check. + + + + +ozone.om.bootstrap.checkpoint.estimated.space.headroom.ratio +1.25 Review Comment: @sadanand48 headroom default is 1.25 here and 2.0 in `OMConfigKeys`. Could you make the default consistent? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HDDS-15171. Add available space check on follower during bootstrap. [ozone]
sadanand48 commented on code in PR #10185:
URL: https://github.com/apache/ozone/pull/10185#discussion_r3440715469
##
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOMDbCheckpointServletInodeBasedXfer.java:
##
@@ -1005,6 +1015,51 @@ public static Map>
readFileToMap(String filePath) throws IO
return dataMap;
}
+ /**
+ * Follower bootstrap must abort before streaming when the leader's SST
estimate header
+ * implies more free space than is available (v2 inode-based checkpoint URL).
+ */
+ @Test
+ public void
testBootstrapSnapshotDownloadAbortsWhenDiskSpaceBelowLeaderSstEstimateV2()
Review Comment:
done.
##
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/ratis_snapshot/TestOmRatisSnapshotProvider.java:
##
@@ -80,6 +87,46 @@ public void setup(@TempDir File snapshotDir,
false, connectionFactory);
}
+ @Test
+ public void testIsDiskFullOrQuotaIOExceptionDetectsNoSpaceMessage() {
+assertTrue(OmRatisSnapshotProvider.isDiskFullOrQuotaIOException(
+new IOException("No space left on device")));
+ }
+
+ @Test
+ public void
testIsDiskFullOrQuotaIOExceptionDetectsFileSystemExceptionReason() {
+IOException wrapped = new IOException("write failed",
+new FileSystemException("p", null, "No space left on device"));
+assertTrue(OmRatisSnapshotProvider.isDiskFullOrQuotaIOException(wrapped));
+ }
+
+ @Test
+ public void testIsDiskFullOrQuotaIOExceptionReturnsFalseForOtherErrors() {
+assertFalse(OmRatisSnapshotProvider.isDiskFullOrQuotaIOException(
+new IOException("Connection reset")));
+ }
+
+ @Test
+ public void testBootstrapDiskSpaceCheckSkippedWhenZero(@TempDir File
snapshotDir) {
+OzoneConfiguration conf = new OzoneConfiguration();
+conf.set(OMConfigKeys.OZONE_OM_BOOTSTRAP_MIN_SPACE_KEY, "0GB");
+OmRatisSnapshotProvider provider =
+new OmRatisSnapshotProvider(conf, snapshotDir, new HashMap<>());
+assertDoesNotThrow(() -> provider.ensureBootstrapDiskSpace());
+ }
+
+ @Test
+ public void testBootstrapDiskSpaceCheckFailsWhenBelowMinimum(@TempDir File
snapshotDir) {
+OzoneConfiguration conf = new OzoneConfiguration();
+conf.set(OMConfigKeys.OZONE_OM_BOOTSTRAP_MIN_SPACE_KEY, "1024EB");
+OmRatisSnapshotProvider provider =
+new OmRatisSnapshotProvider(conf, snapshotDir, new HashMap<>());
+IOException ex =
+assertThrows(IOException.class, provider::ensureBootstrapDiskSpace);
+assertEquals(true,
+
ex.getMessage().contains(OMConfigKeys.OZONE_OM_BOOTSTRAP_MIN_SPACE_KEY));
Review Comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] HDDS-15171. Add available space check on follower during bootstrap. [ozone]
sadanand48 commented on code in PR #10185:
URL: https://github.com/apache/ozone/pull/10185#discussion_r3440717186
##
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java:
##
@@ -291,6 +291,18 @@ public final class OMConfigKeys {
OZONE_OM_SNAPSHOT_PROVIDER_REQUEST_TIMEOUT_DEFAULT =
TimeDuration.valueOf(30, TimeUnit.MILLISECONDS);
+ public static final String OZONE_OM_BOOTSTRAP_MIN_SPACE_KEY =
+ "ozone.om.bootstrap.min.space";
+ public static final String OZONE_OM_BOOTSTRAP_MIN_SPACE_DEFAULT = "5GB";
+
+ /**
+ * Multiplier applied to the leader-reported estimated SST bytes when
deciding
+ * minimum free space before downloading a checkpoint (tar + unpack
headroom).
+ */
+ public static final String OZONE_OM_BOOTSTRAP_CHECKPOINT_HEADROOM_RATIO_KEY =
+ "ozone.om.bootstrap.checkpoint.estimated.space.headroom.ratio";
+ public static final double
OZONE_OM_BOOTSTRAP_CHECKPOINT_HEADROOM_RATIO_DEFAULT = 1.25D;
Review Comment:
done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] HDDS-15171. Add available space check on follower during bootstrap. [ozone]
sadanand48 commented on code in PR #10185:
URL: https://github.com/apache/ozone/pull/10185#discussion_r3436993269
##
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java:
##
@@ -4101,6 +4052,13 @@ public synchronized TermIndex
installSnapshotFromLeader(String leaderId) throws
omDBCheckpoint = omRatisSnapshotProvider.
downloadDBSnapshotFromLeader(leaderId);
} catch (IOException ex) {
+ if (OmRatisSnapshotProvider.isDiskFullOrQuotaIOException(ex)) {
Review Comment:
revamped the code so the comment doesn't apply , resolving
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] HDDS-15171. Add available space check on follower during bootstrap. [ozone]
smengcl commented on code in PR #10185: URL: https://github.com/apache/ozone/pull/10185#discussion_r3284782473 ## hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOMDbCheckpointServletInodeBasedXfer.java: ## @@ -1005,6 +1015,51 @@ public static Map> readFileToMap(String filePath) throws IO return dataMap; } + /** + * Follower bootstrap must abort before streaming when the leader's SST estimate header + * implies more free space than is available (v2 inode-based checkpoint URL). + */ + @Test + public void testBootstrapSnapshotDownloadAbortsWhenDiskSpaceBelowLeaderSstEstimateV2() Review Comment: could you test v1 as well since that would also have the new header? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HDDS-15171. Add available space check on follower during bootstrap. [ozone]
smengcl commented on code in PR #10185:
URL: https://github.com/apache/ozone/pull/10185#discussion_r3284690015
##
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/ratis_snapshot/TestOmRatisSnapshotProvider.java:
##
@@ -80,6 +87,46 @@ public void setup(@TempDir File snapshotDir,
false, connectionFactory);
}
+ @Test
+ public void testIsDiskFullOrQuotaIOExceptionDetectsNoSpaceMessage() {
+assertTrue(OmRatisSnapshotProvider.isDiskFullOrQuotaIOException(
+new IOException("No space left on device")));
+ }
+
+ @Test
+ public void
testIsDiskFullOrQuotaIOExceptionDetectsFileSystemExceptionReason() {
+IOException wrapped = new IOException("write failed",
+new FileSystemException("p", null, "No space left on device"));
+assertTrue(OmRatisSnapshotProvider.isDiskFullOrQuotaIOException(wrapped));
+ }
+
+ @Test
+ public void testIsDiskFullOrQuotaIOExceptionReturnsFalseForOtherErrors() {
+assertFalse(OmRatisSnapshotProvider.isDiskFullOrQuotaIOException(
+new IOException("Connection reset")));
+ }
+
+ @Test
+ public void testBootstrapDiskSpaceCheckSkippedWhenZero(@TempDir File
snapshotDir) {
+OzoneConfiguration conf = new OzoneConfiguration();
+conf.set(OMConfigKeys.OZONE_OM_BOOTSTRAP_MIN_SPACE_KEY, "0GB");
+OmRatisSnapshotProvider provider =
+new OmRatisSnapshotProvider(conf, snapshotDir, new HashMap<>());
+assertDoesNotThrow(() -> provider.ensureBootstrapDiskSpace());
+ }
+
+ @Test
+ public void testBootstrapDiskSpaceCheckFailsWhenBelowMinimum(@TempDir File
snapshotDir) {
+OzoneConfiguration conf = new OzoneConfiguration();
+conf.set(OMConfigKeys.OZONE_OM_BOOTSTRAP_MIN_SPACE_KEY, "1024EB");
+OmRatisSnapshotProvider provider =
+new OmRatisSnapshotProvider(conf, snapshotDir, new HashMap<>());
+IOException ex =
+assertThrows(IOException.class, provider::ensureBootstrapDiskSpace);
+assertEquals(true,
+
ex.getMessage().contains(OMConfigKeys.OZONE_OM_BOOTSTRAP_MIN_SPACE_KEY));
Review Comment:
nit: use `assertThat().contains(...)` instead. See HDDS-9951
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] HDDS-15171. Add available space check on follower during bootstrap. [ozone]
smengcl commented on code in PR #10185:
URL: https://github.com/apache/ozone/pull/10185#discussion_r3284671097
##
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java:
##
@@ -291,6 +291,18 @@ public final class OMConfigKeys {
OZONE_OM_SNAPSHOT_PROVIDER_REQUEST_TIMEOUT_DEFAULT =
TimeDuration.valueOf(30, TimeUnit.MILLISECONDS);
+ public static final String OZONE_OM_BOOTSTRAP_MIN_SPACE_KEY =
+ "ozone.om.bootstrap.min.space";
+ public static final String OZONE_OM_BOOTSTRAP_MIN_SPACE_DEFAULT = "5GB";
+
+ /**
+ * Multiplier applied to the leader-reported estimated SST bytes when
deciding
+ * minimum free space before downloading a checkpoint (tar + unpack
headroom).
+ */
+ public static final String OZONE_OM_BOOTSTRAP_CHECKPOINT_HEADROOM_RATIO_KEY =
+ "ozone.om.bootstrap.checkpoint.estimated.space.headroom.ratio";
+ public static final double
OZONE_OM_BOOTSTRAP_CHECKPOINT_HEADROOM_RATIO_DEFAULT = 1.25D;
Review Comment:
should this be 2? considering unpack
```suggestion
public static final double
OZONE_OM_BOOTSTRAP_CHECKPOINT_HEADROOM_RATIO_DEFAULT = 2.0D;
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] HDDS-15171. Add available space check on follower during bootstrap. [ozone]
SaketaChalamchala commented on code in PR #10185:
URL: https://github.com/apache/ozone/pull/10185#discussion_r3222752747
##
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis_snapshot/OmRatisSnapshotProvider.java:
##
@@ -86,6 +92,75 @@ public class OmRatisSnapshotProvider extends
RDBSnapshotProvider {
private final boolean spnegoEnabled;
private final URLConnectionFactory connectionFactory;
private final boolean useV2CheckpointApi;
+ /** Minimum usable bytes on snapshot volume before download; 0 = disabled. */
+ private final long bootstrapMinSpaceBytes;
+
+ /**
+ * Whether this {@link IOException} (or its causes) typically means the
+ * local filesystem ran out of space or hit a quota while writing.
+ */
+ public static boolean isDiskFullOrQuotaIOException(IOException ioe) {
+for (Throwable t = ioe; t != null; t = t.getCause()) {
+ if (t instanceof FileSystemException) {
+FileSystemException fse = (FileSystemException) t;
+String reason = fse.getReason();
Review Comment:
Can do the error message string matching once
```suggestion
String reason = (t instanceof FileSystemException fse &&
fse.getReason() != null)
? fse.getReason() : t.getMessage();
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] HDDS-15171. Add available space check on follower during bootstrap. [ozone]
SaketaChalamchala commented on code in PR #10185:
URL: https://github.com/apache/ozone/pull/10185#discussion_r3222766832
##
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java:
##
@@ -4101,6 +4052,13 @@ public synchronized TermIndex
installSnapshotFromLeader(String leaderId) throws
omDBCheckpoint = omRatisSnapshotProvider.
downloadDBSnapshotFromLeader(leaderId);
} catch (IOException ex) {
+ if (OmRatisSnapshotProvider.isDiskFullOrQuotaIOException(ex)) {
Review Comment:
Isn't an error already logged in `downloadDBSnapshotFromLeader`? Why do we
need to log it again here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] HDDS-15171. Add available space check on follower during bootstrap. [ozone]
SaketaChalamchala commented on code in PR #10185: URL: https://github.com/apache/ozone/pull/10185#discussion_r3222699757 ## hadoop-hdds/common/src/main/resources/ozone-default.xml: ## @@ -2378,6 +2379,16 @@ request OM snapshot from OM Leader. + +ozone.om.bootstrap.min.space +5GB Review Comment: +1. Maybe reuse the estimates from `org.apache.hadoop.ozone.om.snapshot.logEstimatedTarballSize` in the preemptive space check before the transfer. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HDDS-15171. Add available space check on follower during bootstrap. [ozone]
smengcl commented on code in PR #10185: URL: https://github.com/apache/ozone/pull/10185#discussion_r3204048521 ## hadoop-hdds/common/src/main/resources/ozone-default.xml: ## @@ -2378,6 +2379,16 @@ request OM snapshot from OM Leader. + +ozone.om.bootstrap.min.space +5GB Review Comment: This is a good first step, but the best approach is to get an estimate on how much this space it would actually need to download and unpack, because it could be well exceeding 5GB? CMIIW -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HDDS-15171. Add available space check on follower during bootstrap. [ozone]
smengcl commented on PR #10185: URL: https://github.com/apache/ozone/pull/10185#issuecomment-4398442112 Thanks @sadanand48 . CI is failing in checkstyle: - https://github.com/sadanand48/hadoop-ozone/actions/runs/25367784140/job/74383081956 - https://github.com/sadanand48/hadoop-ozone/actions/runs/25367784140/job/74383082028 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
