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.

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

Fix session handling #648

Merged
merged 6 commits into from
Mar 5, 2022
Merged

Fix session handling #648

merged 6 commits into from
Mar 5, 2022

Conversation

hylkevds
Copy link
Collaborator

@hylkevds hylkevds commented Feb 2, 2022 •
edited
Loading

See discussion in #608.

  • Add a remove method to IQueueRepository, so queues can actually be removed when no longer needed.
  • Make the Session the only place where the Queue lives outside of the repository by removing the queues field from SessionRegistry.
  • Tie a Queue and a Session in a 1-to-1 way.
  • Remove and clean the old Session and Queue before creating a new pair for a ClientId.
  • Clean Session and Queue when they are no longer needed.
  • In case a new Connection takes over a Session from an existing, connected connection, ensure the old session can not influence the Session any more after the Session was taken over.

This was referenced Feb 2, 2022
Closed
Merged
@hylkevds hylkevds force-pushed the fix_session_handling branch from f97c158 to e3167e7 Compare February 6, 2022 12:47
@hylkevds hylkevds marked this pull request as ready for review February 6, 2022 14:20
@hylkevds hylkevds force-pushed the fix_session_handling branch from e3167e7 to c63dcc8 Compare February 28, 2022 12:58
@hylkevds hylkevds requested a review from andsel February 28, 2022 18:21
hylkevds added 2 commits March 1, 2022 14:23
@hylkevds
Added interface for removing queues from repositories
6cb467a
@hylkevds
Reworked session and queue creation
699e40a 
Only one queue per clientId can exist. Don't create a new session/queue
unless it is certain we need a new session, and make sure the old session
/queue is cleaned up first.
@hylkevds hylkevds force-pushed the fix_session_handling branch from c63dcc8 to 699e40a Compare March 1, 2022 13:23
andsel
andsel requested changes Mar 5, 2022
View reviewed changes
Copy link
Collaborator

@andsel andsel left a comment

Choose a reason for hiding this comment

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

@hylkevds we are almost there, just a couple of nitpick and the PR then is ready.

broker/src/main/java/io/moquette/broker/SessionRegistry.java Outdated Show resolved Hide resolved
broker/src/main/java/io/moquette/broker/SessionRegistry.java Outdated Show resolved Hide resolved
broker/src/main/java/io/moquette/broker/SessionRegistry.java Outdated Show resolved Hide resolved
broker/src/main/java/io/moquette/broker/SessionRegistry.java Outdated Show resolved Hide resolved
broker/src/main/java/io/moquette/broker/SessionRegistry.java Outdated Show resolved Hide resolved
broker/src/main/java/io/moquette/broker/SessionRegistry.java Outdated Show resolved Hide resolved
@hylkevds
Copy link
Collaborator Author

hylkevds commented Mar 5, 2022

@hylkevds we are almost there, just a couple of nitpick and the PR then is ready.

Excellent. They should all be fixed now.

@hylkevds hylkevds requested a review from andsel March 5, 2022 11:04
andsel
andsel approved these changes Mar 5, 2022
View reviewed changes
Copy link
Collaborator

@andsel andsel left a comment

Choose a reason for hiding this comment

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

LGTM

@andsel andsel merged commit 85f62f2 into moquette-io:main Mar 5, 2022
@hylkevds hylkevds deleted the fix_session_handling branch March 6, 2022 19:13
This was referenced Nov 23, 2022
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andsel andsel andsel 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.
2 participants
@hylkevds @andsel