Skip to content

Document new behaviour of session.on_disconnect #682

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

Closed
Gerold103 opened this issue Dec 11, 2018 · 8 comments
Closed

Document new behaviour of session.on_disconnect #682

Gerold103 opened this issue Dec 11, 2018 · 8 comments
Assignees
Labels
need feedback [special status] On hold, awaiting feedback

Comments

@Gerold103
Copy link
Contributor

Now session.on_disconnect triggers are called right after disconnect. Not after a last request is finished.

@pgulutzan
Copy link
Contributor

In what version did this occur?

If it's a significant behaviour change, we have to say "starting in version x this incompatible change was made ...". But I wonder whether the version-10 documentation is already correct by accident, because it says
"Define a trigger for execution after a client has disconnected. If the trigger function causes an error, the error is logged but otherwise is ignored. The trigger is invoked while the session associated with the client still exists and can access session properties, such as box.session.id()."
So, what in the current documentation is now incorrect?

@pgulutzan pgulutzan added the need feedback [special status] On hold, awaiting feedback label Dec 17, 2018
@Gerold103
Copy link
Contributor Author

The new behaviour appears since 1.10. As I know, beforehand on_disconnect triggers were called only after a last request has finished. Now they are called right after disconnect. If the documentation does not specify the time of triggers invocation, then maybe it should start specifying.

@Gerold103
Copy link
Contributor Author

Also, it is worth documenting that not all session properties are valid after disconnect. box.session.fd() will return -1, box.session.sync() will return 0, and box.session.push will return nil + error after disconnect.

@pgulutzan
Copy link
Contributor

The manual says that the on_disconnect trigger function is called after disconnect. As far as I can tell, it does not wait for the last request to finish, but that is true in both version 1.9 and version 1.10. It does wait for other requests to yield,but that seems obvious -- otherwise how can the trigger function be called?

Here is an example based on how I understand what you are saying.
On shell #1, as admin:
tarantool
box.cfg{listen=3302}
function on_disconnect_impl() print('disconnect ' .. box.session.id()); os.exit(); end
box.session.on_disconnect(on_disconnect_impl)
box.schema.user.grant('guest','read,write,execute','universe')
fiber = require('fiber')
fiber.sleep(1000)
On shell #2, as guest:
tarantoolctl connect 3302
^C i.e. control-C
Result: shell #1 displays and stops, regardless of Tarantool version.

@Gerold103
Copy link
Contributor Author

box.cfg{listen = 3313}
netbox = require('net.box')
fiber = require('fiber')
box.schema.user.grant('guest', 'read,write,execute', 'universe')
finish_long = false
disconnected = false
function long()
	box.session.on_disconnect(function() disconnected = true end)
	while not finish_long do fiber.sleep(0.01) end
end
c = netbox.connect(box.cfg.listen)
c:call('long', {}, {timeout = 0.01})
c:close()
disconnected -- (1), false in 1.9, true in 1.10
finish_long = true
while not disconnected do fiber.sleep(0.01) end
disconnected -- (2), true in both

In 1.9 label (1) will print false, (2) - true. In 1.10 they both print true.
Under finishing a last request I mean not a last request of Tarantool, but of a connection.

@pgulutzan
Copy link
Contributor

Okay, I can add, following what I quoted previously: "Since version 1.10, the trigger function is invoked immediately after the disconnect, even if requests that were made during the session have not finished." It doesn't seem significant enough to deserve a warning about changed behaviour, since in the version 1.9 manual we don't say something different. And in section https://tarantool.io/en/doc/1.10/book/box/box_session/#box-session-id I will change "The result can be 0 meaning there is no session." because the result might be -1; plus in the overview I can add that some items become invalid after disconnect.

@Gerold103
Copy link
Contributor Author

Box.session.id() does not change in case of disconnect. Only sync and fd. Also push() returns an error.

@pgulutzan
Copy link
Contributor

Okay, I made the additions that I described, except that instead of putting something in the overview I added notes in id() and push() (we don't document fd). The mention of -1 is not about what one sees from the trigger function after disconnect, it merely is stated that -1 is a possible value.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need feedback [special status] On hold, awaiting feedback
Projects
None yet
Development

No branches or pull requests

2 participants