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

Use monotonic time or modify the process.py script? #218

Closed
ryan44guo opened this issue Jun 19, 2018 · 7 comments
Closed

Use monotonic time or modify the process.py script? #218

ryan44guo opened this issue Jun 19, 2018 · 7 comments
Assignees

Comments

@ryan44guo
Copy link

ryan44guo commented Jun 19, 2018

Hi,
We meet a problem when starting.
When syncd starting and enter RUNNING state, the start.sh should exit almost immediately. But between the two events, the time changed by ntpd. so when the process.py run the code to exit the process, it find it quit too quickly, but its state is not STARTING (but RUNNING), so asserted.
In my opinion, in SONIC system, we should use monotonic time to control the interval-related process, maybe standarnd python library do not have it? although it seems not a difficult problem, we can install it if needed. I think it is not an isolation problem.
Of course, if we only deal with this question, we can modify the process.py, i.e, not assert the state to STARTING.
Hope to know the thought of the writer: What is your opinion about it?

@lguohan
Copy link
Contributor

lguohan commented Aug 4, 2018

@jleveque , i remember we have address this issue by patching supervisord, can you confirm?

@jleveque
Copy link
Contributor

jleveque commented Aug 6, 2018

@lguohan: Yes, this is a supervisor issue and should have been addressed by my patch from January: sonic-net/sonic-buildimage#1311.

@ryan44guo: Are you building SONiC from the latest source code?

@ryan44guo
Copy link
Author

ryan44guo commented Aug 7, 2018

hi all,
I am running 201803 branch, so your patch have been applied.
I check your patches again, i think you have not resolved all questions about time.
In this problem, syncd have changed to RUNNING state, because it had been running more than 1 sec.
But when call "finish" after that, the time have been changed in this moment,so when this function get time again, it entered too_quickly branch, we know, this branch only valid for STARTING state, but now the state is RUNNING.
Our questions prove that these problems will occur in several situations, so i suggest that we should use monotonic time instead.
@jleveque @lguohan

@lguohan
Copy link
Contributor

lguohan commented Aug 21, 2018

@ryan44guo , thanks for the feedback. you mentioned the monotonic time, how can we get it in python? Any idea?

@ryan44guo
Copy link
Author

@lguohan , i am not familiar with python, but i think python should have some libs about monotonic time, although maybe it is not the default libs. there is an URL https://pypi.org/project/monotonic/ about it, maybe someone familiar with python can use it? Or, i have seen some codes on github, but i do not know which one is better.

@jleveque
Copy link
Contributor

The maintainer of Supervisor wasn't happy with the way https://pypi.org/project/monotonic/ implements monotonic time for Python2, so he wasn't willing to use that library.

I found and fixed what I believe was the last remaining issue with clock rollback here: sonic-net/sonic-buildimage#2624

My changes were also merged into upstream Supervisor here: Supervisor/supervisor#1047

I am closing this issue for now. Please reopen if you see the issue again in the future.

@mnaberez
Copy link

I found and fixed what I believe was the last remaining issue with clock rollback here: sonic-net/sonic-buildimage#2624

My changes were also merged into upstream Supervisor here: Supervisor/supervisor#1047

Supervisor 3.4.0 (PyPI package; includes this patch)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants