Skip to content

Commit

Permalink
Merge pull request #551 from conveyal/dev
Browse files Browse the repository at this point in the history
Bugfix release: do not limit number of analysis worker HTTP threads
  • Loading branch information
abyrd authored Sep 12, 2019
2 parents 65f17f9 + 0a3f6f0 commit 9cd14c4
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 15 deletions.
24 changes: 24 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,27 @@ R5 is developed primarily as a routing library for use in other projects (Convey
## Performing a Release

Releases are automatically generated using [maven-semantic-release](https://github.com/conveyal/maven-semantic-release).

## Structured Commit Messages

We use structured commit messages to allow automated tools to determine release version numbers and generate changelogs.

The first line of these messages is in the following format: `<type>(<scope>): <summary>`

The `(<scope>)` is optional. The `<summary>` should be in the present tense. The type should be one of the following:

- feat: A new feature from the user point of view, not a new feature for the build.
- fix: A bug fix from the user point of view, not a fix to the build.
- docs: Changes to the user documentation, or to code comments.
- style: Formatting, semicolons, brackets, indentation, line breaks. No change to program logic.
- refactor: Changes to code which do not change behavior, e.g. renaming a variable.
- test: Adding tests, refactoring tests. No changes to user code.
- chore: Updating build process, scripts, etc. No changes to user code.

The body of the commit message (if any) should begin after one blank line. If the commit meets the definition of a major version change according to semantic versioning (e.g. a change in API visible to an external module), the commit message body should begin with `BREAKING CHANGE: <description>`.

Presence of a `fix` commit in a release should increment the number in the third (PATCH) position.
Presence of a `feat` commit in a release should increment the number in the second (MINOR) position.
Presence of a `BREAKING CHANGE` commit in a release should increment the number in the first (MAJOR) position.

This is based on https://www.conventionalcommits.org.
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

<groupId>com.conveyal</groupId>
<artifactId>r5</artifactId>
<version>4.7.0</version>
<version>4.7.1-SNAPSHOT</version>
<packaging>jar</packaging>
<licenses>
<license>
Expand Down
23 changes: 9 additions & 14 deletions src/main/java/com/conveyal/r5/analyst/cluster/AnalystWorker.java
Original file line number Diff line number Diff line change
Expand Up @@ -93,15 +93,6 @@ public class AnalystWorker implements Runnable {
/** The port on which the worker will listen for single point tasks forwarded from the backend. */
public static final int WORKER_LISTEN_PORT = 7080;

/**
* The number of threads the worker will use to receive HTTP connections. This crudely limits memory consumption
* from the worker handling single point requests.
* Unfortunately we can't set this very low. We get a message saying we need at least 10 threads:
* max=2 < needed(acceptors=1 + selectors=8 + request=1)
* TODO find a more effective way to limit simultaneous computations, e.g. feed them through the regional thread pool.
*/
public static final int WORKER_SINGLE_POINT_THREADS = 10;

// TODO make non-static and make implementations swappable
// This is very ugly because it's static but initialized at class instantiation.
public static FilePersistence filePersistence;
Expand Down Expand Up @@ -331,12 +322,16 @@ public void run() {
// single-endpoint web server on this worker to receive single-point requests that must be handled immediately.
// This is listening on a different port than the backend API so that a worker can be running on the backend.
// When testing cluster functionality, e.g. task redelivery, many workers run on the same machine. In that
// case, this HTTP server is distabled on all workers but one to avoid port conflicts.
// case, this HTTP server is disabled on all workers but one to avoid port conflicts.
// Ideally we would limit the number of threads the worker will use to handle HTTP connections, in order to
// crudely limit memory consumption and load from simultaneous single point requests. Unfortunately we can't
// call sparkHttpService.threadPool(NTHREADS) because we get an error message saying we need over 10 threads:
// "needed(acceptors=1 + selectors=8 + request=1)". Even worse, in container-based testing environments this
// required number of threads is even higher and any value we specify can cause the server (and tests) to fail.
// TODO find a more effective way to limit simultaneous computations, e.g. feed them through the regional thread pool.
if (listenForSinglePointRequests) {
// Trying out the new Spark syntax for non-static configuration.
sparkHttpService = spark.Service.ignite()
.port(WORKER_LISTEN_PORT)
.threadPool(WORKER_SINGLE_POINT_THREADS);
// Use the newer non-static Spark framework syntax.
sparkHttpService = spark.Service.ignite().port(WORKER_LISTEN_PORT);
sparkHttpService.post("/single", new AnalysisWorkerController(this)::handleSinglePoint);
}

Expand Down

0 comments on commit 9cd14c4

Please sign in to comment.