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

nsqd: dont retain client pointer for inflight msgs #242

Merged
merged 1 commit into from
Aug 5, 2013

Conversation

mreiferson
Copy link
Member

see commit comments cc @jehiah

@@ -27,6 +27,7 @@ type NSQd struct {
sync.RWMutex
options *nsqdOptions
workerId int64
clientID int64
Copy link
Member

Choose a reason for hiding this comment

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

perhaps this should be clientIDSequence or clientIDCounter or something along those lines?

Copy link
Member Author

Choose a reason for hiding this comment

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

yep

@jehiah
Copy link
Member

jehiah commented Aug 3, 2013

LGTM. rebase please.

@mreiferson
Copy link
Member Author

comments addressed @jehiah

more cleanup relating to what pointers might be retained
preventing proper GC.

this changes the in-flight messages struct so that it
uses an int64 ID rather than a pointer to ensure
that only the original receiver of a message can
respond to it.  this required changes to the channel
to store clients as a map[int64]Consumer in order
to lookup the proper client by ID.
jehiah added a commit that referenced this pull request Aug 5, 2013
nsqd: dont retain client pointer for inflight msgs
@jehiah jehiah merged commit c67c43d into nsqio:master Aug 5, 2013
mreiferson added a commit to mreiferson/nsq that referenced this pull request Aug 11, 2013
mreiferson added a commit to mreiferson/nsq that referenced this pull request Aug 11, 2013
mreiferson added a commit to mreiferson/nsq that referenced this pull request Aug 11, 2013
mreiferson added a commit to mreiferson/nsq that referenced this pull request Aug 11, 2013
mreiferson added a commit to mreiferson/nsq that referenced this pull request Aug 11, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants