HDDS-15453. [Recon] Refresh all capacity APIs on auto refresh#10420
HDDS-15453. [Recon] Refresh all capacity APIs on auto refresh#10420priyeshkaratha wants to merge 2 commits into
Conversation
|
@spacemonkd @devmadhuu Can you please review the code? |
spacemonkd
left a comment
There was a problem hiding this comment.
Thanks for this change @priyeshkaratha.
LGTM, +1
|
@priyeshkaratha thanks for the patch. It looks like we are shifting the problem from one place to another. Auto refresh may make user to continuously see "waiting for results" when refresh will happen automatically and between 1st and 2nd refresh, one collection was completed and 2nd was happening, but removing auto refresh and keeping just manual refresh will not solve the issue either. How user will know when to refresh to see the actual collected data or we expect user to keep clicking refresh button which is not a good experience. IMO, we should have our UI polling mechanism and based on a flag denotes completion, results should be auto refreshed on UI. |
|
@devmadhuu Thanks for reviewing the changes. Looks PR description seems confusing. The backend polling mechanism will still remain in place. This PR only removes the auto-refresh toggle button from the UI. If we keep the toggle enabled now, it will not perform any action, which may create confusion and lead users to believe that the feature is not working. Therefore, it is better to remove the toggle option from the UI. I also updated the PR description. |
51d47db to
5f550b9
Compare
@priyeshkaratha So as discussed offline, keep the behavior of page consistent in terms of refresh of page because Pending Deletion card also there on page. So better when UI already polling and continue to poll till refresh of page by calling respective APIs once they finish their execution, we should also call Pending Deletion APIs and refresh their data on the page. |
@devmadhuu I changed implementation with auto refresh option only. The workflow is as follows:
These APIs return results immediately.
|
devmadhuu
left a comment
There was a problem hiding this comment.
Thanks @priyeshkaratha for updating the patch. Pls see the comments inline.
|
|
||
| const intervalId = window.setInterval(() => { | ||
| dnPendingDeletes.refetch(); | ||
| }, PENDING_POLL_INTERVAL); |
There was a problem hiding this comment.
Here, Replace the whole standalone setInterval effect with a tiny interval-selector
const autoReload = useAutoReload(loadData); // default 60s
// One timer for everything: poll fast while a DN scan is running,
// fall back to the normal interval once it finishes.
React.useEffect(() => {
if (!autoReload.isPolling) {
return; // toggle is OFF — honor it for ALL refreshing
}
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
There was a problem hiding this comment.
Yes, this makes sense. Updated PR with these changes.
What changes were proposed in this pull request?
Use the Auto Refresh toggle to reload all capacity APIs on the default 60s interval. Poll the DN pending deletion API separately every 5s until the scan status is FINISHED, so fast DN updates no longer override full-page refresh behavior.
The earlier concern was regarding whether auto refresh cause unnecessary API calls for long running tasks. But additionally, there is a 30-second delay between metric collection cycles. After a collection completes, the next collection will not start for at least 30 seconds. This ensures that users can view stable results for a reasonable period before a new collection begins.
What is the link to the Apache JIRA
HDDS-15453
How was this patch tested?
Tested manually