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

wait for 10 seconds after sending SIGTERM #264

Closed
wants to merge 1 commit into from
Closed

Conversation

buzz0r
Copy link

@buzz0r buzz0r commented Jun 6, 2022

It's better to wait until the main ioBorker controller process is finished with it's shutdown.
If we don't wait for the clean shutdown, the adapter hm-rpc can not disconnect from the ccu gateway and is not working after a container restart.
See:
ioBroker/ioBroker.hm-rpc#595

@buanet
Copy link
Owner

buanet commented Jun 13, 2022

Thank you. But this is not what I had planned for optimizing the graceful shutdown.
I'm now testing a different timeout method (inspired by the rewritten maintenance script) to shorten the timeout time.
Maybe you can take a look at b8c67b7 and check if the solution will also fix the hm-rpc issue.
The solution will be available with new beta version within the next days.

BTW: I'm using Homematic and rpc/ xmlrpx too. I've never faced that issue so far...

Regards,
André

@buanet buanet closed this Jun 13, 2022
@buzz0r
Copy link
Author

buzz0r commented Jun 14, 2022

Hi,
if I understand the proposed pull request correctly, so far, it’s only handling the maintance.sh script.
But if a user does, for example, a "docker-compose stop" pkill is till instantly killing the main process without giving the running adapters some time to do a clean shutdown.

I could do another pull request based on the timeout implementation used in the maintance.sh script and adapt it in the iobroker_startup.sh, if that would be better fitting in the project.
What do you think?

@buanet
Copy link
Owner

buanet commented Jun 14, 2022

Hi,
please take a closer look here: b8c67b7#diff-d7a77b1b100f0e35f6ffadd80381e880c07fc52f4db3dc347d5af9079c87122f
I already implemented the solution from the rewritten maintenance script so the startup script with yesterdays commit.
So no more pr needed.

However. I will publish the latest adjustments as new beta version soon. Please check if this helps you with your issue.

Regards,
André

@buzz0r
Copy link
Author

buzz0r commented Jun 14, 2022

Ah sorry, I missed this file change! Looks good, but of course I will test it as soon as a new beta release is available!
Thx again!

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

Successfully merging this pull request may close these issues.

2 participants