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

api: add events subscription support #230

Merged
merged 4 commits into from
Dec 7, 2022

Conversation

oleg-jukovec
Copy link
Collaborator

@oleg-jukovec oleg-jukovec commented Nov 17, 2022

A user can create watcher by the Connection.NewWatcher() call:

    watcher = conn.NewWatcker("key", func(event WatchEvent) {
        // The callback code.
    })

After that, the watcher callback is invoked for the first time. In this case, the callback is triggered whether or not the key has already been broadcast. All subsequent invocations are triggered with box.broadcast() called on the remote host. If a watcher is subscribed for a key that has not been broadcast yet, the callback is triggered only once, after the registration of the watcher.

If the key is updated while the watcher callback is running, the callback will be invoked again with the latest value as soon as it returns.

Multiple watchers can be created for one key.

If you don’t need the watcher anymore, you can unregister it using the Unregister method:

    watcher.Unregister()

The api is similar to net.box implementation [1].

It also adds a BroadcastRequest to make it easier to send broadcast messages.

  1. https://www.tarantool.io/en/doc/latest/reference/reference_lua/net_box/#conn-watch

I didn't forget about (remove if it is not applicable):

Related issues:

Closes #119

@oleg-jukovec oleg-jukovec force-pushed the oleg-jukovec/gh-119-watchers-support branch 6 times, most recently from dd9a1fa to 22455f6 Compare November 22, 2022 11:44
@oleg-jukovec
Copy link
Collaborator Author

oleg-jukovec commented Nov 22, 2022

There are few things to do after #226 will be merged into master:

  1. Rebase Connection.writeRequest() implementation in connection.go .
  2. Add feature check into Connection.NewWatcher() call (I left TODO in the place).
  3. Add feature check into ConnectionPool.NewWatcher() call (I left TODO in the place).

But it doesn't look critical and can be done quickly. This will not affect the idea of the pull request.

@oleg-jukovec oleg-jukovec marked this pull request as ready for review November 22, 2022 12:09
@oleg-jukovec oleg-jukovec force-pushed the oleg-jukovec/gh-119-watchers-support branch 3 times, most recently from dafb5e1 to 401de6d Compare November 22, 2022 15:55
@DifferentialOrange
Copy link
Member

  1. https://www.tarantool.io/ru/doc/latest/reference/reference_lua/net_box/#conn-watch

It is better to refer to /en/ documentation.

@oleg-jukovec oleg-jukovec force-pushed the oleg-jukovec/gh-119-watchers-support branch from 401de6d to 3240a5a Compare November 23, 2022 11:34
Copy link
Member

@DifferentialOrange DifferentialOrange left a comment

Choose a reason for hiding this comment

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

I've try to review it one more time later. It's rather hard to catch on all these concurrency details from the first time (especially if you're not familiar with them).

connection.go Outdated Show resolved Hide resolved
connection.go Show resolved Hide resolved
connection.go Show resolved Hide resolved
connection.go Outdated Show resolved Hide resolved
connection.go Outdated Show resolved Hide resolved
connection.go Show resolved Hide resolved
connection.go Show resolved Hide resolved
tarantool_test.go Show resolved Hide resolved
tarantool_test.go Outdated Show resolved Hide resolved
connection.go Outdated Show resolved Hide resolved
@oleg-jukovec
Copy link
Collaborator Author

oleg-jukovec commented Nov 23, 2022

I've try to review it one more time later. It's rather hard to catch on all these concurrency details from the first time (especially if you're not familiar with them).

Thank you! I could recommend to review in this order:

  1. Connection.
  2. ConnectionMulti.
  3. ConnectionPool.

It would be possible to split the commit into several (Connection, ConnectionMulti, ConnectionPool, Connector), but it look like a single feature, so I did not do it.

@oleg-jukovec oleg-jukovec force-pushed the oleg-jukovec/gh-119-watchers-support branch from 3240a5a to 2725aa3 Compare November 24, 2022 07:28
Copy link

@better0fdead better0fdead left a comment

Choose a reason for hiding this comment

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

Hello, thx for patch. I am generally ok, just some small comments.

connection_pool/round_robin.go Show resolved Hide resolved
connection.go Show resolved Hide resolved
@oleg-jukovec oleg-jukovec force-pushed the oleg-jukovec/gh-119-watchers-support branch 3 times, most recently from 672e012 to f3691d1 Compare November 24, 2022 14:27
@DifferentialOrange
Copy link
Member

DifferentialOrange commented Nov 29, 2022

If a watcher is subscribed for a key that has not been broadcast yet, the callback is triggered only once

I'm still a bit confused. If a watcher subscribed for a key that has been broadcasted before the subscription, will callback be triggered twice? If yes, with which values? Or will it be triggered for each historical value in their order? Or will it trigger only once for the current key value? If yes, I don't get what "If a watcher is subscribed for a key that has not been broadcast yet, the callback is triggered only once" should mean since they all are triggered only once on bootstrap.

@oleg-jukovec
Copy link
Collaborator Author

Or will it trigger only once for the current key value? If yes, I don't get what "If a watcher is subscribed for a key that has not been broadcast yet, the callback is triggered only once" should mean since they all are triggered only once on bootstrap.

It is copy-paste from: https://www.tarantool.io/ru/doc/latest/reference/reference_lua/box_events/#how-a-watcher-works

I think I need to look at the whole paragraph. The last sentence explains what will happen if there are no broadcasts yet. But I can delete the sentence =)

connection.go Outdated Show resolved Hide resolved
connection.go Show resolved Hide resolved
connection.go Show resolved Hide resolved
connection.go Outdated Show resolved Hide resolved
connection.go Outdated Show resolved Hide resolved
connection.go Outdated Show resolved Hide resolved
connection.go Outdated Show resolved Hide resolved
watch.go Outdated Show resolved Hide resolved
connection.go Outdated Show resolved Hide resolved
connection.go Outdated Show resolved Hide resolved
@oleg-jukovec oleg-jukovec force-pushed the oleg-jukovec/gh-119-watchers-support branch 4 times, most recently from 6620388 to 4662c58 Compare November 30, 2022 12:31
@oleg-jukovec oleg-jukovec force-pushed the oleg-jukovec/gh-119-watchers-support branch 5 times, most recently from f4b4d71 to 706b3e6 Compare November 30, 2022 15:31
connection.go Show resolved Hide resolved
connection.go Outdated Show resolved Hide resolved
connection.go Outdated Show resolved Hide resolved
connection.go Outdated Show resolved Hide resolved
@oleg-jukovec oleg-jukovec force-pushed the oleg-jukovec/gh-119-watchers-support branch from 706b3e6 to a719437 Compare December 5, 2022 08:40
CHANGELOG.md Show resolved Hide resolved
test_helpers/utils.go Outdated Show resolved Hide resolved
test_helpers/utils.go Outdated Show resolved Hide resolved
protocol.go Show resolved Hide resolved
connection.go Outdated Show resolved Hide resolved
connection.go Outdated Show resolved Hide resolved
connection.go Show resolved Hide resolved
connection.go Outdated Show resolved Hide resolved
connection.go Show resolved Hide resolved
tarantool_test.go Outdated Show resolved Hide resolved
@oleg-jukovec oleg-jukovec force-pushed the oleg-jukovec/gh-119-watchers-support branch 3 times, most recently from cda1f1a to c3e2254 Compare December 5, 2022 12:00
connection.go Show resolved Hide resolved
tarantool_test.go Outdated Show resolved Hide resolved
connection_pool/connection_pool.go Outdated Show resolved Hide resolved
@oleg-jukovec oleg-jukovec force-pushed the oleg-jukovec/gh-119-watchers-support branch 2 times, most recently from e496d79 to c3925d0 Compare December 6, 2022 15:59
Due to a bug [1], our test workflow fails on Tarantool 1.10. It is
a quick fix until the problem is fixed.

1. tarantool/setup-tarantool#37
The patch removes a last newline character from log messages because
the character will be added anyway [1][2].

1. https://pkg.go.dev/log#Logger.Printf
2. https://pkg.go.dev/log#Output

Part of #119
A user can create watcher by the Connection.NewWatcher() call:

    watcher = conn.NewWatcker("key", func(event WatchEvent) {
        // The callback code.
    })

After that, the watcher callback is invoked for the first time. In
this case, the callback is triggered whether or not the key has
already been broadcast. All subsequent invocations are triggered with
box.broadcast() called on the remote host. If a watcher is subscribed
for a key that has not been broadcast yet, the callback is triggered
only once, after the registration of the watcher.

If the key is updated while the watcher callback is running, the
callback will be invoked again with the latest value as soon as it
returns.

Multiple watchers can be created for one key.

If you don’t need the watcher anymore, you can unregister it using
the Unregister method:

    watcher.Unregister()

The api is similar to net.box implementation [1].

It also adds a BroadcastRequest to make it easier to send broadcast
messages.

1. https://www.tarantool.io/en/doc/latest/reference/reference_lua/net_box/#conn-watch

Closes #119
@oleg-jukovec oleg-jukovec force-pushed the oleg-jukovec/gh-119-watchers-support branch from c3925d0 to bc79947 Compare December 7, 2022 07:46
@oleg-jukovec oleg-jukovec merged commit 265f96c into master Dec 7, 2022
@oleg-jukovec oleg-jukovec deleted the oleg-jukovec/gh-119-watchers-support branch December 7, 2022 08:11
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.

Support watchers (subscription to server events)
3 participants