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 disk state checking synchronous? #3

Closed
garybernhardt opened this issue Jan 31, 2018 · 5 comments
Closed

Make disk state checking synchronous? #3

garybernhardt opened this issue Jan 31, 2018 · 5 comments
Assignees

Comments

@garybernhardt
Copy link

A clarification on my toots about this:

Right now, you have an include/exclude system to watch certain files but not others. That will probably always be useful for complex apps. But for simple apps (like the ones I maintain), it's only a burden. E.g., my scripts don't have extensions, so how do I specify that they should be watched? And even failing that, I just don't want to maintain a list of watched files; that's work.

I think it's strictly better to check disk state synchronously:

  • A request comes in.
  • You halt the request in your proxy.
  • You calculate the current disk state and compare it to the last observed state. (serveit does this via recursively calling stat(2) and just keeping a list of the results, which works great.)
  • If the disk state changed, then you restart the server.
  • Finally, you wait for the server to come up, then forward the request to it.

This has two huge benefits:

First, no load is generated unless a request comes in. With the current design, my CPU spikes every time I write a file, which causes a server reload. There's really no benefit to doing that, because you're always going to write some file immediately before a refreshing the browser anyway. Any server restarts before that final write are wasted, only serving to load the CPU and slow my other tools down.

Second, if the design is synchronous then many apps can just let tychus watch all files. There can never be a reload loop, even if the app writes logs or other files during startup or during requests. (Unless, I suppose, it makes a request to itself, which would be pretty weird.) It's true that disk writes done by the server will cause each new request to do a server reload, but there can never be a runaway reload loop. (I've already seen such a reload loop with the current design while I was testing it out.)

I didn't look closely enough to see how big a change this would be, but I suspect it's probably not that big. You obviously have the proxy in place, which is what I got hung up on when I gave this a try. (Nice work on that, by the way; I haven't noticed any problems with it!)

@PatKoperwas
Copy link
Collaborator



Ah I believe I approached this problem from the inverse perspective. I built the reloader first, then the proxy which probably influenced many of my decisions - I didn’t want a performance penalty on the request side (checking the filesystem on each request).

But you bring up an excellent point that I really should just do the opposite. 


Right now the flow goes like this



1. File system change

2. Pause proxy

3. Restart

4. Unpause proxy


It is quick and easy to flip the order to match you specified above. You pay a penalty on each request, but that penalty should be a lot smaller than continuously watching the file system and using a whole bunch of CPU to constantly recompile.



I’m just trying to think of some edge cases, and what would / should happen…



1. Request comes in, and there are changes.

2. Process begins to restart

3. You make another file system change after the process is shutdown, but 
   before it finishes compiling and is ready to serve requests (really only a 
   problem with compiled languages).

Technically, the request would be served using “stale” code. But I think that’s okay - the request was made before those code changes were in place, and I wouldn’t expect them to be incorporated. The subsequent request would trigger another restart and at that point everything should be fresh.



@garybernhardt
Copy link
Author

I think there is a little timing hole present as you describe. But the normal action for the user is always to save a source file, then refresh browser, so the build should only be triggered once the user is sitting there waiting for the refresh. Hopefully no files are written then.

You could theoretically check for additional changes after the build, and repeat the build if things changed. But then you open yourself up to reload loops again, because tychus might pick up build artifact changes, trigger a build, causing more artifact changes, and so on forever.

One more note because I might forget to mention it later. One really nice thing that serveit does is re-compute the disk state after the build:

  1. Compare disk state to last known state. If it's different, then:
  2. Do build.
  3. Capture another disk state, post-build, to be the new "last known disk state".

This ensures that the build process can change the disk, and those changes are effectively ignored by the reloader. So you don't have to tell the reloader to ignore your .o files (or whatever your build artifacts are). They'll always be picked up by the second post-build disk state check, and so won't show up as changes on the next refresh.

@PatKoperwas
Copy link
Collaborator

Had the 💡 moment implementing this last night after reading through the serveit readme. I didn't even think about the use case where the command you're running through tychus is not the webserver. Could be a script to generate markdown, assets, images, w/e that are served by a server you're running concurrently, and just want requests to pause while that thing happens.

@PatKoperwas
Copy link
Collaborator

PatKoperwas commented Feb 1, 2018

Turns out you can chain tychus together (well, once I push the code up). Not sure if useful but pretty neat.

Example (modified from serveit):

# Folder structure
- app/
  - main.go
- index.md
// main.go - just serves file from ../
func main() {
	http.Handle("/", http.FileServer(http.Dir("../")))
	log.Fatal(http.ListenAndServe(":3000", nil))
}

So you start tychus with the app port pointing to 3000 and the proxy on 4000. Running this inside app means that only changes to main.go will trigger a refresh.

cd app/
tychus go run main.go -a 3000 -p 4000

Back at the root, we can setup another tychus process to run multimarkdown to generate html. And we chain these two process together, by pointing the app port to the first proxy.

tychus run "multimarkdown index.md > index.html" -a 4000 -p 4001 ˙ --wait --ignore=app

So if you modify index.md, when you curl localhost:4001, that will run multimarkdown. Once it exits, it will forward the request to localhost:4000, which will forward the request to the fileserver on localhost:3000. If main.go was also modified, it will get recompiled and started before serving the request.

@PatKoperwas
Copy link
Collaborator

PatKoperwas commented Feb 5, 2018

Resolved by #4 & #5. brew upgrade tychus to get latest version.

@PatKoperwas PatKoperwas self-assigned this Nov 26, 2018
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

2 participants