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

add a new property 'flushBatchSize' for fine tuning the network request #617

Merged
merged 3 commits into from
Nov 13, 2023

Conversation

zihejia
Copy link
Contributor

@zihejia zihejia commented Nov 10, 2023

Add the following property flushBatchSize:

The flushBatchSize property controls the number of events sent in a single network request to the Mixpanel server.
Batch size affects how data is flushed from the client to Mixpanel. By configuring the size of the batch, you can
optimize network usage and control how frequently the client communicates with the server.

Usage:

mixpanel.flushBatchSize = 20

@zihejia zihejia changed the title add new property 'flushBatchSize' for fine tuning the network request add a new property 'flushBatchSize' for fine tuning the network request Nov 10, 2023
@@ -119,6 +119,19 @@ open class MixpanelInstance: CustomDebugStringConvertible, FlushDelegate, AEDele
}
}

/// The flushBatchSize property controls the number of events sent in a single network request to the Mixpanel server.
/// Batch size affects how data is flushed from the client to Mixpanel. By configuring the size of the batch, you can
/// optimize network usage and control how frequently the client communicates with the server.
Copy link
Contributor

Choose a reason for hiding this comment

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

/track has a maximum batch size of 50: https://developer.mixpanel.com/reference/track-event#limits

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch!

Copy link
Contributor

@jaredmixpanel jaredmixpanel left a comment

Choose a reason for hiding this comment

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

LGTM but we should enforce the max batch size of 50 for the /track endpoint

@zihejia zihejia merged commit c9ac4c2 into master Nov 13, 2023
2 checks passed
@RamblinWreck77
Copy link
Contributor

RamblinWreck77 commented Nov 13, 2023

@jaredmixpanel Sorry to necro a merged MR, but am I reading this correctly?

Screenshot 2023-11-13 at 5 32 17 PM

It appears to me that a call to flush(performFullFlush: true) would attempt an upload with Int.max batch size, bypassing the flushBatchSize limit imposed by the API? What does the API do if you send it more than 50? Drop events?

I guess it makes sense that a "full" flush will send everything, but I don't think anyone would want that if it resulted in lost events.

@zihejia
Copy link
Contributor Author

zihejia commented Nov 13, 2023

@jaredmixpanel Sorry to necro a merged MR, but am I reading this correctly?

Screenshot 2023-11-13 at 5 32 17 PM

It appears to me that a call to flush(performFullFlush: true) would attempt an upload with Int.max batch size, bypassing the flushBatchSize limit imposed by the API? What does the API do if you send it more than 50? Drop events?

I guess it makes sense that a "full" flush will send everything, but I don't think anyone would want that if it resulted in lost events.

Hi @RamblinWreck77, if you set the batch size to more than 50, it won't drop events but just fall back to 50. perhaps we should make it clearer in the doc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants