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

notification issues with latest release #1547

Closed
1 of 2 tasks
Tostino opened this issue Aug 23, 2019 · 5 comments
Closed
1 of 2 tasks

notification issues with latest release #1547

Tostino opened this issue Aug 23, 2019 · 5 comments

Comments

@Tostino
Copy link
Contributor

@Tostino Tostino commented Aug 23, 2019

  • bug report
  • feature request

Describe the issue
Notifications are not all delivered at once when calling getNotifications() unless you execute a query beforehand.

I believe it is related to the change in commit: e2623d6 in PGStream in hasMessagePending()

It will add the first notification to the list to be returned, then since the nextStreamAvailableCheckTime gets set to 1s in the future in the prior call, the next call immediately after to get any remaining notifications returns false, even though there are notifications pending.

Driver Version?
42.2.6
Java Version?
8
OS Version?
Windows / Linux
PostgreSQL Version?
10
To Reproduce
If I were to open a connection listening on a channel, then in another process execute NOTIFY 30 times in quick succession for that channel, only one notification will be returned per call to getNotifications() per second.

For example, with a loop calling getNotifications() every 100ms, it would return a single notification every 10 calls, and would take 30 seconds of wall-clock time, and 300 calls of getNotifications() to process the 30 notifications which were queued up.

Running a simple query (SELECT 1;) beforehand will allow all notifications to be received and processed by a single call to getNotifications().

Expected behaviour
All prior notifications to the channel should be returned if a user calls getNotifications().

@davecramer
Copy link
Member

@davecramer davecramer commented Aug 23, 2019

@Tostino OK, I think I have a solution. If peek returns anything then we will not reset the timer. Only reset if it returns -1

Loading

@Tostino
Copy link
Contributor Author

@Tostino Tostino commented Aug 23, 2019

I think that sounds like the right solution, and should still solve the original problem.

Loading

@davecramer
Copy link
Member

@davecramer davecramer commented Aug 23, 2019

can you test #1548

Loading

@Tostino
Copy link
Contributor Author

@Tostino Tostino commented Aug 26, 2019

That looks like it's working just fine for me. Thanks for the effort here!

Loading

@davecramer
Copy link
Member

@davecramer davecramer commented Aug 28, 2019

Fixed in #1548

Loading

@davecramer davecramer closed this Aug 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants