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

Change timeouts configuration #1526

Merged
merged 8 commits into from
Oct 11, 2021
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 2 additions & 10 deletions docs/config-app.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,6 @@ Removes and downloads file again if depending service cant process probably corr
- `external-url` - the setting stands for external URL prebid server is reachable by, for example address of the load-balancer e.g. http://prebid.host.com.
- `admin.port` - the port to listen on administration requests.

## Auction (Legacy)
- `default-timeout-ms` - this setting controls default timeout for /auction endpoint.
- `max-timeout-ms` - this setting controls maximum timeout for /auction endpoint.
- `timeout-adjustment-ms` - reduces timeout value passed in legacy Auction request so that Prebid Server can handle timeouts from adapters and respond to the request before it times out.

## Default bid request
- `default-request.file.path` - path to a JSON file containing the default request

Expand All @@ -84,11 +79,8 @@ Removes and downloads file again if depending service cant process probably corr
- `auction.validations.secure-markup` - enables secure markup validation. Possible values: `skip`, `enforce`, `warn`. Default is `skip`.
- `auction.host-schain-node` - defines global schain node that will be appended to `request.source.ext.schain.nodes` passed to bidders

## Amp (OpenRTB)
- `amp.default-timeout-ms` - default operation timeout for OpenRTB Amp requests.
- `amp.max-timeout-ms` - maximum operation timeout for OpenRTB Amp requests.
- `amp.timeout-adjustment-ms` - reduces timeout value passed in Amp request so that Prebid Server can handle timeouts from adapters and respond to the AMP RTC request before it times out.
- `amp.custom-targeting` - a list of bidders whose custom targeting should be included in AMP responses.
## Event
- `event.default-timeout-ms` - timeout for event notifications

## Timeout notification
- `auction.timeout-notification.timeout-ms` - HTTP timeout to use when sending notifications about bidder timeouts
Expand Down
18 changes: 13 additions & 5 deletions src/main/java/org/prebid/server/auction/TimeoutResolver.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,25 @@ public class TimeoutResolver {
private final long timeoutAdjustment;

public TimeoutResolver(long defaultTimeout, long maxTimeout, long timeoutAdjustment) {
if (maxTimeout < defaultTimeout) {
throw new IllegalArgumentException(
String.format("Max timeout cannot be less than default timeout: max=%d, default=%d", maxTimeout,
defaultTimeout));
}
validateTimeouts(defaultTimeout, maxTimeout);

this.defaultTimeout = defaultTimeout;
this.maxTimeout = maxTimeout;
this.timeoutAdjustment = timeoutAdjustment;
}

private static void validateTimeouts(long defaultTimeout, long maxTimeout) {
if (defaultTimeout <= 0 || maxTimeout <= 0) {
throw new IllegalArgumentException(
String.format("Both default and max timeouts should be grater than 0: max=%d, default=%d",
maxTimeout, defaultTimeout));
} else if (maxTimeout < defaultTimeout) {
throw new IllegalArgumentException(
String.format("Max timeout cannot be less than default timeout: max=%d, default=%d", maxTimeout,
defaultTimeout));
}
}

/**
* Resolves timeout according to given in request and pre-configured default and max values.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,13 @@ public class NotificationEventHandler implements Handler<RoutingContext> {
private static final String TRACKING_PIXEL_PNG = "static/tracking-pixel.png";
private static final String PNG_CONTENT_TYPE = "image/png";

private static final long DEFAULT_TIMEOUT = 1000L;

private final UidsCookieService uidsCookieService;
private final ApplicationEventService applicationEventService;
private final UserService userService;
private final AnalyticsReporterDelegator analyticsDelegator;
private final TimeoutFactory timeoutFactory;
private final ApplicationSettings applicationSettings;
private final long defaultTimeoutMillis;
private final boolean dealsEnabled;
private final TrackingPixel trackingPixel;

Expand All @@ -62,6 +61,7 @@ public NotificationEventHandler(UidsCookieService uidsCookieService,
AnalyticsReporterDelegator analyticsDelegator,
TimeoutFactory timeoutFactory,
ApplicationSettings applicationSettings,
long defaultTimeoutMillis,
boolean dealsEnabled) {

this.uidsCookieService = Objects.requireNonNull(uidsCookieService);
Expand All @@ -70,6 +70,7 @@ public NotificationEventHandler(UidsCookieService uidsCookieService,
this.analyticsDelegator = Objects.requireNonNull(analyticsDelegator);
this.timeoutFactory = Objects.requireNonNull(timeoutFactory);
this.applicationSettings = Objects.requireNonNull(applicationSettings);
this.defaultTimeoutMillis = defaultTimeoutMillis;
this.dealsEnabled = dealsEnabled;

trackingPixel = createTrackingPixel();
Expand Down Expand Up @@ -116,7 +117,7 @@ public void handle(RoutingContext routingContext) {
* Returns {@link Account} fetched by {@link ApplicationSettings}.
*/
private Future<Account> getAccountById(String accountId) {
return applicationSettings.getAccountById(accountId, timeoutFactory.create(DEFAULT_TIMEOUT))
return applicationSettings.getAccountById(accountId, timeoutFactory.create(defaultTimeoutMillis))
.recover(exception -> handleAccountExceptionOrFallback(exception, accountId));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,15 +160,6 @@ SchainResolver schainResolver(
}

@Bean
TimeoutResolver timeoutResolver(
@Value("${default-timeout-ms}") long defaultTimeout,
@Value("${max-timeout-ms}") long maxTimeout,
@Value("${timeout-adjustment-ms}") long timeoutAdjustment) {

return new TimeoutResolver(defaultTimeout, maxTimeout, timeoutAdjustment);
}

@Bean("auctionTimeoutResolver")
TimeoutResolver auctionTimeoutResolver(
@Value("${auction.default-timeout-ms}") long defaultTimeout,
@Value("${auction.max-timeout-ms}") long maxTimeout,
Expand All @@ -177,15 +168,6 @@ TimeoutResolver auctionTimeoutResolver(
return new TimeoutResolver(defaultTimeout, maxTimeout, timeoutAdjustment);
}

@Bean
TimeoutResolver ampTimeoutResolver(
@Value("${amp.default-timeout-ms}") long defaultTimeout,
@Value("${amp.max-timeout-ms}") long maxTimeout,
@Value("${amp.timeout-adjustment-ms}") long timeoutAdjustment) {

return new TimeoutResolver(defaultTimeout, maxTimeout, timeoutAdjustment);
}

@Bean
DebugResolver debugResolver(@Value("${debug.override-token:#{null}") String debugOverrideToken,
BidderCatalog bidderCatalog) {
Expand Down Expand Up @@ -222,7 +204,7 @@ Ortb2RequestFactory openRtb2RequestFactory(
@Value("${auction.blacklisted-accounts}") String blacklistedAccountsString,
UidsCookieService uidsCookieService,
RequestValidator requestValidator,
@Qualifier("auctionTimeoutResolver") TimeoutResolver timeoutResolver,
TimeoutResolver auctionTimeoutResolver,
TimeoutFactory timeoutFactory,
StoredRequestProcessor storedRequestProcessor,
ApplicationSettings applicationSettings,
Expand All @@ -238,7 +220,7 @@ Ortb2RequestFactory openRtb2RequestFactory(
blacklistedAccounts,
uidsCookieService,
requestValidator,
timeoutResolver,
auctionTimeoutResolver,
timeoutFactory,
storedRequestProcessor,
applicationSettings,
Expand All @@ -257,7 +239,7 @@ AuctionRequestFactory auctionRequestFactory(
Ortb2ImplicitParametersResolver ortb2ImplicitParametersResolver,
OrtbTypesResolver ortbTypesResolver,
PrivacyEnforcementService privacyEnforcementService,
@Qualifier("auctionTimeoutResolver") TimeoutResolver timeoutResolver,
TimeoutResolver auctionTimeoutResolver,
DebugResolver debugResolver,
JacksonMapper mapper) {

Expand All @@ -270,7 +252,7 @@ AuctionRequestFactory auctionRequestFactory(
new InterstitialProcessor(),
ortbTypesResolver,
privacyEnforcementService,
timeoutResolver,
auctionTimeoutResolver,
debugResolver,
mapper);
}
Expand All @@ -297,7 +279,7 @@ AmpRequestFactory ampRequestFactory(StoredRequestProcessor storedRequestProcesso
Ortb2ImplicitParametersResolver ortb2ImplicitParametersResolver,
FpdResolver fpdResolver,
PrivacyEnforcementService privacyEnforcementService,
TimeoutResolver timeoutResolver,
TimeoutResolver auctionTimeoutResolver,
DebugResolver debugResolver,
JacksonMapper mapper) {

Expand All @@ -309,7 +291,7 @@ AmpRequestFactory ampRequestFactory(StoredRequestProcessor storedRequestProcesso
ortb2ImplicitParametersResolver,
fpdResolver,
privacyEnforcementService,
timeoutResolver,
auctionTimeoutResolver,
debugResolver,
mapper);
}
Expand All @@ -322,7 +304,7 @@ VideoRequestFactory videoRequestFactory(
Ortb2RequestFactory ortb2RequestFactory,
Ortb2ImplicitParametersResolver ortb2ImplicitParametersResolver,
PrivacyEnforcementService privacyEnforcementService,
TimeoutResolver timeoutResolver,
TimeoutResolver auctionTimeoutResolver,
DebugResolver debugResolver,
JacksonMapper mapper) {

Expand All @@ -333,7 +315,7 @@ VideoRequestFactory videoRequestFactory(
ortb2ImplicitParametersResolver,
storedRequestProcessor,
privacyEnforcementService,
timeoutResolver,
auctionTimeoutResolver,
debugResolver,
mapper);
}
Expand All @@ -355,7 +337,7 @@ VideoStoredRequestProcessor videoStoredRequestProcessor(
VideoRequestValidator videoRequestValidator,
Metrics metrics,
TimeoutFactory timeoutFactory,
TimeoutResolver timeoutResolver,
TimeoutResolver auctionTimeoutResolver,
JacksonMapper mapper,
JsonMerger jsonMerger) {

Expand All @@ -370,7 +352,7 @@ VideoStoredRequestProcessor videoStoredRequestProcessor(
videoRequestValidator,
metrics,
timeoutFactory,
timeoutResolver,
auctionTimeoutResolver,
mapper,
jsonMerger);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,7 @@ NotificationEventHandler notificationEventHandler(
AnalyticsReporterDelegator analyticsReporterDelegator,
TimeoutFactory timeoutFactory,
ApplicationSettings applicationSettings,
@Value("${event.default-timeout-ms}") long defaultTimeoutMillis,
@Value("${deals.enabled}") boolean dealsEnabled) {

return new NotificationEventHandler(
Expand All @@ -400,6 +401,7 @@ NotificationEventHandler notificationEventHandler(
analyticsReporterDelegator,
timeoutFactory,
applicationSettings,
defaultTimeoutMillis,
dealsEnabled);
}

Expand Down
9 changes: 2 additions & 7 deletions src/main/resources/application.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,6 @@ external-url: http://localhost:8080
host-id: localhost
datacenter-region: local
vendor: local
default-timeout-ms: 900
max-timeout-ms: 5000
timeout-adjustment-ms: 30
system: system
sub-system: subSystem
infra: infra
Expand Down Expand Up @@ -130,10 +127,8 @@ auction:
video:
stored-request-required: false
stored-requests-timeout-ms: 90
amp:
default-timeout-ms: 900
max-timeout-ms: 5000
timeout-adjustment-ms: 30
event:
default-timeout-ms: 1000
setuid:
default-timeout-ms: 2000
vtrack:
Expand Down
12 changes: 12 additions & 0 deletions src/test/java/org/prebid/server/auction/TimeoutResolverTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,18 @@ public void setUp() {
timeoutResolver = new TimeoutResolver(DEFAULT_TIMEOUT, MAX_TIMEOUT, 10L);
}

@Test
public void creationShouldFailIfDefaultTimeoutEqualOrLassThanZero() {
assertThatIllegalArgumentException().isThrownBy(() -> new TimeoutResolver(0L, 1L, 0L))
.withMessage("Both default and max timeouts should be grater than 0: max=1, default=0");
}

@Test
public void creationShouldFailIfMaxTimeoutEqualsOrLassThanZero() {
assertThatIllegalArgumentException().isThrownBy(() -> new TimeoutResolver(1L, 0L, 0L))
.withMessage("Both default and max timeouts should be grater than 0: max=0, default=1");
}

@Test
public void creationShouldFailIfMaxTimeoutLessThanDefault() {
assertThatIllegalArgumentException().isThrownBy(() -> new TimeoutResolver(2L, 1L, 0L))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ public void setUp() {
analyticsReporterDelegator,
timeoutFactory,
applicationSettings,
1000,
true);
}

Expand Down Expand Up @@ -399,6 +400,7 @@ public void shouldNotProcessLineItemEventWhenDealsDisabled() {
analyticsReporterDelegator,
timeoutFactory,
applicationSettings,
1000,
false);

given(httpRequest.params()).willReturn(MultiMap.caseInsensitiveMultiMap()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -272,15 +272,11 @@ http-client.circuit-breaker.idle-expire-hours=24
http-client.circuit-breaker.opening-threshold=1
http-client.circuit-breaker.opening-interval-ms=1000
http-client.circuit-breaker.closing-interval-ms=10000
default-timeout-ms=2000
timeout-adjustment-ms=0
auction.default-timeout-ms=2000
auction.timeout-adjustment-ms=0
auction.generate-source-tid=false
currency-converter.external-rates.enabled=true
currency-converter.external-rates.url=http://localhost:8090/currency-rates
amp.default-timeout-ms=2000
amp.timeout-adjustment-ms=30
amp.custom-targeting=rubicon
cache.scheme=http
cache.host=localhost:8090
Expand Down