This repository was archived by the owner on Sep 14, 2021. It is now read-only.
Continue serving requests when highlighting an individual file fails #20
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When Syntect fails to highlight a file it often does so by panicking. Most often these issues come from us making use of syntax definitions which simply aren't supported by Syntect very well (e.g. they use some new or obscure ST3 feature). There is an upstream issue to [return a `Result` type instead of panicking](trishume/syntect#98) when this occurs. Prior to this change, a user requesting syntect_server to highlight a bad file (i.e. hitting a case in the syntax definition not supported by Syntect) would result in `syntect_server` dying. This has been a known issue for a while, but in practice hasn't been that bad because these cases are relatively rare and Kubernetes / Docker restarts the process very quickly anyway. However, when it does occur it terminates all ongoing highlighting requests which causes blips that users see. After this change, we handle these panics by catching and unwinding the stack. This isn't perfect / ideal / idiomatic Rust code (see the `catch_unwind` docs), but it does solve the problem and is a better approach than e.g. adding more replicas of this service. Fixes sourcegraph/sourcegraph#3164
|
Published as |
emidoots
pushed a commit
that referenced
this pull request
Aug 30, 2019
emidoots
pushed a commit
that referenced
this pull request
Oct 3, 2019
emidoots
pushed a commit
that referenced
this pull request
Oct 3, 2019
* use http-server-stabilizer + 4 worker subprocesses This change makes syntect_server resilient to the two classes of problems we've seen in production usage of it: 1. Some specific language grammar/file pairs can cause syntect to panic internally. This is usually because syntect doesn't implement a specific sublime-syntax feature in some way and it [panics instead of returning result types](trishume/syntect#98). 2. Much rarer, some specific language/grammar file pairs can cause syntect to get stuck in an infinite loop internally -- never to return and consuming an entire CPU core until it is restarted manually. Previously we tried to solve #1 through stack unwinding (c5773da), but since the 2nd issue above also appeared it proved to not be sufficient on its own. It is still useful, though, because it can do per-request recovery of the first failure scenario above and as such it will be added back in. Even without stack unwinding, http-server-stabilizer helps both cases above by running and monitoring replicas of syntect_server. See the README in https://github.com/slimsag/http-server-stabilizer for details. It is important to note that all this does is stop these individual file failures from harming other requests to syntect_server. They are still issues on their own, and logging and Prometheus monitoring is now in place for us to identify when this is occurring and in which file it occurred so we can track down the issue and make small reproduction cases to file and fix upstream. Since only one instance of syntect_server was previously running and we now run multiple, more memory is needed. Each instance requires about 1.1 GB at peak (depending on which languages are used). The default is now to run 4 workers, so 4.4 GB is the minimum required and 6 GB is suggested. In the event only one worker is ran (via setting the env var `WORKERS=1`), stability is still greatly improved since the 2nd failure case above can only last a short period of time instead of until the container is restarted manually. Part of sourcegraph/sourcegraph#5406 * Add Dart+Kotlin support; upgrade to Rocket v0.4 * Continue serving requests when highlighting an individual file fails (#20)
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When Syntect fails to highlight a file it often does so by panicking. Most
often these issues come from us making use of syntax definitions which simply
aren't supported by Syntect very well (e.g. they use some new or obscure ST3
feature). There is an upstream issue to return a
Resulttype instead ofpanicking when this occurs.
Prior to this change, a user requesting syntect_server to highlight a bad file
(i.e. hitting a case in the syntax definition not supported by Syntect) would
result in
syntect_serverdying. This has been a known issue for a while, butin practice hasn't been that bad because these cases are relatively rare and
Kubernetes / Docker restarts the process very quickly anyway. However, when it
does occur it terminates all ongoing highlighting requests which causes blips
that users see.
After this change, we handle these panics by catching and unwinding the stack.
This isn't perfect / ideal / idiomatic Rust code (see the
catch_unwinddocs),but it does solve the problem and is a better approach than e.g. adding more
replicas of this service.
Fixes sourcegraph/sourcegraph#3164