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.

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

fix(inflight): reduce shrinking limit #306

Merged
merged 1 commit into from
May 10, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion inflight/inflight.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import (
// We track inflights in the map, maps in golang are not shrinking
// Therefore we track how many inflights were deleted and when it reaches the limit
// we forcefully recreate the map to shrink it
const shrinkInflightsLimit = 1000000
const shrinkInflightsLimit = 1000
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why 1000?
Do we know the cost of map recreation?

Copy link
Collaborator Author

@dkropachev dkropachev May 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just guesstimate.

This is limit of deletions to trigger recreation, recreation it self depends on the number of items in the map, which is limited only by number partition size, which is Uint64.Max/partitionCount usually it is 18446744073709551615/10000 = 1844674407370955.
With current implementation this limit is pretty much reachable, which is source of memory consumption.
But after #309 and #307 are fixed we should not see more than pkBufferReuseSize + concurrency, which is usually 100+10 infligths per partition.


type InFlight interface {
AddIfNotPresent(uint64) bool
Expand Down