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

Session expire on graphd will end up floating session being released #5094

Open
wey-gu opened this issue Dec 22, 2022 · 10 comments
Open

Session expire on graphd will end up floating session being released #5094

wey-gu opened this issue Dec 22, 2022 · 10 comments
Assignees
Labels
affects/none PR/issue: this bug affects none version. help wanted Community: does anyone want to work on it? later Solution: this issue will be handle in later version severity/minor Severity of bug type/bug Type: something is unexpected

Comments

@wey-gu
Copy link
Contributor

wey-gu commented Dec 22, 2022

This is an issue talked about in WeChat Group:Earth, synced msg was pasted in the end of the issue.

It seems in current idle-release session mechanism was monitored and triggered in each graphD, that is, when a session(session-id:foo) was floated from graphd0 to graphd1, and will be actively used in graphd1 and not caming back to graphd0, after idle session timer expires in graphd0, foo will be released from graphd0.

I am on GitHub mobile(don't have access to the code to verify this)due to Covid leave, so just quickly track the issue here.

Plz forgive my unchecking of the code before raising this.

cc @Aiee

ref:

@wey-gu wey-gu added the type/bug Type: something is unexpected label Dec 22, 2022
@github-actions github-actions bot added affects/none PR/issue: this bug affects none version. severity/none Severity of bug labels Dec 22, 2022
@Sophie-Xie Sophie-Xie added this to the v3.4.0 milestone Dec 26, 2022
@HarrisChu HarrisChu added the severity/major Severity of bug label Dec 27, 2022
@github-actions github-actions bot removed the severity/none Severity of bug label Dec 27, 2022
@HarrisChu
Copy link
Contributor

steps:

  1. create a session in graphA.
  2. the session connection is broke, then create the connection with graphB.
  3. graphB find the session in meta, and it exists, so could query with this session.
  4. but session in graphA local would never be updated, and then would be expired by graphA.
  5. after delete the session, use cannot use this session in graphB. (as session has been delete in metad)

more info

  1. create in graph2 (port 10010)

image

  1. connect with graph1 (port 10005)

image

  1. session is delete by graph2

image

@HarrisChu
Copy link
Contributor

HarrisChu commented Dec 27, 2022

we can simply compare the latest graphAddr in meta, if not in the same, could not delete the session.

not sure if there's any other problem.

@Aiee Aiee added the wontfix Solution: this will not be worked on recently label Jan 4, 2023
@Aiee
Copy link
Contributor

Aiee commented Jan 4, 2023

This is a rare case and it won't be fixed in 3.4.
cc @xtcyclist

@Sophie-Xie Sophie-Xie removed this from the v3.4.0 milestone Jan 4, 2023
@Sophie-Xie Sophie-Xie added later Solution: this issue will be handle in later version wontfix Solution: this will not be worked on recently and removed wontfix Solution: this will not be worked on recently later Solution: this issue will be handle in later version labels Jan 4, 2023
@xtcyclist
Copy link
Contributor

we can simply compare the latest graphAddr in meta, if not in the same, could not delete the session.

not sure if there's any other problem.

Need to make sure the timeout mechanism still works, if we do this.

@xtcyclist
Copy link
Contributor

Session is not advised to be kept for a long time. And, it is always recommended to check the aliveness of sessions and recreate sessions if needed. Since this is a low-priority issue and we are freezing the code for 3.4, close this issue for now.

If any developer wants to optimize this part, please feel free to reopen this issue.

@github-actions github-actions bot added the process/fixed Process of bug label Jan 4, 2023
@wey-gu
Copy link
Contributor Author

wey-gu commented Jan 4, 2023

we can simply compare the latest graphAddr in meta, if not in the same, could not delete the session.

not sure if there's any other problem.

I think this is the way to go, I'll fix this in this approach later when I have some bandwidth in my free time

@wey-gu wey-gu added the help wanted Community: does anyone want to work on it? label Jan 4, 2023
@wey-gu
Copy link
Contributor Author

wey-gu commented Jan 5, 2023

@HarrisChu @xtcyclist @Sophie-Xie will this be tagged wont fix intended or we could save it from "won't fix" actually :-P?

@Sophie-Xie
Copy link
Contributor

Session is not advised to be kept for a long time. And, it is always recommended to check the aliveness of sessions and recreate sessions if needed. Since this is a low-priority issue and we are freezing the code for 3.4, close this issue for now.

If any developer wants to optimize this part, please feel free to reopen this issue.

Just a label, I'll change it to avoid confusion.

@Sophie-Xie Sophie-Xie added later Solution: this issue will be handle in later version severity/minor Severity of bug and removed wontfix Solution: this will not be worked on recently process/fixed Process of bug labels Jan 6, 2023
@wey-gu
Copy link
Contributor Author

wey-gu commented Mar 28, 2023

@xtcyclist @yixinglu do we consider to fix this?

wey-gu added a commit to wey-gu/nebula that referenced this issue May 9, 2023
wey-gu added a commit to wey-gu/nebula that referenced this issue May 9, 2023
@xtcyclist
Copy link
Contributor

We need to consider fixing this issue due to potential requirements from customers.

@xtcyclist xtcyclist reopened this Aug 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects/none PR/issue: this bug affects none version. help wanted Community: does anyone want to work on it? later Solution: this issue will be handle in later version severity/minor Severity of bug type/bug Type: something is unexpected
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants