sreejasahithi commented on code in PR #10074:
URL: https://github.com/apache/ozone/pull/10074#discussion_r3072955906
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/ReconServerConfigKeys.java:
##########
@@ -132,9 +132,27 @@ public final class ReconServerConfigKeys {
public static final String
OZONE_RECON_METRICS_HTTP_CONNECTION_REQUEST_TIMEOUT_DEFAULT = "60s";
+ /**
+ * Total container count drift threshold above which the periodic incremental
+ * sync escalates to a full SCM DB snapshot.
+ *
+ * <p>When {@code |SCM_total_containers - Recon_total_containers|} exceeds
+ * this value the targeted 4-pass sync becomes expensive (many batched RPC
+ * rounds) and a full checkpoint replacement is cheaper and more reliable.
+ * For drift at or below this value the incremental sync corrects the gap
+ * without replacing the entire database.
+ *
+ * <p>Note: a full snapshot is also scheduled unconditionally every 24h
+ * (configurable via {@code ozone.recon.scm.snapshot.task.interval.delay})
+ * as a structural safety net, independent of this threshold.
+ *
+ * <p>Default: 10,000. In large clusters (millions of containers) operators
+ * may raise this further since the targeted sync handles per-state
+ * corrections efficiently even at higher drift levels.
+ */
public static final String OZONE_RECON_SCM_CONTAINER_THRESHOLD =
"ozone.recon.scm.container.threshold";
- public static final int OZONE_RECON_SCM_CONTAINER_THRESHOLD_DEFAULT = 100;
+ public static final int OZONE_RECON_SCM_CONTAINER_THRESHOLD_DEFAULT = 10_000;
Review Comment:
Default value of ozone.recon.scm.container.threshold is changed here but its
default value in ozone-default.xml still refers to 100.
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/ReconStorageContainerSyncHelper.java:
##########
@@ -53,52 +131,574 @@ class ReconStorageContainerSyncHelper {
this.containerManager = containerManager;
}
+ /**
+ * Decides what sync action the periodic scheduler should take based on the
+ * observed drift between SCM and Recon.
+ *
+ * <p>Decision logic:
+ * <ol>
+ * <li>If {@code |SCM_total - Recon_total| >
ozone.recon.scm.container.threshold}
+ * (default 10,000): return {@link SyncAction#FULL_SNAPSHOT}. The gap
is
+ * too large for incremental repair; a full checkpoint replacement is
+ * cheaper and more reliable at that scale.</li>
+ * <li>If total drift is positive but within the threshold (1 to 10,000):
+ * return {@link SyncAction#TARGETED_SYNC}.</li>
+ * <li>If total drift is zero, check per-state drift for each active
+ * (non-terminal) lifecycle state against
+ * {@code ozone.recon.scm.per.state.drift.threshold} (default 5):
+ * <ul>
+ * <li><b>OPEN</b>: detects containers stuck OPEN in Recon after SCM
+ * has advanced them to QUASI_CLOSED or CLOSED.</li>
+ * <li><b>QUASI_CLOSED</b>: detects containers stuck QUASI_CLOSED in
+ * Recon after SCM has advanced them to CLOSED. This case
produces
+ * zero OPEN drift and is invisible to an OPEN-only check.</li>
+ * </ul>
+ * If drift in <em>any</em> checked state exceeds the threshold:
+ * return {@link SyncAction#TARGETED_SYNC}.</li>
+ * <li>Otherwise: return {@link SyncAction#NO_ACTION}.</li>
+ * </ol>
+ *
+ * <p>Per-state drift deliberately routes to targeted sync, not a full
+ * snapshot — the targeted sync's per-state passes correct each condition
+ * efficiently without replacing the entire database.
+ *
+ * @return the recommended {@link SyncAction}
+ * @throws IOException if SCM RPC calls to retrieve counts fail
+ */
+ public SyncAction decideSyncAction() throws IOException {
+ int largeThreshold = ozoneConfiguration.getInt(
+ OZONE_RECON_SCM_CONTAINER_THRESHOLD,
+ OZONE_RECON_SCM_CONTAINER_THRESHOLD_DEFAULT);
+ int perStateDriftThreshold = ozoneConfiguration.getInt(
+ OZONE_RECON_SCM_PER_STATE_DRIFT_THRESHOLD,
+ OZONE_RECON_SCM_PER_STATE_DRIFT_THRESHOLD_DEFAULT);
+
+ // --- Check 1: total container count drift ---
+ long scmTotal = scmServiceProvider.getContainerCount();
+ long reconTotal = containerManager.getContainers().size();
+ long totalDrift = Math.abs(scmTotal - reconTotal);
+
+ if (totalDrift > largeThreshold) {
+ LOG.warn("Total container drift {} exceeds threshold {} "
+ + "(SCM={}, Recon={}). Triggering full snapshot.",
+ totalDrift, largeThreshold, scmTotal, reconTotal);
+ return SyncAction.FULL_SNAPSHOT;
+ }
+ if (totalDrift > 0) {
+ LOG.info("Total container drift {} detected (SCM={}, Recon={}). "
+ + "Using targeted sync.", totalDrift, scmTotal, reconTotal);
+ return SyncAction.TARGETED_SYNC;
+ }
+
+ // --- Check 2: per-state drift (total drift = 0, lifecycle state may lag)
---
+ //
+ // These checks intentionally use the lightweight per-state count RPCs so
+ // the decision path remains cheap. CLOSED is derived as the remainder
after
+ // subtracting OPEN and QUASI_CLOSED from the total on each side.
+ long scmOpen =
scmServiceProvider.getContainerCount(HddsProtos.LifeCycleState.OPEN);
+ long scmQuasiClosed =
+
scmServiceProvider.getContainerCount(HddsProtos.LifeCycleState.QUASI_CLOSED);
+ long reconOpen = containerManager.getContainers().stream()
+ .filter(c -> c.getState() == HddsProtos.LifeCycleState.OPEN)
+ .count();
+ long reconQuasiClosed = containerManager.getContainers().stream()
Review Comment:
containerManager.getContainers() is called thrice i.e for reconTotal ,
reconOpen and reconQuasiClosed. So instead can we call it once and store it in
a variable and use that.
--
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]
