Skip to content

iproto: support feature push #247

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

Merged
merged 1 commit into from
Nov 3, 2022

Conversation

GRISHNOV
Copy link
Contributor

@GRISHNOV GRISHNOV commented Oct 19, 2022

Adds support for receiving out-of-band messages from a server that uses box.session.push call.

Implemented for methods: call, eval, select, insert, replace, update, upsert, delete.
To work with out-of-band messages, 2 new optional arguments are used:

  • on_push [function] - callback, launched with the received data for each out-of-band message. Two arguments for this callback are expected:
    • the first is the received from an out-of-band message data.
    • the second is on_push_ctx, variable for recording the result of the callback work.
  • on_push_ctx - result of the on_push work can be written to this variable.

Below is an example of the proposed API with method call and insert. In the described example, before the end of the call and insert, out-of-band messages are processed via specified callback.

For comparison: implementation in the lua version.

Client Server
fiber = require('fiber')
box.cfg({listen = 3301})
box.schema.user.grant(
    'guest',
    'read,write,execute',
    'universe'
)
function server_function()
    x = {0,0}
    while x[1] < 3 do
        x[1] = x[1] + 1
        fiber.sleep(1)
        box.session.push(x)
    end
    fiber.sleep(1)
    return x
end
import tarantool

def callback(data, on_push_ctx=[]): print('run callback with data: ', data) data[0][1] = data[0][1] + 1 on_push_ctx.append(data)
callback_res = []
conn = tarantool.connect(port=3301) res = conn.call( 'server_function', on_push=callback, on_push_ctx=callback_res ) # receiving out-of-band messages, # the conn.call is not finished yet. >>> run callback with data: [[1, 0]] >>> run callback with data: [[2, 0]] >>> run callback with data: [[3, 0]] # the conn.call is finished now.
print(res) >>> [3, 0]
print(callback_res) >>>[[[1, 1]], [[2, 1]], [[3, 1]]]
box.schema.create_space(
 'tester', {
  format = {
    {name = 'id', type = 'unsigned'},
    {name = 'name', type = 'string'},
  }
})
box.space.tester:create_index(
 'primary_index', {
  parts = {
    {field = 1, type = 'unsigned'},
  }
})
function on_replace_callback()
    x = {0,0}
    while x[1] < 300 do
        x[1] = x[1] + 100
        box.session.push(x)
    end
    return x
end
box.space.tester:on_replace(
    on_replace_callback
)
callback_res = []

conn_pool = tarantool.ConnectionPool( [{'host':'localhost', 'port':3301}], user='guest')
res = conn_pool.insert( 'tester', (1, 'Mike'), mode=tarantool.Mode.PREFER_RO, on_push=callback, on_push_ctx=callback_res, ) # receiving out-of-band messages, # the conn_pool.insert is not finished yet. >>> run callback with data: [[100, 0]] >>> run callback with data: [[200, 0]] >>> run callback with data: [[300, 0]] # the conn_pool.insert is finished now.
print(res) >>> [1, 'Mike']
print(callback_res) >>>[[[100, 1]], [[200, 1]], [[300, 1]]]

Closes #201

@Totktonada
Copy link
Member

In the Lua API on_push is a callback (called at receiving of a pushed value) and on_push_ctx is a value to pass to the callback (context). As I see from the PR description, here the API is different: pushed values are just stored in a provided collection. One can't instantly react on a push using a code during a long request. Is it intentional?

@GRISHNOV GRISHNOV force-pushed the igrishnov/gh-201-iproto-push-support branch 2 times, most recently from 9663a63 to df17542 Compare October 20, 2022 13:16
@GRISHNOV
Copy link
Contributor Author

In the Lua API on_push is a callback (called at receiving of a pushed value) and on_push_ctx is a value to pass to the callback (context). As I see from the PR description, here the API is different: pushed values are just stored in a provided collection. One can't instantly react on a push using a code during a long request. Is it intentional?

I have updated the solution and description in PR. Now it is possible to use callback, which will be called streaming when out-of-band messages are received before the end of the main call (for example, the end of call or eval).

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.

Thank you for your draft!

I think we may implement push in a way even more similar to Lua one and describe basic cases with tests and small examples. Rework should be relatively small.

@GRISHNOV GRISHNOV force-pushed the igrishnov/gh-201-iproto-push-support branch from df17542 to cbbe900 Compare October 24, 2022 08:38
@GRISHNOV
Copy link
Contributor Author

Thank you for your feedback! If the current version of the draft is suitable, I will start writing tests and documentation

@GRISHNOV GRISHNOV force-pushed the igrishnov/gh-201-iproto-push-support branch from cbbe900 to 43b2142 Compare October 24, 2022 11:57
@GRISHNOV GRISHNOV requested a review from oleg-jukovec October 24, 2022 12:09
@DifferentialOrange
Copy link
Member

If the current version of the draft is suitable,

I think it is. To fix docs build, I recommend you to rebase on master branch. Moreover, I think it's better to rebase on yet unmerged #251 since it contains fixes for potential test fails.

@GRISHNOV GRISHNOV force-pushed the igrishnov/gh-201-iproto-push-support branch from 43b2142 to 25ae979 Compare October 25, 2022 20:11
@GRISHNOV GRISHNOV force-pushed the igrishnov/gh-201-iproto-push-support branch 3 times, most recently from f999db7 to c237b8e Compare October 26, 2022 09:54
@GRISHNOV GRISHNOV marked this pull request as ready for review October 26, 2022 10:00
@GRISHNOV GRISHNOV force-pushed the igrishnov/gh-201-iproto-push-support branch from c237b8e to 8b6c5c6 Compare October 26, 2022 19:25
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.

Thank you for your PR! I have left several comments. If there is any questions about test infrastructure (currently it's a mess) or doc (it should be in shape after last PRs), feel free to ask me.

@GRISHNOV GRISHNOV force-pushed the igrishnov/gh-201-iproto-push-support branch 4 times, most recently from 8cfa4cc to 57c9055 Compare October 31, 2022 07:46
@DifferentialOrange
Copy link
Member

Sorry, it seems that you'll need to rebase one more time. I think it's would be the last time and your PR would be merged next.

Master changes that are relevant to you: now ConnectionPool is supported on Python 3.6 and you don't need to skip tests anymore.

@GRISHNOV GRISHNOV force-pushed the igrishnov/gh-201-iproto-push-support branch from 57c9055 to 5d63f52 Compare October 31, 2022 12:34
@GRISHNOV
Copy link
Contributor Author

GRISHNOV commented Nov 1, 2022

Thank you for your feedback! I tried to answer all the comments on the code review

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 read the current version once more tomorrow together with examining the complicated case of schema reload with push.

@GRISHNOV GRISHNOV force-pushed the igrishnov/gh-201-iproto-push-support branch 2 times, most recently from 6d4a20e to 87f2bc0 Compare November 2, 2022 13:14
@GRISHNOV GRISHNOV force-pushed the igrishnov/gh-201-iproto-push-support branch from 87f2bc0 to 6c7ae28 Compare November 2, 2022 14:36
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.

Thank you for your updates. I think this is the last round of comments.

Please, add the CHANGELOG entry (sorry for not having a convenient PR checklist yet).

@GRISHNOV GRISHNOV force-pushed the igrishnov/gh-201-iproto-push-support branch from 6c7ae28 to e366e46 Compare November 3, 2022 09:36
Adds support for receiving out-of-band messages
from  server that uses box.session.push call.

Data obtaining is implemented for methods:
`call`, `eval`, `select`, `insert`, `replace`,
`update`, `upsert`, `delete`.
To do this, optional arguments `on_push` and `on_push_ctx`
are used for these methods. Argument `on_push` sets the
callback to call when an out-of-band message is received,
and the `on_push_ctx` argument allows to save the result
of `on_push` work or pass data to it.

So the API is similar to the implementation
of LUA version at the moment.

Closes #201
@GRISHNOV GRISHNOV force-pushed the igrishnov/gh-201-iproto-push-support branch from e366e46 to 64f2b0d Compare November 3, 2022 12:27
@GRISHNOV GRISHNOV requested a review from oleg-jukovec November 3, 2022 12:29
Copy link
Contributor

@oleg-jukovec oleg-jukovec left a comment

Choose a reason for hiding this comment

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

Thank you!

@DifferentialOrange DifferentialOrange merged commit 26c6db1 into master Nov 3, 2022
@DifferentialOrange DifferentialOrange deleted the igrishnov/gh-201-iproto-push-support branch November 3, 2022 13:44
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.

Consider IPROTO_PUSH support
4 participants