-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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.
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
Allow to configure ack-timeout tick time #4760
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Look good to me +1
10 sec by default is ok, if user use a short ack timeout (less than 10s), it’s better to use ack timeout as the tick time. means ack timeout always >= tick time.
run java8 tests
|
run java8 tests |
@codelipenghui Another option I was thinking of, was to just keep a fixed number of time buckets, say like 16 (non configurable). That will automatically tie the precision the order of magnitude of the ack timeout. |
I think it can be used as the default configuration, but it's better for users to configure it. |
### Motivation After the changes in apache#3118, there has a been a sharp increase of memory utilization for the UnackedMessageTracker due to the time buckets being created. This is especially true when the acktimeout is set to a larger value (eg: 1h) where 3600 time-buckets are being created. This lead to use 20MB per partition even when no message is tracked. Allowing to configure the tick time so that application can tune it based on needs. Additionally, fixed the logic that keeps creating hash maps and throwing them away at each tick time iteration, since that creates a lot of garbage and doesn't take care of the fact that the hash maps are expanding based on the required capacity (so next time they are already of the "right" size). On a final note: the current default of 1sec seems very wasteful. Something like 10s should be more appropriate as default.
### Motivation After the changes in #3118, there has a been a sharp increase of memory utilization for the UnackedMessageTracker due to the time buckets being created. This is especially true when the acktimeout is set to a larger value (eg: 1h) where 3600 time-buckets are being created. This lead to use 20MB per partition even when no message is tracked. Allowing to configure the tick time so that application can tune it based on needs. Additionally, fixed the logic that keeps creating hash maps and throwing them away at each tick time iteration, since that creates a lot of garbage and doesn't take care of the fact that the hash maps are expanding based on the required capacity (so next time they are already of the "right" size). On a final note: the current default of 1sec seems very wasteful. Something like 10s should be more appropriate as default. (cherry picked from commit f13af48)
Motivation
After the changes in #3118, there has a been a sharp increase of memory utilization for the UnackedMessageTracker due to the time buckets being created.
This is especially true when the acktimeout is set to a larger value (eg: 1h) where 3600 time-buckets are being created. This lead to use 20MB per partition even when no message is tracked.
Allowing to configure the tick time so that application can tune it based on needs.
Additionally, fixed the logic that keeps creating hash maps and throwing them away at each tick time iteration, since that creates a lot of garbage and doesn't take care of the fact that the hash maps are expanding based on the required capacity (so next time they are already of the "right" size).
On a final note: the current default of 1sec seems very wasteful. Something like 10s should be more appropriate as default.