Skip to content

Commit

Permalink
Fix max search-window when paging
Browse files Browse the repository at this point in the history
The PagingService was initialized with the dynamic max-search-window value, and not the request max search-window.
This commit also updates the Transmodel API doc.
  • Loading branch information
t2gran committed Oct 23, 2024
1 parent cee960f commit c35f1c9
Show file tree
Hide file tree
Showing 12 changed files with 147 additions and 49 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.opentripplanner.apis.transmodel.mapping.TransitIdMapper;
import org.opentripplanner.routing.api.request.RouteRequest;
import org.opentripplanner.standalone.api.OtpServerRequestContext;
import org.opentripplanner.standalone.config.routerconfig.TransitRoutingConfig;
import org.opentripplanner.transit.service.TimetableRepository;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -67,14 +68,20 @@ public TransmodelAPIOldPath(
public static void setUp(
TransmodelAPIParameters config,
TimetableRepository timetableRepository,
RouteRequest defaultRouteRequest
RouteRequest defaultRouteRequest,
TransitRoutingConfig transitRoutingConfig
) {
if (config.hideFeedId()) {
TransitIdMapper.setupFixedFeedId(timetableRepository.getAgencies());
}
tracingHeaderTags = config.tracingHeaderTags();
maxNumberOfResultFields = config.maxNumberOfResultFields();
schema = TransmodelGraphQLSchema.create(defaultRouteRequest, timetableRepository.getTimeZone());
schema =
TransmodelGraphQLSchema.create(
defaultRouteRequest,
timetableRepository.getTimeZone(),
transitRoutingConfig
);
}

@POST
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@
import org.opentripplanner.model.plan.legreference.LegReference;
import org.opentripplanner.model.plan.legreference.LegReferenceSerializer;
import org.opentripplanner.routing.alertpatch.TransitAlert;
import org.opentripplanner.routing.algorithm.raptoradapter.transit.TransitTuningParameters;
import org.opentripplanner.routing.api.request.RouteRequest;
import org.opentripplanner.routing.error.RoutingValidationException;
import org.opentripplanner.routing.graphfinder.NearbyStop;
Expand All @@ -133,17 +134,29 @@ public class TransmodelGraphQLSchema {

private final DefaultRouteRequestType routing;

private final TransitTuningParameters transitTuningParameters;

private final ZoneId timeZoneId;

private final Relay relay = new Relay();

private TransmodelGraphQLSchema(RouteRequest defaultRequest, ZoneId timeZoneId) {
private TransmodelGraphQLSchema(
RouteRequest defaultRequest,
ZoneId timeZoneId,
TransitTuningParameters transitTuningParameters
) {
this.timeZoneId = timeZoneId;
this.routing = new DefaultRouteRequestType(defaultRequest);
this.transitTuningParameters = transitTuningParameters;
}

public static GraphQLSchema create(RouteRequest defaultRequest, ZoneId timeZoneId) {
return new TransmodelGraphQLSchema(defaultRequest, timeZoneId).create();
public static GraphQLSchema create(
RouteRequest defaultRequest,
ZoneId timeZoneId,
TransitTuningParameters transitTuningParameters
) {
return new TransmodelGraphQLSchema(defaultRequest, timeZoneId, transitTuningParameters)
.create();
}

@SuppressWarnings("unchecked")
Expand Down Expand Up @@ -340,6 +353,7 @@ private GraphQLSchema create() {

GraphQLFieldDefinition tripQuery = TripQuery.create(
routing,
transitTuningParameters,
tripType,
durationPerStreetModeInput,
penaltyForStreetMode,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import graphql.schema.GraphQLNonNull;
import graphql.schema.GraphQLOutputType;
import graphql.schema.GraphQLScalarType;
import java.time.Duration;
import org.opentripplanner.apis.transmodel.TransmodelGraphQLPlanner;
import org.opentripplanner.apis.transmodel.model.DefaultRouteRequestType;
import org.opentripplanner.apis.transmodel.model.EnumTypes;
Expand All @@ -20,6 +21,7 @@
import org.opentripplanner.apis.transmodel.model.framework.PassThroughPointInputType;
import org.opentripplanner.apis.transmodel.model.framework.PenaltyForStreetModeType;
import org.opentripplanner.apis.transmodel.model.framework.TransmodelDirectives;
import org.opentripplanner.routing.algorithm.raptoradapter.transit.TransitTuningParameters;
import org.opentripplanner.routing.api.request.preference.RoutingPreferences;
import org.opentripplanner.routing.core.VehicleRoutingOptimizeType;

Expand All @@ -37,6 +39,7 @@ public class TripQuery {

public static GraphQLFieldDefinition create(
DefaultRouteRequestType routing,
TransitTuningParameters transitTuningParameters,
GraphQLOutputType tripType,
GraphQLInputObjectType durationPerStreetModeType,
GraphQLInputObjectType penaltyForStreetMode,
Expand Down Expand Up @@ -87,30 +90,42 @@ Normally this is when the search is performed (now), plus a small grace period t
.newArgument()
.name("searchWindow")
.description(
"The length of the search-window in minutes. This parameter is optional." +
"\n\n" +
"The search-window is defined as the duration between the earliest-departure-time(EDT) and " +
"the latest-departure-time(LDT). OTP will search for all itineraries in this departure " +
"window. If `arriveBy=true` the `dateTime` parameter is the latest-arrival-time, so OTP " +
"will dynamically calculate the EDT. Using a short search-window is faster than using a " +
"longer one, but the search duration is not linear. Using a \"too\" short search-window will " +
"waste resources server side, while using a search-window that is too long will be slow." +
"\n\n" +
"OTP will dynamically calculate a reasonable value for the search-window, if not provided. The " +
"calculation comes with a significant overhead (10-20% extra). Whether you should use the " +
"dynamic calculated value or pass in a value depends on your use-case. For a travel planner " +
"in a small geographical area, with a dense network of public transportation, a fixed value " +
"between 40 minutes and 2 hours makes sense. To find the appropriate search-window, adjust it " +
"so that the number of itineraries on average is around the wanted `numItineraries`. Make " +
"sure you set the `numItineraries` to a high number while testing. For a country wide area like " +
"Norway, using the dynamic search-window is the best." +
"\n\n" +
"When paginating, the search-window is calculated using the `numItineraries` in the original " +
"search together with statistics from the search for the last page. This behaviour is " +
"configured server side, and can not be overridden from the client." +
"\n\n" +
"The search-window used is returned to the response metadata as `searchWindowUsed` for " +
"debugging purposes."
"""
The length of the search-window in minutes. This parameter is optional.
The search-window is defined as the duration between the earliest-departure-time(EDT) and
the latest-departure-time(LDT). OTP will search for all itineraries in this departure
window. If `arriveBy=true` the `dateTime` parameter is the latest-arrival-time, so OTP
will dynamically calculate the EDT. Using a short search-window is faster than using a
longer one, but the search duration is not linear. Using a \"too\" short search-window will
waste resources server side, while using a search-window that is too long will be slow.
OTP will dynamically calculate a reasonable value for the search-window, if not provided. The
calculation comes with a significant overhead (10-20% extra). Whether you should use the
dynamic calculated value or pass in a value depends on your use-case. For a travel planner
in a small geographical area, with a dense network of public transportation, a fixed value
between 40 minutes and 2 hours makes sense. To find the appropriate search-window, adjust it
so that the number of itineraries on average is around the wanted `numTripPatterns`. Make
sure you set the `numTripPatterns` to a high number while testing. For a country wide area like
Norway, using the dynamic search-window is the best.
When paginating, the search-window is calculated using the `numTripPatterns` in the original
search together with statistics from the search for the last page. This behaviour is
configured server side, and can not be overridden from the client. The paging may even
exceed the maximum value.
The search-window used is returned to the response metadata as `searchWindowUsed`.
This can be used by the client to calculate the when the next page start/end.
Note! In some cases you may have to page many times to get all the results you want.
This is intended. Increasing the search-window beyond the max value is NOT going to be
much faster. Instead the client can inform the user about the progress.
Maximum value: {max-search-window}
""".replace(
"{max-search-window}",
durationInMinutesToString(transitTuningParameters.maxSearchWindow())
)
)
.type(Scalars.GraphQLInt)
.build()
Expand All @@ -120,9 +135,12 @@ Normally this is when the search is performed (now), plus a small grace period t
.newArgument()
.name("pageCursor")
.description(
"Use the cursor to go to the next \"page\" of itineraries. Copy the cursor from " +
"the last response and keep the original request as is. This will enable you to " +
"search for itineraries in the next or previous time-window."
"""
Use the cursor to go to the next \"page\" of itineraries. Copy the cursor from the last
response and keep the original request as is. This will enable you to search for
itineraries in the next or previous search-window. The paging will automatically scale
up/down the search-window to fit the `numTripPatterns`.
"""
)
.type(Scalars.GraphQLString)
.build()
Expand Down Expand Up @@ -646,4 +664,11 @@ private static String enumValAsString(GraphQLEnumType enumType, Enum<?> otpVal)
.get()
.getName();
}

/**
* Format and return: "2440 minute (48h)"
*/
private static String durationInMinutesToString(Duration value) {
return "%d minutes (%s)".formatted(value.toMinutes(), value.toHours() + "h");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,10 @@ public static PagingService createPagingService(
) {
return new PagingService(
transitTuningParameters.pagingSearchWindowAdjustments(),
// The dynamic search-window is not the ame as requested search-window, but in lack
// of a something else we use the raptor dynamic min here.
raptorTuningParameters.dynamicSearchWindowCoefficients().minWindow(),
raptorTuningParameters.dynamicSearchWindowCoefficients().maxWindow(),
transitTuningParameters.maxSearchWindow(),
searchWindowOf(raptorSearchParamsUsed),
edt(searchStartTime, raptorSearchParamsUsed),
lat(searchStartTime, raptorSearchParamsUsed),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,11 @@
import java.util.Collections;
import java.util.List;
import java.util.Locale;
import java.util.Objects;
import java.util.function.Consumer;
import javax.annotation.Nullable;
import org.opentripplanner.framework.collection.ListSection;
import org.opentripplanner.framework.lang.ObjectUtils;
import org.opentripplanner.framework.time.DateUtils;
import org.opentripplanner.framework.tostring.ToStringBuilder;
import org.opentripplanner.model.GenericLocation;
Expand All @@ -26,6 +28,7 @@
import org.opentripplanner.routing.api.response.RoutingError;
import org.opentripplanner.routing.api.response.RoutingErrorCode;
import org.opentripplanner.routing.error.RoutingValidationException;
import org.opentripplanner.standalone.config.routerconfig.TransitRoutingConfig;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -332,12 +335,25 @@ private boolean hasMaxSearchWindow() {
return maxSearchWindow != null;
}

/**
* For testing only. Use {@link TransitRoutingConfig#maxSearchWindow()} instead.
* @see #initMaxSearchWindow(Duration)
*/
public Duration maxSearchWindow() {
return maxSearchWindow;
}

public void setMaxSearchWindow(@Nullable Duration maxSearchWindow) {
this.maxSearchWindow = maxSearchWindow;
/**
* Initialize the maxSearchWindow from the transit config. This is necessary because the
* default route request is configured before the {@link TransitRoutingConfig}.
*/
public void initMaxSearchWindow(Duration maxSearchWindow) {
this.maxSearchWindow =
ObjectUtils.requireNotInitialized(
"maxSearchWindow",
this.maxSearchWindow,
Objects.requireNonNull(maxSearchWindow)
);
}

public Locale locale() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public RouterConfig(JsonNode node, String source, boolean logUnusedParams) {
this.transmodelApi = new TransmodelAPIConfig("transmodelApi", root);
this.routingRequestDefaults = mapDefaultRouteRequest("routingDefaults", root);
this.transitConfig = new TransitRoutingConfig("transit", root, routingRequestDefaults);
this.routingRequestDefaults.setMaxSearchWindow(transitConfig.maxSearchWindow());
this.routingRequestDefaults.initMaxSearchWindow(transitConfig.maxSearchWindow());
this.updatersParameters = new UpdatersConfig(root);
this.rideHailingConfig = new RideHailingServicesConfig(root);
this.vectorTileConfig = VectorTileConfig.mapVectorTilesParameters(root, "vectorTiles");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,7 @@ heuristics perform a Raptor search (one-iteration) to find a trip which we use t
.summary("Upper limit for the search-window calculation.")
.description(
"""
Long search windows consumes a lot of resources and may take a long time. Use this parameter to
Long search windows consumes a lot of resources and may take a long time. Use this parameter to
tune the desired maximum search time.
This is the parameter that affects the response time most, the downside is that a search is only
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,8 @@ private void setupTransitRoutingServer() {
TransmodelAPI.setUp(
routerConfig().transmodelApi(),
timetableRepository(),
routerConfig().routingRequestDefaults()
routerConfig().routingRequestDefaults(),
routerConfig().transitTuningConfig()
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
public class DefaultServerRequestContext implements OtpServerRequestContext {

private final List<RideHailingService> rideHailingServices;
private RouteRequest routeRequest = null;
private final Graph graph;
private final TransitService transitService;
private final TransitRoutingConfig transitRoutingConfig;
Expand All @@ -52,6 +51,8 @@ public class DefaultServerRequestContext implements OtpServerRequestContext {
private final StreetLimitationParametersService streetLimitationParametersService;
private final LuceneIndex luceneIndex;

private RouteRequest defaultRouteRequestWithTimeSet = null;

/**
* Make sure all mutable components are copied/cloned before calling this constructor.
*/
Expand Down Expand Up @@ -142,10 +143,10 @@ public static DefaultServerRequestContext create(
@Override
public RouteRequest defaultRouteRequest() {
// Lazy initialize request-scoped request to avoid doing this when not needed
if (routeRequest == null) {
routeRequest = routeRequestDefaults.copyWithDateTimeNow();
if (defaultRouteRequestWithTimeSet == null) {
defaultRouteRequestWithTimeSet = routeRequestDefaults.copyWithDateTimeNow();
}
return routeRequest;
return defaultRouteRequestWithTimeSet;
}

/**
Expand Down
Loading

0 comments on commit c35f1c9

Please sign in to comment.