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]
