RockteMQ-AI commented on code in PR #2010:
URL:
https://github.com/apache/rocketmq-dashboard/pull/2010#discussion_r3766005175
##########
web/src/pages/ops/nameServerConfigDrift.tsx:
##########
@@ -77,42 +79,51 @@ const NameServerConfigDriftPage = () => {
return;
}
const clustersForInstance = await listClusters(firstInstanceId);
- if (cancelled) return;
+ if (cancelled || sequence !== clusterRequestSequence.current) return;
setClusters(clustersForInstance);
const firstClusterId = clustersForInstance[0]?.id;
Review Comment:
The `sequence !== clusterRequestSequence.current` guard at each await point
is the right pattern for preventing stale responses. The cleanup function also
correctly increments both sequences to invalidate any in-flight requests.
##########
web/src/pages/ops/nameServerConfigDrift.tsx:
##########
@@ -46,28 +47,29 @@ const NameServerConfigDriftPage = () => {
const runCheck = useCallback(
async (clusterId: string, instanceId: string) => {
- const sequence = ++requestSequence.current;
+ const sequence = ++checkRequestSequence.current;
Review Comment:
Good refactor — splitting `requestSequence` into `clusterRequestSequence`
and `checkRequestSequence` correctly isolates the two independent async flows
(bootstrap cluster loading vs. drift check). This prevents a stale bootstrap
response from clobbering state after the user switches instances.
##########
web/src/pages/ops/__tests__/NameServerConfigDriftPage.test.tsx:
##########
@@ -187,4 +197,58 @@ describe('NameServerConfigDriftPage', () => {
expect(await screen.findByText('暂无可检查的集群')).toBeInTheDocument();
expect(getNameServerConfigDiff).not.toHaveBeenCalled();
});
+
+ it('ignores a stale bootstrap cluster response after the user selects
another instance', async () => {
+ const user = userEvent.setup();
+ const instanceB = {
+ ...instance,
+ id: 'instance-b',
+ name: 'Backup instance',
+ };
+ const clusterB = {
+ ...cluster,
+ id: 'cluster-b',
+ name: 'Backup cluster',
+ };
+ const bootstrapClusters = deferred<ClusterInfo[]>();
Review Comment:
Well-structured test — using a deferred promise to control when the stale
bootstrap resolves, then asserting that `getNameServerConfigDiff` is never
called with the stale cluster, is a clean way to verify the race condition fix.
--
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]
