Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

FailbackClusterInvoker should limit the size of failed invocations #2425

Closed
ralf0131 opened this issue Sep 2, 2018 · 3 comments
Closed

FailbackClusterInvoker should limit the size of failed invocations #2425

ralf0131 opened this issue Sep 2, 2018 · 3 comments
Assignees
@carryxyh

Comments

@ralf0131
Copy link
Contributor

ralf0131 commented Sep 2, 2018 •
edited
Loading

Currently in FailbackClusterInvoker there is a map to store the failed invocations, however the size is unlimited.

In a high throughput application, It is possible that the failed invocations will fill up the memory and exhaust the memory.

I think it is better to:

  • limit the size of the map
  • choose the strategy to discard the failed invocations. e.g. FIFO, LIFO, etc.
  • retry period should be configurable
  • shutdown the ExecutorService when destroyed
@Jeff-Lv
Copy link
Contributor

Jeff-Lv commented Sep 14, 2018

Agreed, this maybe cause a risk of memory leak.

@carryxyh carryxyh assigned kimmking and unassigned kimmking Sep 28, 2018
@carryxyh
Copy link
Member

This issue has been prevented for some time and I will start to solve this problem in the near future.

@carryxyh carryxyh self-assigned this Oct 12, 2018
@wangweiufofly wangweiufofly mentioned this issue Nov 23, 2018
Merged
carryxyh pushed a commit that referenced this issue Dec 25, 2018
@wangweiufofly @carryxyh
Fix FailbackClusterInvoker one risk of memory leak #2425 (#2822)
669705c 
1. limit the size of the map,default is 1000. can config by key failcapacity
2. retry period,default is 100.can config by key retries.
3. shutdown the timer when destroyed
@carryxyh
Copy link
Member

Close via #2822

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@carryxyh carryxyh

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@kimmking @ralf0131 @Jeff-Lv @carryxyh