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<String, OMNodeDetails> 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<String, OMNodeDetails> 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:
bootstrapMinSpaceBytes and bootstrapCheckpointHeadroomRatio are read
directly from config with no validation. Negative/non-finite headroom ratios
(or negative min space) can effectively disable the disk-space check,
undermining the intent of this change and making behavior dependent on invalid
config values.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java:
##########
@@ -4110,7 +4110,9 @@ public synchronized TermIndex
installSnapshotFromLeader(String leaderId) throws
omDBCheckpoint = omRatisSnapshotProvider.
downloadDBSnapshotFromLeader(leaderId);
} catch (IOException ex) {
- LOG.error("Failed to download snapshot from Leader {}.", leaderId, ex);
+ if (!OmRatisSnapshotProvider.isDiskFullOrQuotaIOException(ex)) {
+ LOG.error("Failed to download snapshot from Leader {}.", leaderId, ex);
+ }
Review Comment:
installSnapshotFromLeader() now suppresses the error log entirely when the
exception looks like disk-full/quota. Disk-full can also happen during untar /
cleanup inside downloadDBSnapshotFromLeader (not just during the HTTP
download), and then this catch will hide the failure from logs. Consider
logging a concise message for disk-full/quota instead of skipping logging
altogether.
--
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]
