chihsuan commented on code in PR #10714:
URL: https://github.com/apache/ozone/pull/10714#discussion_r3577728086
##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/v2/pages/capacity/capacity.tsx:
##########
@@ -202,19 +201,58 @@ const Capacity: React.FC<object> = () => {
}
};
- // Adjust the polling interval based on DN scan status:
- // fast (5s) while a scan is running, normal (60s) once finished.
- // Honors the auto-reload toggle: if polling is OFF, do nothing.
+ // --- DN pending-deletion scan polling
--------------------------------------
+ // The periodic full refresh (all four endpoints) is driven by useAutoReload
+ // when Auto Refresh is enabled, and by the manual reload button otherwise.
+ // On top of that, whenever a DN scan is reported IN_PROGRESS we poll ONLY
the
+ // DN endpoint at a fast cadence until it finishes, then refetch the other
+ // endpoints once so the aggregate pending-deletion numbers reflect the final
+ // DN data. This runs regardless of the Auto Refresh toggle, because a
running
+ // scan must be driven to completion either way.
+
+ // Refetch only the DN endpoint. Kept in a ref so the interval below always
+ // reads the current `loading` value instead of a stale closure (the effect
+ // does not re-run while the status stays IN_PROGRESS).
+ const pollDnScanRef = React.useRef<() => void>(() => {});
Review Comment:
A small lint-related suggestion:
These empty callback initializers, including the similar one for
`syncNonDnDataRef` below, trigger `@typescript-eslint/no-empty-function`.
##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/__tests__/capacity/Capacity.test.tsx:
##########
@@ -307,4 +313,132 @@ describe('Capacity Page', () => {
);
expect(screen.queryByRole('option', { name: 'dn-17'
})).not.toBeInTheDocument();
});
+
+ // Interval constants mirrored from capacity.tsx / autoReload.constants.
+ const PENDING_POLL_INTERVAL = 5* 1000;
Review Comment:
nit:
```suggestion
const PENDING_POLL_INTERVAL = 5 * 1000;
```
##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/v2/pages/capacity/capacity.tsx:
##########
@@ -202,19 +201,58 @@ const Capacity: React.FC<object> = () => {
}
};
- // Adjust the polling interval based on DN scan status:
- // fast (5s) while a scan is running, normal (60s) once finished.
- // Honors the auto-reload toggle: if polling is OFF, do nothing.
+ // --- DN pending-deletion scan polling
--------------------------------------
+ // The periodic full refresh (all four endpoints) is driven by useAutoReload
+ // when Auto Refresh is enabled, and by the manual reload button otherwise.
+ // On top of that, whenever a DN scan is reported IN_PROGRESS we poll ONLY
the
+ // DN endpoint at a fast cadence until it finishes, then refetch the other
+ // endpoints once so the aggregate pending-deletion numbers reflect the final
+ // DN data. This runs regardless of the Auto Refresh toggle, because a
running
+ // scan must be driven to completion either way.
+
+ // Refetch only the DN endpoint. Kept in a ref so the interval below always
+ // reads the current `loading` value instead of a stale closure (the effect
+ // does not re-run while the status stays IN_PROGRESS).
+ const pollDnScanRef = React.useRef<() => void>(() => {});
+ pollDnScanRef.current = () => {
+ if (!dnPendingDeletes.loading) {
+ dnPendingDeletes.refetch();
+ }
+ };
+
+ // Refetch everything except the DN endpoint, to re-sync the OM / SCM /
storage
+ // numbers with the DN data once the scan has finished.
+ const syncNonDnDataRef = React.useRef<() => void>(() => {});
+ syncNonDnDataRef.current = () => {
+ storageDistribution.refetch();
+ scmPendingDeletes.refetch();
+ omPendingDeletes.refetch();
+ setState({ lastUpdated: Number(moment()) });
+ };
+
+ // True while we are actively driving a DN scan, so that we sync the other
+ // endpoints exactly once when it transitions to FINISHED — and not on the
+ // initial load, where all four endpoints were already fetched together.
+ const isDrivingDnScanRef = React.useRef(false);
+
React.useEffect(() => {
- if (!autoReload.isPolling) {
- return;
+ const scanInProgress = dnPendingDeletes.data.status === "IN_PROGRESS";
+
+ if (scanInProgress) {
+ isDrivingDnScanRef.current = true;
+ const timer = window.setInterval(() => pollDnScanRef.current(),
PENDING_POLL_INTERVAL);
+ return () => clearInterval(timer);
}
- autoReload.startPolling(
- dnPendingDeletes.data.status === "FINISHED"
- ? AUTO_RELOAD_INTERVAL_DEFAULT
- : PENDING_POLL_INTERVAL
- );
- }, [dnPendingDeletes.data.status, autoReload.isPolling]); //
eslint-disable-line react-hooks/exhaustive-deps
+
+ // Reached a terminal state (FINISHED / FAILED). If we were driving a scan,
+ // sync the other endpoints once (only on success) and stop tracking it.
+ if (isDrivingDnScanRef.current) {
Review Comment:
Thanks for updating the polling logic! One edge case I noticed:
After a polling request exhausts its retries, `useApiData` resets the data
to the default `NOT_STARTED` status. That would clear this interval, and with
Auto Refresh off, the DN sections could remain loading until the user manually
refreshes.
Would it make sense to keep a recovery path for `NOT_STARTED` or request
failures, and stop polling only for explicitly handled terminal states?
--
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]
