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

[agw] Remove inheritance relationship between task state managers and common state managers #11192

Open
Tracked by #11199
ardzoht opened this issue Jan 18, 2022 · 5 comments
Labels
cpp-migration C++ migration-related issue type: enhancement Enhancements to existing features willfix This will be worked on and shouldn't be reaped

Comments

@ardzoht
Copy link
Contributor

ardzoht commented Jan 18, 2022

Description

  • MME, S1AP, SPGW and NGAP tasks are using the StateManager singleton class which wraps functionality to allocate the UE and task state, as well as reading and writing the state back and forth into the redis database.
  • This was introduced to create a single point of state allocation for any task that holds state in the MME.
  • This creates an unnecessary complex dependencies between the StateManager, StateConverter and tangles the responsibilities of reading/writing state to redis as well as allocating the local state in the hashtable.

Goal

  • The goal is to move away from the StateManager singleton and create different classes for the tasks to allocate the UE and task state there, and to potentially do the reads/writes to redis in a separate class.
@ardzoht ardzoht added type: enhancement Enhancements to existing features cpp-migration C++ migration-related issue labels Jan 18, 2022
@stale
Copy link

stale bot commented Apr 17, 2022

This pull request has been automatically marked as stale because it has not had any recent activity by the author or maintainer in the past 45 days. It will be closed if no further activity occurs in the next 7 days. Thank you for your contributions.

@stale stale bot added the wontfix This will not be worked on label Apr 17, 2022
@stale
Copy link

stale bot commented Apr 24, 2022

This pull request has exceeded its lifecylce and will be closed. Should you wish to continue working on this, please reopen it or start a new pull request. Thank you again for your contributions.

@stale stale bot closed this as completed Apr 24, 2022
@ssanadhya ssanadhya reopened this Apr 27, 2022
@stale stale bot removed the wontfix This will not be worked on label Apr 27, 2022
@ssanadhya ssanadhya added the willfix This will be worked on and shouldn't be reaped label Apr 27, 2022
@ssanadhya
Copy link
Collaborator

@rsarwad , moving our Slack conversation here. Your question on this issue is:
" Across all tasks, there are some common functions like, get_imsi_str(), is_persist_state_enabled(), clear_ue_state_db() and some variables like table_key, task_name, is_initialized, persist_state_enabled etc. Can we keep these common parameters in base class and definition also remains same across all tasks?"

Yes, that should be okay. The goal of this issue is to separate the responsibility for Redis read/write to each task, so that each task can write its own state without being blocked on other tasks. @ardzoht , what do you think?

@ardzoht
Copy link
Contributor Author

ardzoht commented Sep 30, 2022

Agree @ssanadhya, these function should be common across mme tasks, so the definition should not change across them.

@rsarwad
Copy link
Contributor

rsarwad commented Oct 3, 2022

Thanks @ssanadhya and @ardzoht for clarification

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cpp-migration C++ migration-related issue type: enhancement Enhancements to existing features willfix This will be worked on and shouldn't be reaped
Projects
None yet
Development

No branches or pull requests

3 participants