Skip to content

[ISSUE #10244] Trigger MemoryConsumerOrderInfoManager autoClean periodically#10272

Closed
kobe-yang wants to merge 1 commit intoapache:developfrom
go-go-go-kobeyang:fix-10244-memory-consumer-order-info-clean
Closed

[ISSUE #10244] Trigger MemoryConsumerOrderInfoManager autoClean periodically#10272
kobe-yang wants to merge 1 commit intoapache:developfrom
go-go-go-kobeyang:fix-10244-memory-consumer-order-info-clean

Conversation

@kobe-yang
Copy link
Copy Markdown

Summary

This PR fixes the missing periodic cleanup for MemoryConsumerOrderInfoManager.

Previously, the cleanup logic already existed in autoClean(), including the case where the subscription group no longer exists, but it was not triggered periodically for the in-memory manager path.

Changes

  • schedule periodic cleanup for memory consumer order info in broker periodic tasks
  • expose a cleanup entry from PopLiteMessageProcessor
  • keep PopLiteLockManager focused on lock timeout cleanup
  • add unit coverage for the missing-group cleanup case

Why

The issue is not in the cleanup rule itself, but in the missing trigger path for MemoryConsumerOrderInfoManager.autoClean().

Tests

  • git diff --check
  • mvn -pl broker -am -Dmaven.repo.local=/tmp/rocketmq-m2 -Dtest=ConsumerOrderInfoManagerTest -DfailIfNoTests=false -Dsurefire.failIfNoSpecifiedTests=false compiler:compile compiler:testCompile surefire:test

@kobe-yang
Copy link
Copy Markdown
Author

Hi maintainers, this is my first contribution here. Could you please help approve the workflows so the required checks can start running? Thanks!

@qianye1001
Copy link
Copy Markdown
Contributor

Thanks for the contribution. A couple of concerns after reviewing the code:

  1. autoClean() already handles missing groups. The existing PopLiteLockManager already calls autoClean() every 5 minutes, and the autoClean() implementation in QueueLevelConsumerManager already removes entries when containsSubscriptionGroup(group) returns false. So the cleanup for deleted groups is already working as expected — the issue described in [Bug] Clean MemoryConsumerOrderInfo if group does not exist #10244 does not seem to exist in practice.

  2. Potential concurrency issue. Before this PR, both lockService.removeTimeout() and autoClean() ran sequentially in the same PopLiteLockManager ServiceThread. After this PR, autoClean() is moved to BrokerController.scheduledExecutorService while removeTimeout() stays in the PopLiteLockManager thread. This means they now execute concurrently on different threads, which could introduce race conditions between lock timeout cleanup and order info cleanup operating on related data structures simultaneously.

@kobe-yang
Copy link
Copy Markdown
Author

Thanks for the review and for pointing this out.

After re-checking the current implementation, I agree that autoClean() is already triggered periodically by PopLiteLockManager, and the missing-group case is already handled in QueueLevelConsumerManager.autoClean().

That means this PR does not fix a real gap, and the scheduling change indeed introduces unnecessary concurrency risk. I will close this PR.

Thanks again for the careful review.

@kobe-yang kobe-yang closed this Apr 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants