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

Separate network queue #626

Merged
merged 4 commits into from Jan 12, 2017
Merged

Separate network queue #626

merged 4 commits into from Jan 12, 2017

Conversation

pchien
Copy link
Contributor

@pchien pchien commented Dec 23, 2016

here's my notes:

goal: non-network and network happen in parallel
non-network puts stuff in eventsQueue and peopleQueue
flush takes stuff out of eventsQueue and peopleQueue
need to lock on adding, removing, reading
need to lock on saving to disk (reading actually only happens on init so should be okay though maybe should do it anyway)
as it stands flush will flush whatever was added before. naively changing makes it so it will continue flushing as long as stuff keeps getting added in. should read all stuff at start to not have endless flushing
reset needs to wait on flushes as well
flush and other network queue stuff should be dispatched from the serial queue so the pattern of track -> flush always has that track in the flush instead of only sometimes… so everything on the network queue is dispatched through the serial queue…

Peter Chien added 2 commits December 22, 2016 15:23
…ther stuff on the serial queue don't get blocked if the network requests take a long time
[strongMixpanel.peopleQueue addObject:r];
if (strongMixpanel.peopleQueue.count > 500) {
[strongMixpanel.peopleQueue removeObjectAtIndex:0];
@synchronized (self) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

i think this should be synchronized on strongMixpanel

…blocks. fix bug on synchronizing on wrong thing in MixpanelPeople
@yarneo
Copy link
Contributor

yarneo commented Jan 12, 2017

LGTM just fix conflicts

@pchien pchien merged commit c7a75db into master Jan 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants