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

Consistently use clock_timestamp() instead of now() when working with vt #367

Open
brianpursley opened this issue Dec 20, 2024 · 2 comments

Comments

@brianpursley
Copy link
Contributor

Most of the time, pgmq uses clock_timestamp() when setting or comparing vt. This is the case for the following functions:

  • read
  • read_with_poll
  • send
  • send_batch

But these functions use now():

Is there a reason for the difference?

Depending on which is used, the behavior can be different when used inside a transaction, which might be unexpected. I ran into this while testing something and it surprised me until I looked at the code.

Here is an example showing what I'm talking about:
Inside a transaction, send a message to the queue. You can't pop the message, but you can read it.

postgres=# begin transaction;
BEGIN
postgres=*# select pgmq.send('my_queue', '{"foo": "bar1"}');
 send 
------
   38
(1 row)

postgres=*# select pgmq.pop('my_queue');
 pop 
-----
(0 rows)

postgres=*# select pgmq.read('my_queue', 30, 1);
                                             read                                             
----------------------------------------------------------------------------------------------
 (37,2,"2024-12-20 19:44:53.600822+00","2024-12-20 19:46:10.624711+00","{""foo"": ""bar1""}")
(1 row)

postgres=*# commit;
COMMIT

If there isn't a reason for them to be different, I think pop and set_vt should use clock_timestamp(), to be consistent with the other functions.

@ChuckHend
Copy link
Member

ChuckHend commented Dec 26, 2024

I don't think there's a reason that they are different other than the project starter using now() and we changed some of the functions as we were updating the codebase. We should be able to update pop() and set_vt() too.

@v0idpwn
Copy link
Collaborator

v0idpwn commented Dec 26, 2024

PRs welcome :)

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

No branches or pull requests

3 participants