Skip to content

Conversation

@andrewprofile
Copy link

No description provided.

@makasim
Copy link
Member

makasim commented Jun 14, 2022

I am not sure I follow it. Can you please walk me through your thought process?

@andrewprofile
Copy link
Author

andrewprofile commented Jun 15, 2022

@makasim I described the problem in issue #1257 :)

microtime return value in seconds so the conversion to timestamp is invalid, ex : 16551970635236 -> GMT: Monday, 5 July 2494 17:37:15.236

@Steveb-p
Copy link
Contributor

Steveb-p commented Jun 15, 2022

@makasim I described the problem in issue #1257 :)

microtime return value in seconds so the conversion to timestamp is invalid, ex : 16551970635236 -> GMT: Monday, 5 July 2494 17:37:15.236

The issue here is that without multiplication - that you're removing - precision of stored data suffers. Messages become stored with at most 1 second precision, which makes it impossible to maintain their order & microtime() becomes equivalent to time().

I think the issue you're describing is actually valid, but not because of the code. It's because we've called this property "timestamp" while we intended to store a more precise value.

a.k.a. naming is hard 😅

EDIT: Actually, where did you find that this value is supposed to be a timestamp? The class that this code is contained with specificly uses DbalType::INTEGER (which, btw, may not be long enough @makasim and a conversion to BIGINT might be considered)

@andrewprofile
Copy link
Author

andrewprofile commented Jun 15, 2022

@Steveb-p Sorry it's my mistake. The property published_at suggests that it will be the date, but since you operate everywhere on timestamps, I assumed that in this case also to be able to get a specific date from it. But I already understand that this is not the point, but then there is some inconsistency because other values do not operate in such precision.

Closing

@Steveb-p
Copy link
Contributor

@Steveb-p Sorry it's my mistake. The property published_at suggests that it will be the date, but since you operate everywhere on timestamps, I assumed that in this case also to be able to get a specific date from it. But I already understand that this is not the point, but then there is some inconsistency because other values do not operate in such precision.

If you find any discrepancy in this or other transports feel free to open an issue or discussion (which, btw, could be enabled @makasim, so issues don't get polluted by questions). I would not outright dismiss the notion that there might be differences 😅

@makasim
Copy link
Member

makasim commented Jun 15, 2022

@Steveb-p Enabled discussions https://github.com/php-enqueue/enqueue-dev/discussions

@andrewprofile
Copy link
Author

@Steveb-p I meant redeliverAfter or timeToLive properties, that's why I pointed it out and assumed it was an issue :)

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.

3 participants