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

make watch (air) concurrent process problem #24845

Closed
wxiaoguang opened this issue May 22, 2023 · 11 comments
Closed

make watch (air) concurrent process problem #24845

wxiaoguang opened this issue May 22, 2023 · 11 comments
Labels
topic/build PR changes how Gitea is built, i.e. regarding Docker or the Makefile type/bug

Comments

@wxiaoguang
Copy link
Contributor

wxiaoguang commented May 22, 2023

(I do not use air, this issue is just for helping debugging developers reported problem)

It seems that the make watch may not work well in some cases:

air logs:

main.go has changed    !!! The first change and build
building...
Running go generate...
...  go build ... -o gitea
main.go has changed    !!! The second change and build
building...
running...
Running go generate...
2023/05/22 11:45:44 cmd/web.go:123:runWeb() [I] Starting Gitea on PID: 90706
...
...  go build ... -o gitea
...
running...
2023/05/22 11:45:46 cmd/web.go:123:runWeb() [I] Starting Gitea on PID: 90768

If a file is changed when a build is in progress, air doesn't stop the old build, then two Gitea processes will be started.

Two running Gitea processes will conflict with each other: port listen, leveldb lock, etc.

@silverwind
Copy link
Member

silverwind commented May 22, 2023

Have not seen above but have seen two other related issues:

  • When template compilation fails, I need to restart the air server because the current template reloading mechanism does not rebuild on save after one template build failure. Could be solved by making airwatch the template files, removing the backend mechanism, or fix the backend mechanism.
  • Sometimes, CTRL-C does not kill all child processes on watch.sh, and they remain running in the background. Can probably be fixed in the bash script somehow.

@wxiaoguang
Copy link
Contributor Author

wxiaoguang commented May 22, 2023

When template compilation fails, I need to restart the air server because the current template reloading mechanism does not rebuild on save after one template build failure.

I do not see there is any problem. Although I do not use air, I made some quick tests:

  • Start Gitea by make watch
  • Visit some pages
  • Introduce some template errors
  • Gitea reports template compilation error
  • Visit the pages, they still use old templates
  • Fix the template errors
  • Visit the pages, they use thew new/corrected templates

Could be solved by making airwatch the template files, removing the backend mechanism, or fix the backend mechanism.

I do not think so. The backend code already supports hot-reloading the templates which is more convince and faster than completely restarting the Gitea process.

@silverwind
Copy link
Member

silverwind commented May 22, 2023

Sometimes, CTRL-C does not kill all child processes on watch.sh, and they remain running in the background. Can probably be fixed in the bash script somehow.

On this topic, I think we can try kill -9. I think in some cases, like when the go compiler runs, it may ignore a SIGTERM, but if we send SIGKILL, it can not ignore it. Thought there might be a small risk that it's interrupted in a bad moment.

@silverwind
Copy link
Member

When template compilation fails, I need to restart the air server because the current template reloading mechanism does not rebuild on save after one template build failure.

I do not see there is any problem. Although I do not use air, I made some quick tests:

* Start Gitea by `make watch`

* Visit some pages

* Introduce some template errors

* Gitea reports template compilation error

* Visit the pages, they still use old templates

* Fix the template errors

* Visit the pages, they use thew new/corrected templates

Could be solved by making airwatch the template files, removing the backend mechanism, or fix the backend mechanism.

I do not think so. The backend code already supports hot-reloading the templates which is more convince and faster than completely restarting the Gitea process.

It definitely happens to me a lot that I have to restart the go server because templates do not rebuild after an error was fixed. I'm not sure what exact sequence of actions triggers it yet. It is unrelated to air and should be reproducible just with the go server in dev mode.

@wxiaoguang
Copy link
Contributor Author

It definitely happens to me a lot that I have to restart the go server because templates do not rebuild after an error was fixed.

I can imagine a case for your problem: if the go code and tmpl code are changed at the same time, tmpl has errors, then air will restart the Gitea process, but Gitea will report fatal errors when it encounters template error during it's startup. So, the Gitea process just exits due to your template errors. Then if you only fix the template errors, air doesn't see go file change, air won't start a new Gitea process.

It could be fixed by make Gitea keep running in dev mode even if there is template error during startup.

@wxiaoguang
Copy link
Contributor Author

-> Improve RunMode / dev mode #24886

@silverwind
Copy link
Member

Yes, it sounds likely that I've hit that scenario, not 100% sure because most of the time, I actually don't edit go code, just templates and css/js.

techknowlogick pushed a commit that referenced this issue May 25, 2023
1. non-dev mode is treated as prod mode, to protect users from
accidentally running in dev mode if there is a typo in this value.
2. in dev mode, do not need to really exit if there are template errors,
because the template errors could be fixed by developer soon and the
templates get reloaded, help:
* #24845 (comment)
3. Fine tune the mail template loading message.
@silverwind
Copy link
Member

silverwind commented May 27, 2023

Regarding original issue: I think it's an air bug. Maybe we can kill off any old processes when starting new ones. Air also has this option, not sure it would help.

@silverwind
Copy link
Member

silverwind commented May 27, 2023

I reproduced this double build and double run just now after switching branches while air was running:

code.gitea.io/gitea/routers/web
code.gitea.io/gitea/routers/web
code.gitea.io/gitea/routers
code.gitea.io/gitea/cmd
code.gitea.io/gitea/cmd
code.gitea.io/gitea
code.gitea.io/gitea
make[2]: Leaving directory '/Users/silverwind/git/gitea'
running...
make[2]: Leaving directory '/Users/silverwind/git/gitea'
running...
2023/05/27 19:11:57 cmd/web.go:123:runWeb() [I] Starting Gitea on PID: 79445
2023/05/27 19:11:57 cmd/web.go:174:runWeb() [I] Global init
2023/05/27 19:11:57 cmd/web.go:123:runWeb() [I] Starting Gitea on PID: 79446
2023/05/27 19:11:57 cmd/web.go:174:runWeb() [I] Global init

@silverwind
Copy link
Member

I think we shoud look for air alternatives. I think turbowatch is worth taking a look at.

@lunny lunny added the topic/build PR changes how Gitea is built, i.e. regarding Docker or the Makefile label Jun 21, 2023
@wxiaoguang
Copy link
Contributor Author

Not working on it because I am not the user and don't understand air, so close it. Feel free to propose new solutions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
topic/build PR changes how Gitea is built, i.e. regarding Docker or the Makefile type/bug
Projects
None yet
Development

No branches or pull requests

3 participants