Skip to content

Fix task release in session disconnect #107

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Feb 21, 2020
Merged

Fix task release in session disconnect #107

merged 1 commit into from
Feb 21, 2020

Conversation

LeonidVas
Copy link
Contributor

Fixes #103

olegrok
olegrok reviewed Feb 3, 2020
View reviewed changes
@LeonidVas LeonidVas force-pushed the lvasiliev/gh-queue-103-wrong-session-id branch from e4846e3 to 6c2aa6a Compare February 4, 2020 06:27
Mons
Mons approved these changes Feb 17, 2020
View reviewed changes
Copy link

@Mons Mons left a comment

Choose a reason for hiding this comment

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

LGTM

@LeonidVas @Totktonada
Fix task releasing at a client disconnection
0f2d090 
There are tarantool versions, where box.session.id() value may be
clobbered after a yield in on_disconnect trigger, see [1]. This commit
works the problem around.

[1]: tarantool/tarantool#4627

Fixes #103

@Totktonada: style fixes, more comments.
Totktonada
Totktonada reviewed Feb 21, 2020
View reviewed changes
Copy link
Member

@Totktonada Totktonada left a comment

Choose a reason for hiding this comment

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

Commented below. I'll push style fixes and more comments, hope you don't mind. Everything else is not so important: I just want to keep the investigation you made written somewhere.

Verified on tarantool-2.2.1.1: the test fails before the fix in queue and passes after it.

@Totktonada Totktonada force-pushed the lvasiliev/gh-queue-103-wrong-session-id branch from 6c2aa6a to 0f2d090 Compare February 21, 2020 21:14
@Totktonada
Copy link
Member

Ubuntu Cosmic fails: the fail is expected, because the distro reached EOL:

sudo apt-get update > /dev/null
E: The repository 'http://security.ubuntu.com/ubuntu cosmic-security Release' does not have a Release file.
E: The repository 'http://archive.ubuntu.com/ubuntu cosmic Release' does not have a Release file.
E: The repository 'http://archive.ubuntu.com/ubuntu cosmic-updates Release' does not have a Release file.
E: The repository 'http://archive.ubuntu.com/ubuntu cosmic-backports Release' does not have a Release file.

Totktonada
Totktonada approved these changes Feb 21, 2020
View reviewed changes
Copy link
Member

@Totktonada Totktonada left a comment

Choose a reason for hiding this comment

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

LGTM.

@Totktonada Totktonada merged commit 27594b0 into master Feb 21, 2020
@Totktonada Totktonada deleted the lvasiliev/gh-queue-103-wrong-session-id branch February 21, 2020 21:25
@Totktonada Totktonada mentioned this pull request Feb 25, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@olegrok olegrok olegrok left review comments

@Mons Mons Mons approved these changes

@Totktonada Totktonada Totktonada approved these changes

@kostja kostja kostja approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

bug: wrong session id in on_disconnect() trigger
5 participants
@LeonidVas @Totktonada @Mons @kostja @olegrok