hani-fouladgar commented on code in PR #11423:
URL: https://github.com/apache/ozone/pull/11423#discussion_r4391657890
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/StateContext.java:
##########
@@ -375,16 +375,40 @@ public void putBackReports(List<Message> reportsToPutBack,
*/
public List<Message> getAllAvailableReports(
HostAndPort endpoint) {
- int maxLimit = Integer.MAX_VALUE;
- // TODO: It is highly unlikely that we will reach maxLimit for the number
- // for the number of reports, specially as it does not apply to the
- // number of entries in a report. But if maxLimit is hit, should a
- // heartbeat be scheduled ASAP? Should full reports not included be
- // dropped? Currently this code will keep the full reports not sent
- // and include it in the next heartbeat.
+ return getAllAvailableReportsUpToLimit(endpoint, Integer.MAX_VALUE);
+ }
+
+ /**
+ * Returns up to {@code maxLimit} available reports for the endpoint,
+ * removing them from the queue. Any reports beyond the limit stay queued
+ * and are returned by a later call; callers can use {@link
+ * #hasPendingReports(HostAndPort)} to detect that case and schedule a
+ * follow-up heartbeat.
+ *
+ * @return List of reports
+ */
+ public List<Message> getAllAvailableReports(
+ HostAndPort endpoint, int maxLimit) {
return getAllAvailableReportsUpToLimit(endpoint, maxLimit);
}
+ /**
+ * Returns true if any reports are still queued for the endpoint.
+ */
+ public boolean hasPendingReports(HostAndPort endpoint) {
+ synchronized (incrementalReportsQueue) {
+ List<Message> reportsForEndpoint = incrementalReportsQueue.get(endpoint);
+ if (reportsForEndpoint != null && !reportsForEndpoint.isEmpty()) {
+ return true;
+ }
+ }
+ Map<String, AtomicBoolean> fullReports =
Review Comment:
Good callout — since SCM overwrites replica state blindly (no stale/older
guard in `AbstractContainerReportHandler.updateContainerReplica`), correctness
depends entirely on apply order. The cap doesn't change that order:
- Datanode: `getAllAvailableReportsUpToLimit` always collects full reports
before ICRs, and ICRs drain FIFO — so a split sends the FCR no later than the
ICRs, in order. And `getFullContainerReportDiscardPendingICR` discards pending
ICRs when it snapshots the FCR, so no ICR older than an FCR is ever left to
trail it.
- SCM: `SCMDatanodeHeartbeatDispatcher` fires FCR before ICR within a
heartbeat, and a DN's reports run on a single affinity thread in arrival order
— no reordering.
So the cap only changes how many reports per heartbeat, never the
FCR→newer-ICR order; the worst case is a brief staleness between the FCR and
the trailing ICRs, not a regression. On deletes: ContainerReportHandler
reconciles the FCR by absence (processMissingReplicas), so the FCR conveys
deletions — the existing hourly discard already drops delete ICRs and relies on
this.
--
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]
