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

timestamp identifier under enhance mode #146

Closed
wants to merge 1 commit into from
Closed

timestamp identifier under enhance mode #146

wants to merge 1 commit into from

Conversation

vhquang
Copy link

@vhquang vhquang commented Nov 12, 2015

This is mentioned in the README, but extra step can be done in the code to help preventing duplications are being re-sent in enhance mode.

@vhquang
Copy link
Author

vhquang commented Nov 12, 2015

#123

"""
in enhanced mode, send_notification may return error response from APNs if any
"""
if self.enhanced:
self._last_activity_time = time.time()
if identifier is None:
identifier = int(time.time() * 1e5)

Choose a reason for hiding this comment

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

There are a few conditions that could cause time.time() to return the same value for multiple invocations. Might be better to use something like random.getrandbits(32)

(+1 for this fix in-general though. Missing this detail in implementation has nasty results)

Copy link
Author

Choose a reason for hiding this comment

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

Are you thinking about multiple processes that execute this module at the same time? int(time.time()) returns epoch in seconds, and it could have the same value. But if we increase time precision to 1e5, which is 10 micro-seconds, it would be unikely, given the sending rate in the document (1 sent per 1000 micro-seconds).
I am good with using random.getrandbits(32) also. I am more curious about the condition, where timestamp would not give unique value, that I haven't thought about.

Choose a reason for hiding this comment

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

Two conditions: 1) time is changed (e.g. if the server updates/synchronizes it's time in the middle of sending pushes) and 2) low precision values for some systems. From the docs:

Note that even though the time is always returned as a floating point number, not all systems provide time with a better precision than 1 second. While this function normally returns non-decreasing values, it can return a lower value than a previous call if the system clock has been set back between the two calls.

Probably rare conditions, but you never know...

Copy link
Author

Choose a reason for hiding this comment

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

Thank you very taking time pointing that out and enlightening my knowledge! :)

Choose a reason for hiding this comment

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

No problem!

@vhquang vhquang closed this Oct 26, 2016
@vhquang vhquang deleted the duplicate-proof branch October 26, 2016 17:56
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.

2 participants