Skip to content

Commit

Permalink
Set Status.PRESENT after synchronous preload
Browse files Browse the repository at this point in the history
Fixes #949
  • Loading branch information
ansoncfit committed Oct 26, 2024
1 parent e5a9a26 commit fe68b94
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 7 deletions.
7 changes: 5 additions & 2 deletions src/main/java/com/conveyal/r5/analyst/NetworkPreloader.java
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,10 @@ public LoaderState<TransportNetwork> preloadData (AnalysisWorkerTask task) {
* data is prepared.
*/
public TransportNetwork synchronousPreload (AnalysisWorkerTask task) {
return buildValue(Key.forTask(task));
Key key = Key.forTask(task);
TransportNetwork scenarioNetwork = buildValue(key);
setComplete(key, scenarioNetwork);
return scenarioNetwork;
}

@Override
Expand Down Expand Up @@ -140,7 +143,7 @@ protected TransportNetwork buildValue(Key key) {
linkedPointSet.getEgressCostTable(progressListener);
}
}
// Finished building all needed inputs for analysis, return the completed network to the AsyncLoader code.
// Finished building all needed inputs for analysis, return the completed network
return scenarioNetwork;
}

Expand Down
15 changes: 10 additions & 5 deletions src/main/java/com/conveyal/r5/util/AsyncLoader.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
* "value is present in map".
*
* Potential problem: if we try to return immediately saying whether the needed data are available,
* there are some cases where preparing the reqeusted object might take only a few hundred milliseconds or less.
* there are some cases where preparing the requested object might take only a few hundred milliseconds or less.
* In that case then we don't want the caller to have to re-poll. In this case a Future.get() with timeout is good.
*
* Created by abyrd on 2018-09-14
Expand Down Expand Up @@ -123,9 +123,7 @@ public LoaderState<V> get (K key) {
setProgress(key, 0, "Starting...");
try {
V value = buildValue(key);
synchronized (map) {
map.put(key, new LoaderState(Status.PRESENT, null, 100, value));
}
setComplete(key, value);
} catch (Throwable t) {
// It's essential to trap Throwable rather than just Exception. Otherwise the executor
// threads can be killed by any Error that happens, stalling the executor.
Expand All @@ -139,7 +137,8 @@ public LoaderState<V> get (K key) {

/**
* Override this method in concrete subclasses to specify the logic to build/calculate/fetch a value.
* Implementations may call setProgress to report progress on long operations.
* Implementations may call setProgress to report progress on long operations; if they do so, any callers of this
* method are responsible for also calling setComplete() to ensure loaded objects are marked as PRESENT.
* Throw an exception to indicate an error has occurred and the building process cannot complete.
* It's not entirely clear this should return a value - might be better to call setValue within the overridden
* method, just as we call setProgress or setError.
Expand All @@ -155,6 +154,12 @@ public void setProgress(K key, int percentComplete, String message) {
}
}

public void setComplete(K key, V value) {
synchronized (map) {
map.put(key, new LoaderState(Status.PRESENT, "Loaded", 100, value));
}
}

/**
* Call this method inside the buildValue method to indicate that an unrecoverable error has happened.
* FIXME this will permanently associate an error with the key. No further attempt will ever be made to create the value.
Expand Down

0 comments on commit fe68b94

Please sign in to comment.