Skip to content

Commit

Permalink
Revert DefaultDocumentationRequestHandler to be blocking again (#824)
Browse files Browse the repository at this point in the history
Back in January, we changed the blocking DDRH to be non-blocking (fail requests sent during initialization)
to solve an edge-case issue of request thread pool exhaustion. This started causing a lot more complaints,
as many more use cases were now seeing errors for OPTIONS calls that would otherwise be successful.

This change brings back the old blocking behavior as default, and adds NonBlockingDocumentationRequestHandler
as a subclass of the DDRH which users can configure if they see thread pool exhaustion in their use case.
  • Loading branch information
evanw555 authored Aug 26, 2022
1 parent bec0e63 commit 6d06615
Show file tree
Hide file tree
Showing 4 changed files with 95 additions and 19 deletions.
23 changes: 14 additions & 9 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,26 +14,30 @@ and what APIs have changed, if applicable.

## [Unreleased]

## [29.37.16] - 2022-08-24
- Make `DefaultDocumentationRequestHandler` blocking again to avoid the `503` errors users were frequently seeing for `OPTIONS` calls.
- Introduce the existing non-blocking ("fail fast") variant as optional subclass `NonBlockingDocumentationRequestHandler`.

## [29.37.15] - 2022-08-23
- Exclude transitive Netty dependency for ZooKeeper client.

## [29.37.14] - 2022-08-19
- Avoid casting classloader to URLLoader in ResourceModelEncoder and use ClassGraph to search for restspec file
- Avoid casting classloader to `URLLoader` in `ResourceModelEncoder` and use `ClassGraph` to search for restspec file

## [29.37.13] - 2022-08-15
- Fix d2-test-api dependencies
- Fix `d2-test-api` dependencies

## [29.37.12] - 2022-08-10
- Support removing cluster watches created due to cluster failout

## [29.37.11] - 2022-08-09
- Avoid using SmileFactoryBuilder to be more compatible with pre 2.10 jackson at runtime
- Avoid using `SmileFactoryBuilder` to be more compatible with pre-`2.10` jackson at runtime

## [29.37.10] - 2022-08-08
- Fix PrimitiveTemplateSpec not having className
- Fix `PrimitiveTemplateSpec` not having `className`

## [29.37.9] - 2022-08-07
- Add null-checks for cluster and service properties in D2ClientJmxManager
- Add null-checks for cluster and service properties in `D2ClientJmxManager`

## [29.37.8] - 2022-08-04
- Switch to use name regex pattern to skip deprecated fields in spec generation
Expand Down Expand Up @@ -88,7 +92,7 @@ and what APIs have changed, if applicable.
- Revert "Provide a mechanism to set a routing hint for the d2 request to get request symbol table (#787)"

## [29.33.8] - 2022-05-10
- Add (currently unused) models for D2FailoutProperties.
- Add (currently unused) models for `D2FailoutProperties`.

## [29.33.7] - 2022-05-04
- Silence Zookeeper errors in logs on race condition between watched events and async shutdown.
Expand All @@ -97,7 +101,7 @@ and what APIs have changed, if applicable.
- Provide a mechanism to set a routing hint for the d2 request to get request symbol table.

## [29.33.5] - 2022-05-02
- Expose RestLiConfig from RestLiServer.
- Expose `RestLiConfig` from `RestLiServer`.

## [29.33.4] - 2022-04-26
- Support failout redirection in D2 client.
Expand All @@ -112,7 +116,7 @@ and what APIs have changed, if applicable.
- Fix an Avro translation bug where optional fields in a partial default record are not treated properly.

## [29.33.0] - 2022-03-28
- Add Support for ByteString[] Query Parameters
- Add Support for `ByteString[]` Query Parameters

## [29.32.5] - 2022-03-22
- Updating newInstance usage which is deprecated in Java 9+
Expand Down Expand Up @@ -5309,7 +5313,8 @@ patch operations can re-use these classes for generating patch messages.

## [0.14.1]

[Unreleased]: https://github.com/linkedin/rest.li/compare/v29.37.15...master
[Unreleased]: https://github.com/linkedin/rest.li/compare/v29.37.16...master
[29.37.16]: https://github.com/linkedin/rest.li/compare/v29.37.15...v29.37.16
[29.37.15]: https://github.com/linkedin/rest.li/compare/v29.37.14...v29.37.15
[29.37.14]: https://github.com/linkedin/rest.li/compare/v29.37.13...v29.37.14
[29.37.13]: https://github.com/linkedin/rest.li/compare/v29.37.12...v29.37.13
Expand Down
2 changes: 1 addition & 1 deletion gradle.properties
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
version=29.37.15
version=29.37.16
group=com.linkedin.pegasus
org.gradle.configureondemand=true
org.gradle.parallel=true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,18 +34,20 @@
import com.linkedin.restli.internal.server.model.ResourceModel;
import com.linkedin.restli.server.RestLiConfig;
import com.linkedin.restli.server.RestLiDocumentationRequestHandler;
import com.linkedin.restli.server.RestLiServiceException;
import com.linkedin.restli.server.RoutingException;

import java.io.ByteArrayOutputStream;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.concurrent.atomic.AtomicBoolean;


/**
* Default {@link RestLiDocumentationRequestHandler} that serves both HTML and JSON documentation.
* This request handler initializes its renderers lazily (on the first request).
* The initializing is blocking, meaning that subsequent request threads will block until it's done.
*
* If a non-blocking request handler is needed, consider using {@link NonBlockingDocumentationRequestHandler}.
*
* @author Keren Jin
*/
Expand All @@ -72,13 +74,16 @@ public boolean shouldHandle(Request request)
@Override
public void handleRequest(RestRequest request, RequestContext requestContext, Callback<RestResponse> callback)
{
if (_shouldInitialize.getAndSet(false))
{
initializeRenderers();
}
if (_jsonRenderer == null || _htmlRenderer == null)
if (!_initialized)
{
callback.onError(new RestLiServiceException(HttpStatus.S_503_SERVICE_UNAVAILABLE, "Documentation renderers have not yet been initialized."));
synchronized (this)
{
if (!_initialized)
{
initializeRenderers();
_initialized = true;
}
}
}
try
{
Expand Down Expand Up @@ -244,6 +249,10 @@ else if (DOC_DATA_TYPE.equals(typeSegment))
build();
}

protected boolean isInitialized() {
return _initialized;
}

private static RoutingException createRoutingError(String path)
{
return new RoutingException(String.format("Invalid documentation path %s", path), HttpStatus.S_404_NOT_FOUND.getCode());
Expand All @@ -260,5 +269,5 @@ private static RoutingException createRoutingError(String path)
private RestLiDocumentationRenderer _jsonRenderer;
private RestLiConfig _config;
private Map<String, ResourceModel> _rootResources;
private final AtomicBoolean _shouldInitialize = new AtomicBoolean(true);
private volatile boolean _initialized = false;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
/*
Copyright (c) 2022 LinkedIn Corp.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package com.linkedin.restli.docgen;

import com.linkedin.common.callback.Callback;
import com.linkedin.r2.message.RequestContext;
import com.linkedin.r2.message.rest.RestRequest;
import com.linkedin.r2.message.rest.RestResponse;
import com.linkedin.restli.common.HttpStatus;
import com.linkedin.restli.server.RestLiDocumentationRequestHandler;
import com.linkedin.restli.server.RestLiServiceException;
import java.util.concurrent.atomic.AtomicBoolean;


/**
* Non-blocking extension of the default {@link RestLiDocumentationRequestHandler} that is needed in special use cases.
* This implementation blocks on the request thread that lazily initializes the renderers, but refuses to block
* subsequent request threads, failing the requests instead.
*
* The advantage of this approach is that the request thread pool may avoid exhaustion in the case of lengthy initialization.
* The downside is that requests sent during initialization will fail, which may cause client problems and confusion.
*
* @author Evan Williams
*/
public class NonBlockingDocumentationRequestHandler extends DefaultDocumentationRequestHandler
{
private final AtomicBoolean _shouldInitialize = new AtomicBoolean(true);

@Override
public void handleRequest(RestRequest request, RequestContext requestContext, Callback<RestResponse> callback)
{
// The first request thread should perform the initialization and render the response
if (_shouldInitialize.getAndSet(false))
{
super.handleRequest(request, requestContext, callback);
}
// For subsequent requests sent during initialization, immediately return a failure response
else if (!isInitialized())
{
callback.onError(new RestLiServiceException(HttpStatus.S_503_SERVICE_UNAVAILABLE, "Documentation renderers have not yet been initialized."));
}
// For all requests received after initialization has completed, render the response
else
{
super.handleRequest(request, requestContext, callback);
}
}
}

0 comments on commit 6d06615

Please sign in to comment.