-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Refactor duplicate Nats-Msg-Id
tracking
#6174
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
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.
LGTM
5bf06fe
to
5edadf8
Compare
146c96c
to
7c25f10
Compare
Want me to take a look? |
Yes please. I did take a look at |
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.
LGTM
7c25f10
to
2ccbf43
Compare
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.
LGTM
* Swap out `ddmap` for stree (better message ID deduplication, consistent lookup times) * Remove `ddarr` and `ddindex`, replace with linked list tracking * Add unit test to prove the correct behaviour Signed-off-by: Neil Twigg <neil@nats.io>
2ccbf43
to
b7fa8cf
Compare
The current duplicate tracking has a couple of problems, namely that the
ddmap
capacity is potentially never reclaimed if it grows suddenly, and secondly that we have to constantly monitorddarr
's capacity and copy it to stop the array from growing infinitely too.This PR makes the following changes:
ddmap
is now using the stree, which means there is efficient in-place deduplication of message IDs and no infinitely growing backing capacity;ddarr
andddindex
are now gone, replaced with linked-list references andddnext
andddlast
tracking, so we also don't need to watch the capacity and reallocate this either;ddtmr
is now only scheduled when there is actually something to do.Also added a unit test to prove the behaviour as well as the linked-list stitching.
Signed-off-by: Neil Twigg neil@nats.io